Don't warn about concurrently rendering contexts if we finished rendering by eps1lon · Pull Request #22797 · react/react

eps1lon · 2021-11-20T12:48:51Z

Summary

How did you test this change?

yarn test ReactFiberNewContext
CI
~~[ ] codesandbox of linked issue~~ Can't test codesandbox since react-test-renderer isn't built by Codesandbox CI. Though I tested it locally by adding context._currentRenderer2 = null; to popProvider in react-test-renderer.development.js.

eps1lon · 2021-11-20T12:50:09Z

+      } else {
+        ReactNoop.render(<App value={1} />);
+      }
+      expect(Scheduler).toFlushAndYield(['Foo', 'Foo']);


The test above ("warns if multiple renderers concurrently render the same context") did not commit here. Without committing I would expect the warning. But here we actually commit so I don't think we should warn.

sizebot · 2021-11-20T12:53:15Z

Comparing: 4cd788a...72bf322

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	=	131.79 kB	131.78 kB	=	42.40 kB	42.40 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	137.06 kB	137.06 kB	=	44.00 kB	43.99 kB
facebook-www/ReactDOM-prod.classic.js	=	457.17 kB	457.17 kB	=	83.22 kB	83.22 kB
facebook-www/ReactDOM-prod.modern.js	=	442.41 kB	442.41 kB	=	80.94 kB	80.94 kB
facebook-www/ReactDOMForked-prod.classic.js	=	457.94 kB	457.94 kB	=	83.33 kB	83.33 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 72bf322

eps1lon · 2021-11-20T15:26:11Z

  if (isPrimaryRenderer) {
    context._currentValue = currentValue;
+    if (__DEV__) {
+      context._currentRenderer = null;


What I find odd is that ReactFizzNewContext does handle the _currentRenderer in popProvider but does not reset it but set it it to rendererSigil i.e. push and pop do the same things to _currentRenderer:

https://github.com/facebook/react/blob/a724a3b578dce77d427bef313102a4d0e978d9b4/packages/react-server/src/ReactFizzNewContext.js#L273

Maybe we should actually restore _currentRenderer and not just reset it?

Maybe we should actually restore _currentRenderer and not just reset it?

Implemented in 04c19a8 (#22797)

eps1lon · 2022-03-05T16:46:52Z

  providerFiber: Fiber,
 ): void {
  const currentValue = valueCursor.current;
-  pop(valueCursor, providerFiber);


Moved down since we push(value) -> push(rendererN) so we need to pop(rendererN) -> pop(value)

gnoff

This seems correct. There is an equivalent implementation for ReactFizzNewContext which probably should get the same treatment

…ring (#22797) Closes #22796 DiffTrain build for [555ece0](555ece0) [View git log for this commit](https://github.com/facebook/react/commits/555ece0cd14779abd5a1fc50f71625f9ada42bef)

Summary: This sync includes the following changes: - **[48b687f](react/react@48b687fc9 )**: [trusted types][www] Add enableTrustedTypesIntegration flag back in ([#26016](react/react#26016)) //<an onion>// - **[9b1423c](react/react@9b1423cc0 )**: Revert "Hold host functions in var" ([#26079](react/react#26079)) //<Samuel Susla>// - **[ce09ace](react/react@ce09ace9a )**: Improve Error Messages when Access Client References ([#26059](react/react#26059)) //<Sebastian Markbåge>// - **[0652bdb](react/react@0652bdbd1 )**: Add flow types to Maps in ReactNativeViewConfigRegistry.js ([#26064](react/react#26064)) //<Samuel Susla>// - **[ee85098](react/react@ee8509801 )**: [cleanup] remove deletedTreeCleanUpLevel feature flag ([#25529](react/react#25529)) //<Jan Kassens>// - **[0e31dd0](react/react@0e31dd028 )**: Remove findDOMNode www shim ([#25998](react/react#25998)) //<Jan Kassens>// - **[379dd74](react/react@379dd741e )**: [www] set enableTrustedTypesIntegration to false ([#25997](react/react#25997)) //<Jan Kassens>// - **[555ece0](react/react@555ece0cd )**: Don't warn about concurrently rendering contexts if we finished rendering ([#22797](react/react#22797)) //<Sebastian Silbermann>// - **[0fce6bb](react/react@0fce6bb49 )**: [cleanup] remove feature flags warnAboutDefaultPropsOnFunctionComponents and warnAboutStringRefs ([#25980](react/react#25980)) //<Jan Kassens>// - **[7002a67](react/react@7002a6743 )**: [cleanup] remove unused values from ReactFeatureFlags.www-dynamic ([#25575](react/react#25575)) //<Jan Kassens>// - **[a48e54f](react/react@a48e54f2b )**: [cleanup] remove old feature flag warnAboutDeprecatedLifecycles ([#25978](react/react#25978)) //<Jan Kassens>// - **[0f4a835](react/react@0f4a83596 )**: Remove duplicate JSResourceReferenceImpl mock ([#25976](react/react#25976)) //<Jan Kassens>// - **[c491316](react/react@c49131669 )**: Remove unused Flow suppressions ([#25977](react/react#25977)) //<Jan Kassens>// - **[afe6521](react/react@afe6521e1 )**: Refactor: remove useless parameter ([#25923](react/react#25923)) //<Chris>// - **[34464fb](react/react@34464fb16 )**: Upgrade to Flow 0.196.3 ([#25974](react/react#25974)) //<Jan Kassens>// - **[e2424f3](react/react@e2424f33b )**: [flow] enable exact_empty_objects ([#25973](react/react#25973)) //<Jan Kassens>// - **[0b4f443](react/react@0b4f44302 )**: [flow] enable enforce_local_inference_annotations ([#25921](react/react#25921)) //<Jan Kassens>// - **[0b97441](react/react@0b974418c )**: [Fizz] Fork Fizz instruction set for inline script and external runtime ([#25862](react/react#25862)) //<mofeiZ>// - **[5379b61](react/react@5379b6123 )**: Batch sync, default and continuous lanes ([#25700](react/react#25700)) //<Tianyu Yao>// - **[bbf4d22](react/react@bbf4d2211 )**: Update import for babel-code-frame in build script ([#25963](react/react#25963)) //<Ming Ye>// - **[b83baf6](react/react@b83baf63f )**: Transform updates to support Flow this annotation syntax ([#25918](react/react#25918)) //<Jan Kassens>// - **[c2d6552](react/react@c2d655207 )**: Unify `use` and `renderDidSuspendDelayIfPossible` implementations ([#25922](react/react#25922)) //<Andrew Clark>// - **[48274a4](react/react@48274a43a )**: Remove vestigial Suspense batching logic ([#25861](react/react#25861)) //<Andrew Clark>// - **[de7d1c9](react/react@de7d1c907 )**: Add `fetchPriority` to `<img>` and `<link>` ([#25927](react/react#25927)) //<Steven>// - **[81d4ee9](react/react@81d4ee9ca )**: reconciler docs: fix small typo - "mode" (instead of "node") ([#25863](react/react#25863)) //<satelllte>// - **[5fcf1a4](react/react@5fcf1a4b4 )**: Bugfix: Synchronous ping during render phase sometimes unwinds the stack, leading to crash ([#25851](react/react#25851)) //<Andrew Clark>// - **[2b1fb91](react/react@2b1fb91a5 )**: ESLint upgrade to use hermes-eslint ([#25915](react/react#25915)) //<Jan Kassens>// - **[fabef7a](react/react@fabef7a6b )**: Resubmit Add HydrationSyncLane ([#25878](react/react#25878)) //<Tianyu Yao>// - **[7efa9e5](react/react@7efa9e597 )**: Fix unwinding context during selective hydration ([#25876](react/react#25876)) //<Tianyu Yao>// - **[84a0a17](react/react@84a0a171e )**: Rename experimental useEvent to useEffectEvent ([#25881](react/react#25881)) //<Sebastian Markbåge>// - **[4dda96a](react/react@4dda96a40 )**: [react-www] remove forked bundle ([#25866](react/react#25866)) //<Jan Kassens>// - **[9c09c1c](react/react@9c09c1cd6 )**: Revert "Fork ReactDOMSharedInternals for www ([#25791](react/react#25791))" ([#25864](react/react#25864)) //<lauren>// - **[996e4c0](react/react@996e4c0d5 )**: Offscreen add attach ([#25603](react/react#25603)) //<Samuel Susla>// - **[b14d7fa](react/react@b14d7fa4b )**: Add support for setNativeProps to Fabric ([#25737](react/react#25737)) //<Samuel Susla>// - **[8196872](react/react@819687279 )**: [Float] Fix typo in ReactDOMResourceValidation.js ([#25798](react/react#25798)) //<Ikko Ashimine>// - **[5dfc485](react/react@5dfc485f6 )**: fix tests for when float is off ([#25839](react/react#25839)) //<Josh Story>// - **[bfcbf33](react/react@bfcbf3306 )**: toString children of title ([#25838](react/react#25838)) //<Sebastian Markbåge>// - **[d4bc16a](react/react@d4bc16a7d )**: Revert "[react-www] remove forked bundle" ([#25837](react/react#25837)) //<Ricky>// - **[d69b2cf](react/react@d69b2cf82 )**: [bug fix] revert values in ReactFiberFlags to keep consistency for devtools ([#25832](react/react#25832)) //<Mengdi Chen>// - **[645ae26](react/react@645ae2686 )**: [react-www] remove forked bundle ([#25831](react/react#25831)) //<Jan Kassens>// - **[d807eb5](react/react@d807eb52c )**: Revert recent hydration changes ([#25812](react/react#25812)) //<Andrew Clark>// - **[2ccfa65](react/react@2ccfa657d )**: Fork ReactDOMSharedInternals for www ([#25791](react/react#25791)) //<lauren>// - **[f0534ae](react/react@f0534ae94 )**: Avoid replaying SelectiveHydrationException in dev ([#25754](react/react#25754)) //<Tianyu Yao>// - **[7fab379](react/react@7fab379d8 )**: fix link to ReactDOMHostconfig in reconciler docs ([#25788](react/react#25788)) //<Dmitry>// - **[500c8aa](react/react@500c8aa08 )**: Add component name to StrictMode error message ([#25718](react/react#25718)) //<Samuel Susla>// - **[353c302](react/react@353c30252 )**: Hold host functions in var ([#25741](react/react#25741)) //<Samuel Susla>// Changelog: [General][Changed] - React Native sync for revisions 17f6912...48b687f jest_e2e[run_all_tests] Reviewed By: rubennorte Differential Revision: D42855483 fbshipit-source-id: c244a595bb2d490a23b333c1b16d04a459ec94fc

iammerrick · 2024-04-29T21:26:13Z

I was expecting this to land in 18.3 or 19 but I'm still getting the warnings... Is that expected?

vladinator1000 · 2024-05-21T14:07:52Z

Same here, here's a reproduction repo https://stackoverflow.com/q/78512399/4957288

Also, hii @iammerrick 😱 ❤️ Long time no see, how are you??? Never expected React warnings to be bringing people together like that 😂

eps1lon · 2024-05-21T14:17:19Z

18.3 only included deprecation warnings and no other changes such as this. It will be part of React 19 though.

facebook-github-bot added the CLA Signed label Nov 20, 2021

eps1lon commented Nov 20, 2021

View reviewed changes

eps1lon force-pushed the fix/concurrent-context-render branch 3 times, most recently from bdf6797 to e9bd500 Compare November 20, 2021 13:00

eps1lon marked this pull request as ready for review November 20, 2021 13:11

eps1lon commented Nov 20, 2021

View reviewed changes

eps1lon force-pushed the fix/concurrent-context-render branch 2 times, most recently from 294a975 to 04c19a8 Compare March 5, 2022 16:40

eps1lon commented Mar 5, 2022

View reviewed changes

eps1lon changed the title ~~Don't warn about concurrent provider renders if we finished rendering~~ Don't warn about concurrently rendering contexts if we finished rendering Mar 5, 2022

eps1lon force-pushed the fix/concurrent-context-render branch from 04c19a8 to 42f964f Compare March 19, 2022 09:46

eps1lon mentioned this pull request May 11, 2022

Match Preact behavior for boolean props on custom elements #24541

Merged

3 tasks

eps1lon force-pushed the fix/concurrent-context-render branch from 42f964f to c9bf660 Compare May 12, 2022 06:26

eps1lon force-pushed the fix/concurrent-context-render branch from c9bf660 to 72bf322 Compare July 2, 2022 07:20

eps1lon added 4 commits January 11, 2023 08:59

Expected behavior

67b5c96

Don't warn about concurrent provider renders if we finished rendering

bb33098

Only need to reset in DEV since we only set in DEV

91b9705

Restore currentRenderN instead of resetting it

2ab9b80

eps1lon force-pushed the fix/concurrent-context-render branch from 72bf322 to 2ab9b80 Compare January 11, 2023 08:01

eps1lon commented Jan 11, 2023

View reviewed changes

Comment thread packages/react-reconciler/src/__tests__/ReactNewContext-test.js Outdated

Update packages/react-reconciler/src/__tests__/ReactNewContext-test.js

4e9a32e

gnoff approved these changes Jan 11, 2023

View reviewed changes

eps1lon merged commit 555ece0 into react:main Jan 12, 2023

eps1lon deleted the fix/concurrent-context-render branch January 12, 2023 12:17

Haroenv mentioned this pull request Jan 12, 2023

WARNING - Detected multiple renderers concurrently rendering the same context provider (React 18.0.0?) algolia/instantsearch#5243

Closed

sammy-SC mentioned this pull request Apr 5, 2023

Use native scheduler if defined sammy-SC/react#1

Closed

fossabot Bot mentioned this pull request Oct 2, 2025

Bump react and @types/react in /web-app franchb/console#14

Open

fossabot Bot mentioned this pull request Oct 20, 2025

chore(deps): bump react and @types/react blinklabs-io/after-town-hall#281

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't warn about concurrently rendering contexts if we finished rendering#22797

Don't warn about concurrently rendering contexts if we finished rendering#22797
eps1lon merged 5 commits into
react:mainfrom
eps1lon:fix/concurrent-context-render

eps1lon commented Nov 20, 2021 •

edited

Loading

Uh oh!

eps1lon Nov 20, 2021

Uh oh!

sizebot commented Nov 20, 2021 •

edited

Loading

Uh oh!

eps1lon Nov 20, 2021

Uh oh!

eps1lon Mar 5, 2022

Uh oh!

eps1lon Mar 5, 2022

Uh oh!

Uh oh!

gnoff left a comment

Uh oh!

iammerrick commented Apr 29, 2024

Uh oh!

vladinator1000 commented May 21, 2024 •

edited

Loading

Uh oh!

eps1lon commented May 21, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

eps1lon commented Nov 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

How did you test this change?

Uh oh!

eps1lon Nov 20, 2021

Choose a reason for hiding this comment

Uh oh!

sizebot commented Nov 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Critical size changes

Significant size changes

Uh oh!

eps1lon Nov 20, 2021

Choose a reason for hiding this comment

Uh oh!

eps1lon Mar 5, 2022

Choose a reason for hiding this comment

Uh oh!

eps1lon Mar 5, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gnoff left a comment

Choose a reason for hiding this comment

Uh oh!

iammerrick commented Apr 29, 2024

Uh oh!

vladinator1000 commented May 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eps1lon commented May 21, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

eps1lon commented Nov 20, 2021 •

edited

Loading

sizebot commented Nov 20, 2021 •

edited

Loading

vladinator1000 commented May 21, 2024 •

edited

Loading