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 concept of StrictMode levels 1 & 2 (no public API yet) #20844

Closed
wants to merge 2 commits into from

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Feb 18, 2021

PR Stack


Strict mode level 0 (aka loose or sloppy mode)

This isn't really a mode so much as it's the absence of any strict modes.

Strict mode level 1 (aka legacy mode)

The exported <React.StrictMode> tag remains the same and opts legacy subtrees into strict mode level one (mode == StrictLegacyMode). 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.

Strict mode level 2 (aka strict effects mode)

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 createRoot). This will be the way to opt into strict mode level 2 (mode == StrictEffectsMode). This mode will enable DEV-only double invoking of effects on initial mount (see #19523). 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).

Strict mode level >= 3?

In the future we may add additional strict mode levels (TBD).

External changes?

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).

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Feb 18, 2021
@bvaughn bvaughn force-pushed the strict-mode-level-1-and-2 branch from e12cf07 to f0a5fac Compare February 18, 2021 21:30
@sizebot
Copy link

sizebot commented Feb 18, 2021

Comparing: 4190a34...372bb8b

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.06% 404.67 kB 404.90 kB +0.07% 75.15 kB 75.20 kB
facebook-www/ReactDOM-prod.modern.js +0.06% 393.02 kB 393.24 kB +0.08% 73.24 kB 73.30 kB
facebook-www/ReactDOMForked-prod.classic.js +0.06% 404.68 kB 404.91 kB +0.07% 75.15 kB 75.20 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
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 372bb8b

@dai-shi
Copy link
Contributor

dai-shi commented Feb 18, 2021

StrictMode level sounds nice.
I hope it would be possible to add useRef read warning on mutable values, at some level.
#18545 (comment)

@bvaughn
Copy link
Contributor Author

bvaughn commented Feb 18, 2021

Both of the new warnings you refer to are related to render phase side effects, so if anything they both belong in level 1. An argument could be made for warning about mutated refs even outside of strict mode.

@jacobp100
Copy link

Sorry if this is too off-topic - does this mean double invoking the functions passed to useEffect?

@eps1lon
Copy link
Collaborator

eps1lon commented Feb 19, 2021

Sorry if this is too off-topic - does this mean double invoking the functions passed to useEffect?

@jacobp100 For what "strict effects mode" is, check out the original PR:

TL;DR: It doesn't just double invoke the functions passed to useEffect (create) but also runs their cleanup logic inbetween:

This PR changes the behavior in DEV when an effect runs from create() to create() -> destroy() -> create()

--#19523

@bvaughn bvaughn changed the title Add StrictMode levels 1 & 2 Add concept of StrictMode levels 1 & 2 (no public API yet) Feb 19, 2021
@bvaughn bvaughn force-pushed the strict-mode-level-1-and-2 branch from e52ca35 to dd40e13 Compare February 19, 2021 16:33
@bvaughn
Copy link
Contributor Author

bvaughn commented Feb 19, 2021

Circle CI doing something weird here I think. Hm. Should sort itself out next time I rebase onto master.

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 still wonder if we can wait on this until we have more information but happy to move forward if others want to approve and land.

@bvaughn bvaughn force-pushed the strict-mode-level-1-and-2 branch from dd40e13 to 11b8797 Compare February 22, 2021 20:09
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.

Looks great, these changes are really exciting 🎉

@@ -46,7 +46,7 @@ import {
} from './ReactFiber.new';
import {emptyRefsObject} from './ReactFiberClassComponent.new';
import {isCompatibleFamilyForHotReloading} from './ReactFiberHotReloading.new';
import {StrictMode} from './ReactTypeOfMode';
import {StrictLegacyMode} from './ReactTypeOfMode';
Copy link
Member

Choose a reason for hiding this comment

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

Chatted offline and we can follow up with this if we agree - we may want to name this something other than "legacy" since the term overloaded to mean "legacy root", "legacy strict mode" and any other "legacys" we have. I was thinking something like:

  • NoMode = 0
  • StrictWarningMode = Level 1
  • StrictRenderMode = Level 2
  • StrictEffectMode = Level 3

Brian Vaughn added 2 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
@bvaughn bvaughn force-pushed the strict-mode-level-1-and-2 branch from 11b8797 to 372bb8b 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-level-1-and-2 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.

8 participants