Remove createRootStrictEffectsByDefault flag #28102

rickhanlonii · 2024-01-26T04:59:34Z

There's no need to separate strict mode from strict effects mode any more.

I didn't clean up the StrictEffectMode fiber flag, because it's used to prevent strict effects in legacy mode. I could replace those checks with LegacyMode checks, but when we remove legacy mode, we can remove that flag and condense them into one StrictMode flag away.

rickhanlonii · 2024-01-26T05:00:54Z

scripts/jest/TestFlags.js

@@ -50,9 +50,6 @@ const environmentFlags = {
  FIXME: false,
  TODO: false,

-  // Turn these flags back on (or delete) once the effect list is removed in
-  // favor of a depth-first traversal using `subtreeTags`.
-  dfsEffectsRefactor: true,


Also should be able to remove this, which is only used for strict effects tests.

react-sizebot · 2024-01-26T05:03:15Z

Comparing: c7e735f...2a23aeb

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	=	176.34 kB	176.34 kB	=	54.93 kB	54.93 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	178.44 kB	178.44 kB	=	55.57 kB	55.57 kB
facebook-www/ReactDOM-prod.classic.js	=	572.79 kB	572.79 kB	=	100.80 kB	100.80 kB
facebook-www/ReactDOM-prod.modern.js	=	556.55 kB	556.55 kB	=	97.88 kB	97.88 kB
test_utils/ReactAllWarnings.js	Deleted	67.55 kB	0.00 kB	Deleted	16.54 kB	0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
test_utils/ReactAllWarnings.js	Deleted	67.55 kB	0.00 kB	Deleted	16.54 kB	0.00 kB

Generated by 🚫 dangerJS against 2a23aeb

rickhanlonii · 2024-01-26T05:27:36Z

packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js

-    } else {
-      expect(ref.current).toBe(span2);
-    }
+    expect(ref.current).toBe(null);


TestFlags hard coded this to true, but it was otherwise off everywhere (the flag doesn't even exist), so this is the current behavior.

There's no need to separate strict mode from strict effects mode any more. I didn't clean up the `StrictEffectMode` fiber flag, because it's used to prevent strict effects in legacy mode. I could replace those checks with `LegacyMode` checks, but when we remove legacy mode, we can remove that flag and condense them into one StrictMode flag away. DiffTrain build for [3d1da1f](3d1da1f)

There's no need to separate strict mode from strict effects mode any more. I didn't clean up the `StrictEffectMode` fiber flag, because it's used to prevent strict effects in legacy mode. I could replace those checks with `LegacyMode` checks, but when we remove legacy mode, we can remove that flag and condense them into one StrictMode flag away.

There's no need to separate strict mode from strict effects mode any more. I didn't clean up the `StrictEffectMode` fiber flag, because it's used to prevent strict effects in legacy mode. I could replace those checks with `LegacyMode` checks, but when we remove legacy mode, we can remove that flag and condense them into one StrictMode flag away.

There's no need to separate strict mode from strict effects mode any more. I didn't clean up the `StrictEffectMode` fiber flag, because it's used to prevent strict effects in legacy mode. I could replace those checks with `LegacyMode` checks, but when we remove legacy mode, we can remove that flag and condense them into one StrictMode flag away. DiffTrain build for commit 3d1da1f.

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jan 26, 2024

rickhanlonii commented Jan 26, 2024

View reviewed changes

Remove createRootStrictEffectsByDefault flag

6cd95b9

rickhanlonii force-pushed the rh/rm-effect-flag branch from b301095 to 6cd95b9 Compare January 26, 2024 05:26

rickhanlonii commented Jan 26, 2024

View reviewed changes

rickhanlonii mentioned this pull request Jan 26, 2024

[test] strict effect tests should fail #28104

Closed

Fix lint

2a23aeb

rickhanlonii requested a review from jackpope January 29, 2024 21:34

noahlemen approved these changes Feb 1, 2024

View reviewed changes

rickhanlonii merged commit 3d1da1f into facebook:main Feb 1, 2024
36 checks passed

rickhanlonii deleted the rh/rm-effect-flag branch February 1, 2024 19:54

gnoff mentioned this pull request Feb 5, 2024

Update React from 60a927d04 to 2bc7d336a vercel/next.js#61522

Merged

ztanner mentioned this pull request Mar 19, 2024

(backport) Update React from 60a927d04 to 14898b6a9 vercel/next.js#63442

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove createRootStrictEffectsByDefault flag #28102

Remove createRootStrictEffectsByDefault flag #28102

rickhanlonii commented Jan 26, 2024

rickhanlonii Jan 26, 2024

react-sizebot commented Jan 26, 2024 •

edited

Loading

rickhanlonii Jan 26, 2024

Remove createRootStrictEffectsByDefault flag #28102

Remove createRootStrictEffectsByDefault flag #28102

Conversation

rickhanlonii commented Jan 26, 2024

rickhanlonii Jan 26, 2024

Choose a reason for hiding this comment

react-sizebot commented Jan 26, 2024 • edited Loading

Critical size changes

Significant size changes

rickhanlonii Jan 26, 2024

Choose a reason for hiding this comment

react-sizebot commented Jan 26, 2024 •

edited

Loading