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

Add a feature flag to disable legacy context #16269

Merged
merged 7 commits into from
Aug 2, 2019
Merged

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Jul 31, 2019

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.context always an empty object, and context argument to functions always undefined. 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

@sizebot
Copy link

sizebot commented Jul 31, 2019

ReactDOM: size: 0.0%, gzip: 🔺+0.1%

Details of bundled changes.

Comparing: 4279455...892df46

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.profiling.min.js 0.0% -0.0% 115.03 KB 115.03 KB 36.19 KB 36.19 KB NODE_PROFILING
react-dom-server.browser.development.js +1.1% +0.6% 137.04 KB 138.51 KB 36.22 KB 36.43 KB UMD_DEV
ReactDOM-dev.js +0.3% +0.2% 929.88 KB 932.35 KB 205.77 KB 206.09 KB FB_WWW_DEV
react-dom-server.browser.production.min.js 0.0% 🔺+0.1% 19.39 KB 19.39 KB 7.27 KB 7.27 KB UMD_PROD
react-dom-test-utils.development.js 0.0% -0.0% 55.72 KB 55.72 KB 14.95 KB 14.95 KB UMD_DEV
react-dom-unstable-fizz.browser.development.js 0.0% -0.1% 3.78 KB 3.78 KB 1.52 KB 1.52 KB UMD_DEV
react-dom-test-utils.production.min.js 0.0% 🔺+0.1% 11.19 KB 11.19 KB 4.11 KB 4.11 KB UMD_PROD
react-dom-unstable-fizz.browser.production.min.js 0.0% -0.3% 1.21 KB 1.21 KB 708 B 706 B UMD_PROD
react-dom-test-utils.development.js 0.0% -0.0% 53.99 KB 53.99 KB 14.62 KB 14.62 KB NODE_DEV
react-dom-unstable-fizz.browser.development.js 0.0% -0.1% 3.61 KB 3.61 KB 1.48 KB 1.48 KB NODE_DEV
react-dom-test-utils.production.min.js 0.0% 0.0% 10.97 KB 10.97 KB 4.04 KB 4.04 KB NODE_PROD
react-dom-unstable-fizz.browser.production.min.js 0.0% -0.2% 1.05 KB 1.05 KB 638 B 637 B NODE_PROD
react-dom.development.js +0.2% +0.1% 906.48 KB 908.65 KB 205.72 KB 205.96 KB UMD_DEV
react-dom.production.min.js 0.0% -0.0% 111.4 KB 111.4 KB 35.79 KB 35.78 KB UMD_PROD
react-dom.profiling.min.js 0.0% -0.0% 114.81 KB 114.81 KB 36.83 KB 36.83 KB UMD_PROFILING
react-dom.development.js +0.2% +0.1% 900.78 KB 902.95 KB 204.12 KB 204.37 KB NODE_DEV
react-dom-server.node.development.js +1.1% +0.6% 135.1 KB 136.57 KB 35.83 KB 36.04 KB NODE_DEV
react-dom-server.node.production.min.js 0.0% 🔺+0.1% 20.17 KB 20.17 KB 7.57 KB 7.57 KB NODE_PROD
ReactDOM-prod.js 🔺+0.3% 🔺+0.3% 370.55 KB 371.62 KB 68.12 KB 68.3 KB FB_WWW_PROD
ReactDOM-profiling.js +0.3% +0.3% 375.2 KB 376.27 KB 69.13 KB 69.32 KB FB_WWW_PROFILING
react-dom-server.browser.development.js +1.1% +0.6% 133.17 KB 134.64 KB 35.29 KB 35.5 KB NODE_DEV
react-dom-server.browser.production.min.js 0.0% 0.0% 19.32 KB 19.32 KB 7.26 KB 7.27 KB NODE_PROD
react-dom-unstable-native-dependencies.development.js 0.0% -0.0% 60.76 KB 60.76 KB 15.85 KB 15.85 KB UMD_DEV
ReactDOMServer-dev.js +1.3% +0.6% 135.2 KB 136.95 KB 34.68 KB 34.9 KB FB_WWW_DEV
ReactDOMServer-prod.js 🔺+0.6% 🔺+0.8% 46.38 KB 46.68 KB 10.75 KB 10.83 KB FB_WWW_PROD
react-dom-unstable-native-dependencies.development.js 0.0% -0.0% 60.43 KB 60.43 KB 15.72 KB 15.72 KB NODE_DEV
react-dom-unstable-fizz.node.development.js 0.0% -0.1% 3.85 KB 3.85 KB 1.5 KB 1.5 KB NODE_DEV
react-dom-unstable-fizz.node.production.min.js 0.0% -0.3% 1.1 KB 1.1 KB 669 B 667 B NODE_PROD

react-art

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-art.development.js +0.3% +0.3% 643.83 KB 645.99 KB 140.56 KB 140.93 KB UMD_DEV
react-art.production.min.js 0.0% -0.0% 102.3 KB 102.3 KB 31.21 KB 31.21 KB UMD_PROD
react-art.development.js +0.4% +0.2% 574.7 KB 576.87 KB 123.25 KB 123.5 KB NODE_DEV
react-art.production.min.js 0.0% -0.0% 67.34 KB 67.34 KB 20.53 KB 20.53 KB NODE_PROD
ReactART-dev.js +0.4% +0.2% 588.65 KB 591.12 KB 122.83 KB 123.13 KB FB_WWW_DEV
ReactART-prod.js 🔺+0.5% 🔺+0.5% 219.98 KB 221.04 KB 37.5 KB 37.7 KB FB_WWW_PROD

react-test-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactTestRenderer-dev.js +0.4% +0.2% 599.62 KB 602.07 KB 125.29 KB 125.57 KB FB_WWW_DEV
react-test-renderer-shallow.development.js 0.0% -0.0% 39.51 KB 39.51 KB 9.95 KB 9.95 KB UMD_DEV
react-test-renderer-shallow.production.min.js 0.0% -0.0% 11.66 KB 11.66 KB 3.56 KB 3.56 KB UMD_PROD
react-test-renderer-shallow.development.js 0.0% -0.0% 33.64 KB 33.64 KB 8.54 KB 8.53 KB NODE_DEV
react-test-renderer-shallow.production.min.js 0.0% -0.0% 11.81 KB 11.81 KB 3.69 KB 3.69 KB NODE_PROD
react-test-renderer.development.js +0.4% +0.2% 587.61 KB 589.77 KB 125.85 KB 126.09 KB UMD_DEV
react-test-renderer.production.min.js 0.0% -0.0% 68.85 KB 68.85 KB 21.05 KB 21.05 KB UMD_PROD
react-test-renderer.development.js +0.4% +0.2% 583.15 KB 585.31 KB 124.73 KB 125 KB NODE_DEV
react-test-renderer.production.min.js 0.0% -0.0% 68.59 KB 68.59 KB 20.85 KB 20.85 KB NODE_PROD

react-reconciler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-reconciler.development.js +0.4% +0.2% 572.79 KB 574.96 KB 121.81 KB 122.07 KB NODE_DEV
react-reconciler-reflection.development.js 0.0% -0.0% 16.63 KB 16.63 KB 5.16 KB 5.16 KB NODE_DEV
react-reconciler-reflection.production.min.js 0.0% -0.1% 2.59 KB 2.59 KB 1.14 KB 1.14 KB NODE_PROD
react-reconciler-persistent.development.js +0.4% +0.2% 569.8 KB 571.97 KB 120.55 KB 120.82 KB NODE_DEV
react-reconciler-persistent.production.min.js 0.0% -0.0% 68.58 KB 68.58 KB 20.38 KB 20.38 KB NODE_PROD

react-native-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactNativeRenderer-prod.js -0.0% -0.0% 271.97 KB 271.94 KB 46.65 KB 46.65 KB RN_OSS_PROD
ReactNativeRenderer-profiling.js -0.0% -0.0% 280.39 KB 280.36 KB 48.19 KB 48.18 KB RN_OSS_PROFILING
ReactFabric-prod.js -0.0% -0.0% 265.65 KB 265.62 KB 45.7 KB 45.69 KB RN_OSS_PROD
ReactFabric-profiling.js -0.0% -0.0% 274.31 KB 274.28 KB 47.16 KB 47.15 KB RN_OSS_PROFILING
ReactFabric-dev.js +0.3% +0.2% 733.51 KB 735.94 KB 155.19 KB 155.48 KB RN_FB_DEV
ReactFabric-prod.js -0.0% -0.0% 265.65 KB 265.62 KB 45.71 KB 45.7 KB RN_FB_PROD
ReactNativeRenderer-dev.js +0.3% +0.2% 722.67 KB 725.1 KB 153.1 KB 153.4 KB RN_OSS_DEV
ReactFabric-profiling.js -0.0% -0.0% 274.31 KB 274.28 KB 47.16 KB 47.16 KB RN_FB_PROFILING
ReactNativeRenderer-dev.js +0.3% +0.2% 722.76 KB 725.2 KB 153.15 KB 153.45 KB RN_FB_DEV
ReactNativeRenderer-prod.js -0.0% -0.0% 271.96 KB 271.93 KB 46.66 KB 46.66 KB RN_FB_PROD
ReactNativeRenderer-profiling.js -0.0% -0.0% 280.38 KB 280.35 KB 48.19 KB 48.18 KB RN_FB_PROFILING
ReactFabric-dev.js +0.3% +0.2% 733.41 KB 735.84 KB 155.14 KB 155.44 KB RN_OSS_DEV

Generated by 🚫 dangerJS

Copy link
Collaborator

@acdlite acdlite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Collaborator Author

gaearon commented Aug 1, 2019

Previously, classes that weren't context consumers would get {}. You're saying it should be undefined too. That's a bigger breaking change but maybe it makes sense because it lets you find code with bad assumptions more easily.

- invariant -> warning
- Make this.context and context argument actually undefined
@acdlite
Copy link
Collaborator

acdlite commented Aug 1, 2019

That's a bigger breaking change but maybe it makes sense because it lets you find code with bad assumptions more easily.

Right, and then we don't have to do a second breaking change later

@gaearon
Copy link
Collaborator Author

gaearon commented Aug 1, 2019

We could still remove the feature but keep it forever {}. Since it doesn't cost us anything to keep setting it once.

@acdlite
Copy link
Collaborator

acdlite commented Aug 1, 2019

@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? 🤷‍♀️

@gaearon gaearon merged commit 0c1ec04 into facebook:master Aug 2, 2019
@gaearon gaearon deleted the ctx branch August 2, 2019 00:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants