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

Separate strict effects mode feature flags #20847

Closed
wants to merge 4 commits into from

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Feb 19, 2021

PR Stack


Builds on top of PR #20844

The feature flag that was previously debugRenderPhaseSideEffectsForStrictMode has been split into two flags:

  • enableStrictEffects*: This enables the new strict effects mode.
  • createRootStrictEffectsByDefault: This determines whether createRoot (and createBlockingRoot) apps will default to strict effects mode (if true) or strict mode (if false).

For now, both flags are only enabled within Facebook for testing purposes.

I've renamed strict effects test files slightly to better reflect their purpose but there were no other substantial changes.

Next step in this stack of PRs will be to add support for a new StrictMode "level" attribute to toggle between these modes as well as a new createRoot option to change the default mode for the tree. This PR unblocks the latter.

External changes?

For now, this commit changes no public facing behavior. The only mechanism for opting into strict mode level 2 is the createRootStrictEffectsByDefault feature flag (only enabled within Facebook for now).

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Feb 19, 2021
@bvaughn bvaughn changed the title Split strict effects mode feature flags Separate strict effects mode feature flags Feb 19, 2021
@sizebot
Copy link

sizebot commented Feb 19, 2021

Comparing: 4190a34...a1d6475

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 122.36 kB 122.36 kB = 39.41 kB 39.41 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 128.94 kB 128.94 kB = 41.47 kB 41.47 kB
facebook-www/ReactDOM-prod.classic.js +0.09% 404.67 kB 405.03 kB +0.09% 75.15 kB 75.22 kB
facebook-www/ReactDOM-prod.modern.js +0.09% 393.02 kB 393.38 kB +0.10% 73.24 kB 73.31 kB
facebook-www/ReactDOMForked-prod.classic.js +0.09% 404.68 kB 405.04 kB +0.09% 75.15 kB 75.22 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
facebook-www/ReactIs-dev.modern.js +0.80% 9.87 kB 9.95 kB +0.67% 2.68 kB 2.70 kB
facebook-www/ReactIs-dev.classic.js +0.80% 9.87 kB 9.95 kB +0.67% 2.68 kB 2.70 kB
facebook-www/SchedulerTracing-dev.modern.js +0.71% 11.16 kB 11.24 kB +0.77% 2.46 kB 2.48 kB
facebook-www/SchedulerTracing-dev.classic.js +0.71% 11.16 kB 11.24 kB +0.77% 2.46 kB 2.48 kB
oss-experimental/react-debug-tools/cjs/react-debug-tools.development.js +0.48% 21.31 kB 21.41 kB +0.89% 5.84 kB 5.89 kB
oss-stable/react-debug-tools/cjs/react-debug-tools.development.js +0.48% 21.31 kB 21.41 kB +0.89% 5.84 kB 5.89 kB

Generated by 🚫 dangerJS against a1d6475

Copy link
Member

@rickhanlonii rickhanlonii left a comment

Choose a reason for hiding this comment

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

I like this strategy a lot.

Brian Vaughn added 4 commits February 24, 2021 14:54
…y subtrees into strict mode level one ('mode == StrictModeL1'). This mode enables DEV-only double rendering, double component lifecycles, string ref warnings, legacy context warnings, etc. The primary purpose of this mode is to help detected render phase side effects. No new behavior. Roots created with experimental 'createRoot' and 'createBlockingRoot' APIs will also (for now) continue to default to strict mode level 1.

In a subsequent commit I will add support for a 'level' attribute on the '<React.StrictMode>' tag (as well as a new option supported by ). This will be the way to opt into strict mode level 2 ('mode == StrictModeL2'). This mode will enable DEV-only double invoking of effects on initial mount. This will simulate future Offscreen API semantics for trees being mounted, then hidden, and then shown again. The primary purpose of this mode is to enable applications to prepare for compatibility with the new Offscreen API (more information to follow shortly).

For now, this commit changes no public facing behavior. The only mechanism for opting into strict mode level 2 is the pre-existing 'enableDoubleInvokingEffects' feature flag (only enabled within Facebook for now).
StrictModeL1 -> StrictLegacyMode and StrictModeL2 -> StrictEffectsMode
One flag ('enableStrictEffects') enables strict mode level 2. It is similar to 'debugRenderPhaseSideEffectsForStrictMode' which enables srtict mode level 1.

The second flag ('createRootStrictEffectsByDefault') controls the default strict mode level for 'createRoot' trees. For now, all 'createRoot' trees remain level 1 by default. We will experiment with level 2 within Facebook.

This is a prerequisite for adding a configurable option to 'createRoot' that enables choosing a different StrictMode level than the default.
@bvaughn bvaughn force-pushed the strict-mode-separate-flags branch from 2431a51 to a1d6475 Compare February 24, 2021 20:24
@bvaughn
Copy link
Contributor Author

bvaughn commented Feb 24, 2021

Merged via #20849

@bvaughn bvaughn closed this Feb 24, 2021
@bvaughn bvaughn deleted the strict-mode-separate-flags branch February 24, 2021 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants