Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hoist-non-react-methods #11016

Closed
bradennapier opened this issue Oct 1, 2017 · 1 comment
Closed

hoist-non-react-methods #11016

bradennapier opened this issue Oct 1, 2017 · 1 comment

Comments

@bradennapier
Copy link

bradennapier commented Oct 1, 2017

I know this has been discussed in various places in the past, specifically requesting that @gaearon add it to redux IIRC. I know the reasoning behind not using something like this is somewhat sound and this is a topic that doesn't really have a perfect solution at this time... but what other "drawbacks" can you guys think of to a package to hoist-non-react-methods when utilizing HOC?

The way I was previously doing it doesn't work with React16 / Fiber so I've recently had to rewrite it a bit and move the logic into componentDidMount and use a ref of the child rather than the component prototype... but it appears to do the job well enough.

Want to make sure I am not forgetting something significant, especially with how Fiber works.

Obviously using this heavily wouldn't be a good idea, but in this case it is a HOC which largely should be invisible in its implementation (simply injecting a prop). Changing how passing refs works simply didn't seem to be an appropriate solution.

https://gist.github.com/bradennapier/5a61c77318b075cf0e4bc207c40faba8

Where the implementation would look something like this:

https://gist.github.com/bradennapier/59192f736bf1c4d77a4cda8a7532facf

Obviously it isn't ideal... just trying to consider the best way to implement a more transparent way of handling this.


So far the only real problem I can see with the implementation would be methods which are not present on the child at the time of mounting... which I think is acceptable drawback. I would think it wouldn't be too hard to adjust for that situation as well though?

@gaearon
Copy link
Collaborator

gaearon commented Oct 1, 2017

IMO the “hoisting” is a clumsy workaround for a missing feature in React: #4213.
Let’s keep the discussion there.

@gaearon gaearon closed this as completed Oct 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants