-
Notifications
You must be signed in to change notification settings - Fork 47.6k
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
Unify context implementations #13140
Conversation
Currently, context can only be read by a special type of component, ContextConsumer. We want to add support to all fibers, including classes and functional components. Each fiber may read from one or more contexts. To enable quick, mono- morphic access of this list, we'll store them on a fiber property.
unstable_read can be called anywhere within the render phase. That includes the render method, getDerivedStateFromProps, constructors, functional components, and context consumer render props. If it's called outside the render phase, an error is thrown.
React: size: 🔺+2.4%, gzip: 🔺+1.7% ReactDOM: size: 🔺+0.8%, gzip: 🔺+0.7% Details of bundled changes.Comparing: 6731bfb...eb84045 react
react-dom
react-art
react-test-renderer
react-reconciler
react-native-renderer
Generated by 🚫 dangerJS |
fe64cbd
to
fe9e602
Compare
Implements the legacy, string-based context API on top of the new context API. This doesn't save much in code size because most of the legacy stuff deals with merging and masking. Arguably, it's a conceptual complexity win. I'm ambivalent about whether we land this.
Doing this in a separate commit so the diff is less confusing
Would this break unstable_renderSubtreeIntoContainer (#12493)? |
@sophiebits 🤔I was commenting on the wrong issue in the morning. I had the very same question here: #13139 (comment) (just want to link it for completeness) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we'll want to do this if it is a breaking change. More likely, we'd just deprecate the legacy context.
To clarify, it’s not a breaking change. In my comment above I meant that we don’t plan to make |
But I’d also prefer to just deprecate legacy context :D |
Not worth it |
Based on #13139
Implements the legacy, string-based context API on top of the new context API. This doesn't save much in code size because most of the legacy stuff deals with merging and masking. Arguably, it's a conceptual complexity win.
I'm ambivalent about whether we land this.