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

Add a note that this is discouraged #37

Closed
gaearon opened this issue Jul 24, 2018 · 4 comments
Closed

Add a note that this is discouraged #37

gaearon opened this issue Jul 24, 2018 · 4 comments

Comments

@gaearon
Copy link

gaearon commented Jul 24, 2018

Hi! I help maintain React.

I realize where the need is coming from, but I would appreciate if the project included a large disclaimer that this approach is discouraged. It is very fragile, already doesn’t match the exact semantics of React, and the gap will only widen in the future. We don’t recommend any libraries that want to work well with React in longer term to depend on this, or to use an approach like this.

What’s the alternatives? We are currently working on getting Suspense ready for use in open source. That should eventually cover data fetching needs (which is what motivated Apollo to do this). For other needs there may be other appropriate solutions.

Sorry to be a bummer. Thanks!

@hamlim
Copy link

hamlim commented Jul 24, 2018

Coming from the standpoint of a developer that has worked a decent amount with a similar implementation as react-tree-walker I can understand the need to warn about the use of libraries that rely on other library internals.

However it would be nice to get a better understanding on some of the other appropriate solutions to these problems. Even for data fetching at the moment, at least until suspense is available in a stable release of react-dom / react-dom/server.

One use case I ran into is communicating leaf node data back up to the parent component (some of this data is static, other parts rely on where the leaf node is nested within the react tree). To me the react-call-return pattern seemed like the closest option that was available to support this kind of need, but that has been removed and never supported createContext if I understand correctly.

Let me know if there are other discussions elsewhere (on github or in other support communities around React) that are already talking about these problems/patterns that I can move this discussion to. I don't want to sidetrack this issue, only looking for additional insight/guidance/ideas.

@ctrlplusb
Copy link
Owner

@gaearon I really appreciate your effort to ensure the health of the React eco-system. 👏

I am more than happy to update the docs to inform user's of the risk involved when using this library, along with a suggestion to keep their eyes on the imminent release of Suspense.

I apologise that hadn't considered doing this earlier - it reflects a bit of irresponsibility on my part.

Can't wait for Suspense. 🙏

@ctrlplusb
Copy link
Owner

Done 👍

@gaearon
Copy link
Author

gaearon commented Jul 25, 2018

One use case I ran into is communicating leaf node data back up to the parent component (some of this data is static, other parts rely on where the leaf node is nested within the react tree). To me the react-call-return pattern seemed like the closest option that was available to support this kind of need, but that has been removed and never supported createContext if I understand correctly.

Wanna raise an RFC with your ideas? We want to bring back call/return in some form but we need to figure out a compelling API for it.

@ctrlplusb Thanks for super fast response!

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

3 participants