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

Is it possible to share contexts between renderers? #17275

Open
diegomura opened this issue Nov 5, 2019 · 18 comments
Open

Is it possible to share contexts between renderers? #17275

diegomura opened this issue Nov 5, 2019 · 18 comments

Comments

@diegomura
Copy link
Contributor

diegomura commented Nov 5, 2019

What is the current behavior?

Hey 👋 I maintain react-pdf. Thanks for your awesome work and making react-reconciler for us to use!

I've got many issues lately regarding context not working on my library and when doing tests I found out that context values aren't shared between renderers. This makes it impossible to share state such as themes, i18n, redux and more. As a bit of context, React-pdf is not a primary renderer, and as such, when used in the browser it runs on top of react-dom.

I found the isPrimaryRenderer reconciler option that's supposed to be used for "multiple renderers concurrently render using the same context objects" but still any access of the context inside react-pdf components get's just the initial value (even if the context was updated with other value). The same happens for react-art that also set isPrimaryRenderer=false.

Minimal demo

I prepared a quick demo using react-art so you can see how it currently works:

https://codesandbox.io/s/pedantic-hill-54kid?fontsize=14

What is the expected behavior?

Share contexts between renderers when using isPrimaryRenderer config. Is there a way of achieving this? Am I missing something?

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

React: 16.11.0
React-dom: 16.11.0

@sophiebits
Copy link
Contributor

Check out how react-art’s Surface component works. In particular, pass a class component’s “this” value to updateContainer:

https://github.com/facebook/react/blob/master/packages/react-art/src/ReactART.js

Hope that helps.

@markerikson
Copy link
Contributor

Paging @drcmda and @lavrton , who might be interested in bridging contexts between renderers.

@drcmda
Copy link

drcmda commented Nov 6, 2019

@sophiebits is this possible with function components as well?

@lavrton
Copy link

lavrton commented Nov 6, 2019

@markerikson thanks for mentioning.

@sompylasar, unfortunately, it doesn't help. The minimal demo, posted by @diegomura, shows the issue with react-art.

Related issue: #13332

As far as I know, there are currently no ways to pass contexts automatically into other renderers.
The workaround is to "bridge" the context manually:

<ThemeContext.Consumer>
      {value => (
        <Stage width={window.innerWidth} height={window.innerHeight}>
          <ThemeContext.Provider value={value}>
            <Layer>
              <ThemedRect />
            </Layer>
          </ThemeContext.Provider>
        </Stage>
      )}
</ThemeContext.Consumer>

@diegomura
Copy link
Contributor Author

@lavrton I came up to the same conclusion, although in my quick test just adding the context provider inside my renderer elements was enough to bridge the state (without manually passing value from one consumer to the provider). But I might have to check that again.

@sophiebits thank you so much for you quick response, but as @lavrton said, this issue is also happening in react-art. Does that means this is a bug?

@RodrigoHamuy
Copy link

Hi @lavrton , do you know why is not possible to pass context between renderers automatically? Sorry, it is still not clear to me why this doesn't work.

Coming from the same issue but with react-pixi:

Demo: https://codesandbox.io/s/react-pixi-context-example-eky26
Issue: pixijs/pixi-react#165

Thanks

@drcmda
Copy link

drcmda commented Nov 13, 2019

It only looks like the jsx you're giving to pixi via the stage component is in the same tree as the dom related jsx, which belongs to the react-dom reconciler, the one that holds the context provider. stage constructs a new reconciler, which is entirely isolated, and renders the children it received (the jsx inside stage). pixies reconciler has no idea where it is, it bears no relation to the surrounding reconciler and can't share its dynamics, like context, suspense, error boundaries and so on.

if react wanted to solve this we would probably need something like a first class react component, like a portal, which is returned by the react-reconcilers createContainer function. this will be rendered into the root reconciler as a regular element belonging to its tree. this way it would know it must pass its internals on. @gaearon described something like that once, but can't find it right now.

@jquense
Copy link
Contributor

jquense commented May 21, 2020

This doesn't seem to be solved? I'd also really like to see this happen!

@stale
Copy link

stale bot commented Aug 22, 2020

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Aug 22, 2020
@drcmda
Copy link

drcmda commented Aug 22, 2020

It's very much still affecting us

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Aug 22, 2020
@stale
Copy link

stale bot commented Dec 25, 2020

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Dec 25, 2020
@lavrton
Copy link

lavrton commented Dec 25, 2020

Sorry, bot. But many of us are still interested in the solution.

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Dec 25, 2020
RenaudRohlinger added a commit to RenaudRohlinger/reactfire that referenced this issue Nov 11, 2021
Hello! 
To consume the contexts with react reconcilier you need to forward the contexts using a "bridge".
See facebook/react#17275
Exposing each context is then necessary if we don't want to wrap all our components twice in that case.
@hansagames
Copy link

HI, anyone have found good way to fix this ? tried out solutions suggested here but no luck (got errors from react it self after doing that)

@Minious
Copy link

Minious commented Jan 29, 2024

Still having the issue trying to use next-intl NextIntlClientProvider inside the react-pdf.

Error: Failed to call 'useTranslations' because the context from 'NextIntlClientProvider' was not found.

@lavrton
Copy link

lavrton commented Jan 29, 2024

BTW, with https://github.com/pmndrs/its-fine, the issue can be resolved. All custom renderers may use it to automatically bridge all the contexts. react-konva and react-three-fiber are already doing that.

@drcmda
Copy link

drcmda commented Mar 8, 2024

https://twitter.com/sebmarkbage/status/1765881794648269239 😂

but maybe this project was silly enough for react to see that this is needed. why have secondary renderers when they can't participate in primary matters.

@rickhanlonii
Copy link
Member

Sharing the context from @sebmarkbage on #28524:

Spent some time investigating. Regardless the versions would have to be in perfect lock-step but that turns out to be a requirement anyway so that's fine.

The proper solution that behaves like a Portal should forward the value during a concurrent render into the child. Meaning it should actually be part of the same render phase. That's not just an issue with Context but Props too. Ideally it shouldn't have to commit the outer root before forwarding the inner values.

Additionally, it's not just Context that is contextual. There's internal contexts and for example if you Suspend in the child renderer outside a Suspense boundary, it should affect the parent render. E.g. a startTransition in the parent should stall or .

That's the proper implementation. It's very difficult, and may require compromising on performance for everyone, but it's not impossible. Realistically, that amount of work, I don't see that getting prioritized any time soon. There's just way more pressing issues.

The smaller version of converting an outer render into a sync secondary render after commit, with no Suspense or Transition integration etc. is easier. That might be "good enough". However, there's the question of whether encouraging thinking of these is a single root given that new features won't work seamlessly anyway. So it might've been "good enough" before but not "good enough" in the future.

There's also a whole other approach of creating fake DOM nodes using the DOM renderer.

@mathematikoi
Copy link

mathematikoi commented May 25, 2024

I managed to achieve this by writing a shim for react context to use events, you can check it out here:

https://github.com/mathematikoi/eventually.git

still wip, but you can see for yourself

https://codesandbox.io/p/sandbox/context-events-qnqr4f?file=%2Fsrc%2F100%25-event-driven.tsx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests