search

ctrlplusb / react-tree-walker

Walk a React (or Preact) element tree, executing a "visitor" function against each element.
MIT License
345 stars 34 forks source link

Add a note that this is discouraged #37

Closed gaearon closed 6 years ago

gaearon commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

Done 👍

gaearon commented 6 years ago

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!