search

facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
227.85k stars 46.51k forks source link

unstable_renderSubtreeIntoContainer does not transfer context #16721

Closed quantizor closed 5 years ago

quantizor commented 5 years ago

Do you want to request a feature or report a bug? bug

What is the current behavior? unstable_renderSubtreeIntoContainer does not inherit the context from its current tree and transfer it to the rendered subtree

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below: https://codesandbox.io/s/portal-context-propagation-169-jid1w

What is the expected behavior? the second set of text should also be red

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React? React v16+

gaearon commented 5 years ago

This API is on its way to being deprecated and deleted. Portals are the intended alternative.

As a legacy API that was never stable, it’s not expected to support new features like modern Context.

fedulovivan commented 4 years ago

Also faced with this problem during upgrade of legacy code base from react 15 to latest react 16. We depend on material ui v0 which have utility component RenderToLayer which is implemented on top of unstable_renderSubtreeIntoContainer. A components connected to redux with connect loose access to context with redux state when being renderred as children of material-ui/Dialog or material-ui/Popover. As the result child component suddenly throws Could not find "store" in the context of "Connect(...)". Either wrap the root component in a \<Provider>, or pass a custom React context provider to \<Provider> and the corresponding React context consumer to Connect(...) in connect options

gaearon commented 4 years ago

As I mentioned before, unstable_renderSubtreeIntoContainer has never been a supported API and will be deleted. You need to either upgrade the library or fork it and migrate RenderToLayer to use portals instead.

fedulovivan commented 4 years ago

Yep, thanks for quick reply, Dan. Here is almost working replacement for RenderToLayer - https://github.com/facebook/react/issues/11387#issuecomment-355395803

mogelbrod commented 4 years ago

As I mentioned before, unstable_renderSubtreeIntoContainer has never been a supported API and will be deleted. You need to either upgrade the library or fork it and migrate RenderToLayer to use portals instead.

@gaearon: Any kind of feedback would be appreciated on the issue @fedulovivan mentioned. The last response we got from any React contributor was way back in 2018, and all the workarounds we've come up with are still problematic 😐

gaearon commented 4 years ago

@mogelbrod I don't currently have anything to add to that, but something like this (https://github.com/facebook/react/issues/11387#issuecomment-355395803) seems reasonable to me if you're migrating an existing component.

mogelbrod commented 4 years ago

@mogelbrod I don't currently have anything to add to that, but something like this (#11387 (comment)) seems reasonable to me if you're migrating an existing component.

Thanks for the quick response!

Pointing out some notable issues with that workaround:

Is the best bet for us to stay on the lookout for issues/PRs related to portals/unstable_renderSubtreeIntoContainer and keep referring to the issue, or are there other ways we might be able to contribute?

A solution that works in "all" cases is obviously what the people in the thread are striving for. Unfortunately it doesn't look to be possible with the current React API, so the next best thing would be some insight into if any changes are being considered, so we can adapt accordingly. Of course no single person (you or any other individual) owes us a response, but the current situation results a sense of hopelessness (at least for me).

gaearon commented 4 years ago

Thanks for the context about workaround. Since you already have that domain knowledge, the best next step is probably to write an RFC for the behavior you want and the alternatives you considered: https://github.com/reactjs/rfcs. Keep in mind that an RFC saying "let's just change this" is unlikely to be helpful. Writing a good RFC requires both understanding of why we have the current behavior, and a plan to change it in a way that suits your use cases without regressing on others.

Regardless of that, unstable_renderSubtreeIntoContainer is not supported, so let's untangle these two discussions. We will not be adding propagation of Context to it because the whole API is frozen and will not be updated.

mogelbrod commented 4 years ago

@gaearon Thanks for the clear response and suggestion, it's very appreciated! An RFC definitely sounds lake the logical approach - I'll continue the discussion in the original issue 👍