Uh oh!
There was an error while loading. Please reload this page.
Add a feature flag to disable legacy context - #16269
Merged
Merged
Conversation
ReactDOM: size: 0.0%, gzip: 🔺+0.1% Details of bundled changes.Comparing: 4279455...892df46 react-dom
react-art
react-test-renderer
react-reconciler
react-native-renderer
Generated by 🚫 dangerJS |
acdlite
approved these changes
Jul 31, 2019
acdlite
left a comment
Collaborator
There was a problem hiding this comment.
What does this.context point to when the flag is on? Or the second argument to function components?
I think they should be undefined, instead of an empty object — i.e. it should behave the same as if legacy context never existed, no vestiges.
| instance.__reactInternalMemoizedUnmaskedChildContext = unmaskedContext; | ||
| instance.__reactInternalMemoizedMaskedChildContext = maskedContext; | ||
| if (disableLegacyContext) { | ||
| invariant( |
Collaborator
There was a problem hiding this comment.
Can we make these dev-only warnings instead? Once everyone removes legacy context from their apps, we can remove the warning in a minor, whereas removing an invariant is an according-to-semver-pedants breaking change.
gaearon
commented
Aug 1, 2019
CollaboratorAuthor
Previously, classes that weren't context consumers would get |
- invariant -> warning - Make this.context and context argument actually undefined
acdlite
commented
Aug 1, 2019
Collaborator
Right, and then we don't have to do a second breaking change later |
CollaboratorAuthor
We could still remove the feature but keep it forever |
acdlite
commented
Aug 1, 2019
Collaborator
@gaearon It's the second argument to functions that I care more about, personally. We might want to use that for something else. Don't really care about classes. What's a class? 🤷♀️ |
mrizwanashiq pushed a commit
to mrizwanashiq/react
that referenced
this pull request
Jun 25, 2026
* Add a feature flag to disable legacy context
* Address review
- invariant -> warning
- Make this.context and context argument actually undefined
* Increase test coverage for lifecycles
* Also disable it on the server is flag is on
* Make this.context {} when disabled, but function context is undefined
* Move checks inside
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a feature flag that disables the legacy context API. The feature flag is turned off in open source builds. Strict Mode is already a mechanism that warns about it, but this feature flag actually makes
this.contextalways an empty object, andcontextargument to functions alwaysundefined. So it's a way to enforce it. If we later offer "modern" builds, we can turn it on there by default. For now, I want to use this internally for newly written code.No whitespace: https://github.com/facebook/react/compare/master...gaearon:ctx?expand=1&w=1