Add consoleManagedByDevToolsDuringStrictMode feature flag in React Reconciler #22196

lunaruan · 2021-08-27T19:55:12Z

Right now we don't have support to read the React DevTools settings before initial render on React Native. This PR adds a React feature flag to always disable double logging console logs on React Native until React Native adds support for synchronously reading settings in the future

sizebot · 2021-08-27T19:59:44Z

Comparing: 8723e77...260ab26

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	=	127.60 kB	127.60 kB	=	40.73 kB	40.73 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	130.42 kB	130.42 kB	=	41.66 kB	41.66 kB
facebook-www/ReactDOM-prod.classic.js	=	405.18 kB	405.18 kB	=	75.05 kB	75.05 kB
facebook-www/ReactDOM-prod.modern.js	=	393.75 kB	393.75 kB	=	73.33 kB	73.33 kB
facebook-www/ReactDOMForked-prod.classic.js	=	405.18 kB	405.18 kB	=	75.05 kB	75.05 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
react-native/implementations/ReactFabric-dev.fb.js	+0.25%	718.45 kB	720.23 kB	+0.28%	155.24 kB	155.68 kB
react-native/implementations/ReactNativeRenderer-dev.fb.js	+0.24%	733.59 kB	735.36 kB	+0.26%	158.64 kB	159.06 kB

Generated by 🚫 dangerJS against 260ab26

bvaughn

I think we should also add a DevTools feature flag to turn off the new setting for standalone build (until React Native supports it)? Otherwise we'll show an option in DevTools that won't make any sense?

bvaughn · 2021-08-30T19:56:03Z

packages/shared/ReactFeatureFlags.js

@@ -175,3 +175,5 @@ export const enableSyncDefaultUpdates = true;
 export const allowConcurrentByDefault = false;

 export const enablePersistentOffscreenHostContainer = false;
+
+export const enableConsoleLogsInDoubleRender = false;


nit Thinking out loud...I wonder if we could pick a name that also mentions DevTools here?

Like consoleManagedByDevToolsDuringStrictMode or something?

I dunno maybe that sucks 😂 I don't feel strongly about this. Feel free to ignore.

lunaruan · 2021-08-30T20:31:12Z

I think we should also add a DevTools feature flag to turn off the new setting for standalone build (until React Native supports it)? Otherwise we'll show an option in DevTools that won't make any sense?

Yep! I have a feature flag PR ready that I'll put up after the RN builds PR lands!

jstejada · 2021-08-31T19:53:22Z

@lunaruan so this pr is for adding it to the reconciler, and the one that already landed (#22215) is for adding it in devtools? is this one good to land now?

lunaruan · 2021-08-31T20:14:21Z

@lunaruan so this pr is for adding it to the reconciler, and the one that already landed (#22215) is for adding it in devtools? is this one good to land now?

yup! Should be pending review!

bvaughn · 2021-08-31T20:24:42Z

yup! Should be pending review!

That one (#22215) has been approved, right?

lunaruan · 2021-08-31T23:12:48Z

Yeah I already landed that one. Waiting for this one to be approved so I can land it too

gaearon · 2021-09-06T18:34:52Z

I feel like I don't fully follow what the flag represents and in which case it's supposed to be on/off.

It's currently on for WWW (FB), for Test Renderer (open source and FB). It's currently off for web React (open source), and for RN (open source and FB).

What is the pattern between these? I originally thought it's for RN only (per PR description). But it's off for RN. Then I thought it's meant to be enabled everywhere except RN. But it's also off for open source web React (not RN). So I'm not sure.

gaearon · 2021-09-06T18:45:51Z

Sent #22253 (review) with what trips me up

bvaughn · 2021-09-06T21:00:43Z

It's currently off for web React (open source)

This was a mistake/oversight.

It's currently off...for RN (open source and FB).

This was intentional, as DevTools has no way to intercept console logs during initial render / mount in React Native.

Summary: This sync includes the following changes: - **[95d762e40](facebook/react@95d762e40 )**: Remove duplicate test //<Andrew Clark>// - **[d4d1dc085](facebook/react@d4d1dc085 )**: Reorder VARIANT feature flags ([#22266](facebook/react#22266)) //<Dan Abramov>// - **[2f156eafb](facebook/react@2f156eafb )**: Adjust consoleManagedByDevToolsDuringStrictMode feature flag ([#22253](facebook/react#22253)) //<Dan Abramov>// - **[cfd819332](facebook/react@cfd819332 )**: Add useSyncExternalStore to react-debug-tools ([#22240](facebook/react#22240)) //<Andrew Clark>// - **[8e80592a3](facebook/react@8e80592a3 )**: Remove state queue from useSyncExternalStore ([#22265](facebook/react#22265)) //<Andrew Clark>// - **[06f98c168](facebook/react@06f98c168 )**: Implement useSyncExternalStore in Fiber ([#22239](facebook/react#22239)) //<Andrew Clark>// - **[77912d9a0](facebook/react@77912d9a0 )**: Wire up the native API for useSyncExternalStore ([#22237](facebook/react#22237)) //<Andrew Clark>// - **[031abd24b](facebook/react@031abd24b )**: Add warning and test for useSyncExternalStore when getSnapshot isn't cached ([#22262](facebook/react#22262)) //<salazarm>// - **[b8884de24](facebook/react@b8884de24 )**: break up import keyword to avoid being accidentally parsed as dynamic import statement in external code ([#21918](facebook/react#21918)) //<Jianhua Zheng>// - **[6d6bba5bf](facebook/react@6d6bba5bf )**: Fix typo in ReactUpdatePriority-test.js ([#21958](facebook/react#21958)) //<Ikko Ashimine>// - **[0c0d1ddae](facebook/react@0c0d1ddae )**: feat(eslint-plugin-react-hooks): support ESLint 8.x ([#22248](facebook/react#22248)) //<Michaël De Boey>// - **[1314299c7](facebook/react@1314299c7 )**: Initial shim of useSyncExternalStore ([#22211](facebook/react#22211)) //<Andrew Clark>// - **[fc40f02ad](facebook/react@fc40f02ad )**: Add consoleManagedByDevToolsDuringStrictMode feature flag in React Reconciler ([#22196](facebook/react#22196)) //<Luna Ruan>// - **[46a0f050a](facebook/react@46a0f050a )**: Set up use-sync-external-store package ([#22202](facebook/react#22202)) //<Andrew Clark>// - **[8723e772b](facebook/react@8723e772b )**: Fix a string interpolation typo in ReactHooks test ([#22174](facebook/react#22174)) //<Matt Hargett>// - **[60a30cf32](facebook/react@60a30cf32 )**: Console Logging for StrictMode Double Rendering ([#22030](facebook/react#22030)) //<Luna Ruan>// - **[76bbad3e3](facebook/react@76bbad3e3 )**: Add maxYieldMs feature flag in Scheduler ([#22165](facebook/react#22165)) //<Ricky>// - **[b0b53ae2c](facebook/react@b0b53ae2c )**: Add feature flags for scheduler experiments ([#22105](facebook/react#22105)) //<Ricky>// Changelog: [General][Changed] - React Native sync for revisions bd5bf55...95d762e jest_e2e[run_all_tests] Reviewed By: mdvacca Differential Revision: D30809906 fbshipit-source-id: 131cfdf91e15f67fa59a5d925467e538ee89fe10

…conciler (facebook#22196)

lunaruan requested review from bvaughn and jstejada August 27, 2021 19:55

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Aug 27, 2021

added enableConsoleLogsInDoubleRender

21a345a

lunaruan force-pushed the feature_flags branch from 10e15af to 21a345a Compare August 27, 2021 22:39

bvaughn reviewed Aug 30, 2021

View reviewed changes

bvaughn added the Component: Developer Tools label Aug 30, 2021

fixed strict mode tests

260ab26

lunaruan force-pushed the feature_flags branch from 657b6d0 to 260ab26 Compare August 30, 2021 20:36

lunaruan changed the title ~~added enableConsoleLogsInDoubleRender feature flag~~ Add consoleManagedByDevToolsDuringStrictMode feature flag in React Reconciler Aug 30, 2021

bvaughn approved these changes Aug 31, 2021

View reviewed changes

lunaruan merged commit fc40f02 into facebook:main Sep 1, 2021

gaearon mentioned this pull request Sep 6, 2021

Adjust consoleManagedByDevToolsDuringStrictMode feature flag #22253

Merged

zhengjitf pushed a commit to zhengjitf/react that referenced this pull request Apr 15, 2022

Add consoleManagedByDevToolsDuringStrictMode feature flag in React Re…

1393dd9

…conciler (facebook#22196)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add consoleManagedByDevToolsDuringStrictMode feature flag in React Reconciler #22196

Add consoleManagedByDevToolsDuringStrictMode feature flag in React Reconciler #22196

lunaruan commented Aug 27, 2021

sizebot commented Aug 27, 2021 •

edited

Loading

bvaughn left a comment

bvaughn Aug 30, 2021

lunaruan commented Aug 30, 2021

jstejada commented Aug 31, 2021 •

edited

Loading

lunaruan commented Aug 31, 2021

bvaughn commented Aug 31, 2021 •

edited

Loading

lunaruan commented Aug 31, 2021

gaearon commented Sep 6, 2021 •

edited

Loading

gaearon commented Sep 6, 2021

bvaughn commented Sep 6, 2021

Add consoleManagedByDevToolsDuringStrictMode feature flag in React Reconciler #22196

Add consoleManagedByDevToolsDuringStrictMode feature flag in React Reconciler #22196

Conversation

lunaruan commented Aug 27, 2021

sizebot commented Aug 27, 2021 • edited Loading

Critical size changes

Significant size changes

bvaughn left a comment

Choose a reason for hiding this comment

bvaughn Aug 30, 2021

Choose a reason for hiding this comment

lunaruan commented Aug 30, 2021

jstejada commented Aug 31, 2021 • edited Loading

lunaruan commented Aug 31, 2021

bvaughn commented Aug 31, 2021 • edited Loading

lunaruan commented Aug 31, 2021

gaearon commented Sep 6, 2021 • edited Loading

gaearon commented Sep 6, 2021

bvaughn commented Sep 6, 2021

sizebot commented Aug 27, 2021 •

edited

Loading

jstejada commented Aug 31, 2021 •

edited

Loading

bvaughn commented Aug 31, 2021 •

edited

Loading

gaearon commented Sep 6, 2021 •

edited

Loading