From 67b5c96357c6f72cdcdcef96f974b176e9e59c35 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Sat, 20 Nov 2021 13:46:05 +0100 Subject: [PATCH 1/5] Expected behavior --- .../src/__tests__/ReactNewContext-test.js | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/packages/react-reconciler/src/__tests__/ReactNewContext-test.js b/packages/react-reconciler/src/__tests__/ReactNewContext-test.js index 9bfc2cbb549e9..d49ccf9315dc4 100644 --- a/packages/react-reconciler/src/__tests__/ReactNewContext-test.js +++ b/packages/react-reconciler/src/__tests__/ReactNewContext-test.js @@ -874,6 +874,48 @@ describe('ReactNewContext', () => { } }); + it('warns not if multiple renderers finished rendering the same context', () => { + spyOnDev(console, 'error'); + const Context = React.createContext(0); + + function Foo(props) { + Scheduler.unstable_yieldValue('Foo'); + return null; + } + + function App(props) { + return ( + + + + + ); + } + + if (gate(flags => flags.enableSyncDefaultUpdates)) { + React.startTransition(() => { + ReactNoop.render(); + }); + } else { + ReactNoop.render(); + } + expect(Scheduler).toFlushAndYield(['Foo', 'Foo']); + + // Get a new copy of ReactNoop + jest.resetModules(); + React = require('react'); + ReactNoop = require('react-noop-renderer'); + Scheduler = require('scheduler'); + + // Render the provider again using a different renderer + ReactNoop.render(); + expect(Scheduler).toFlushAndYield(['Foo', 'Foo']); + + if (__DEV__) { + expect(console.error).not.toHaveBeenCalled(); + } + }); + it('provider bails out if children and value are unchanged (like sCU)', () => { const Context = React.createContext(0); From bb3309874eccc55dcc91ff843bd30742d90fec85 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Sat, 20 Nov 2021 13:46:47 +0100 Subject: [PATCH 2/5] Don't warn about concurrent provider renders if we finished rendering --- packages/react-reconciler/src/ReactFiberNewContext.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberNewContext.js b/packages/react-reconciler/src/ReactFiberNewContext.js index acaafbce15266..e314acd1cda0e 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.js @@ -141,6 +141,7 @@ export function popProvider( } else { context._currentValue = currentValue; } + context._currentRenderer = null; } else { if ( enableServerContext && @@ -150,6 +151,7 @@ export function popProvider( } else { context._currentValue2 = currentValue; } + context._currentRenderer2 = null; } } From 91b97053442c661221ca3b04dcc7e3db1306c5da Mon Sep 17 00:00:00 2001 From: eps1lon Date: Sat, 20 Nov 2021 13:54:10 +0100 Subject: [PATCH 3/5] Only need to reset in DEV since we only set in DEV --- packages/react-reconciler/src/ReactFiberNewContext.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberNewContext.js b/packages/react-reconciler/src/ReactFiberNewContext.js index e314acd1cda0e..7d6bb2a5cd6cb 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.js @@ -141,7 +141,9 @@ export function popProvider( } else { context._currentValue = currentValue; } - context._currentRenderer = null; + if (__DEV__) { + context._currentRenderer = null; + } } else { if ( enableServerContext && @@ -151,7 +153,9 @@ export function popProvider( } else { context._currentValue2 = currentValue; } - context._currentRenderer2 = null; + if (__DEV__) { + context._currentRenderer2 = null; + } } } From 2ab9b80c375c81ad98889a405c4049a5e28c0a62 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Sat, 5 Mar 2022 17:39:14 +0100 Subject: [PATCH 4/5] Restore currentRenderN instead of resetting it --- .../src/ReactFiberNewContext.js | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberNewContext.js b/packages/react-reconciler/src/ReactFiberNewContext.js index 7d6bb2a5cd6cb..9e9abfb620798 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.js @@ -49,6 +49,15 @@ import {REACT_SERVER_CONTEXT_DEFAULT_VALUE_NOT_LOADED} from 'shared/ReactSymbols const valueCursor: StackCursor = createCursor(null); +let rendererCursorDEV: StackCursor; +if (__DEV__) { + rendererCursorDEV = createCursor(null); +} +let renderer2CursorDEV: StackCursor; +if (__DEV__) { + renderer2CursorDEV = createCursor(null); +} + let rendererSigil; if (__DEV__) { // Use this to detect multiple renderers using the same context @@ -94,6 +103,8 @@ export function pushProvider( context._currentValue = nextValue; if (__DEV__) { + push(rendererCursorDEV, context._currentRenderer, providerFiber); + if ( context._currentRenderer !== undefined && context._currentRenderer !== null && @@ -111,6 +122,8 @@ export function pushProvider( context._currentValue2 = nextValue; if (__DEV__) { + push(renderer2CursorDEV, context._currentRenderer2, providerFiber); + if ( context._currentRenderer2 !== undefined && context._currentRenderer2 !== null && @@ -131,7 +144,7 @@ export function popProvider( providerFiber: Fiber, ): void { const currentValue = valueCursor.current; - pop(valueCursor, providerFiber); + if (isPrimaryRenderer) { if ( enableServerContext && @@ -142,7 +155,9 @@ export function popProvider( context._currentValue = currentValue; } if (__DEV__) { - context._currentRenderer = null; + const currentRenderer = rendererCursorDEV.current; + pop(rendererCursorDEV, providerFiber); + context._currentRenderer = currentRenderer; } } else { if ( @@ -154,9 +169,13 @@ export function popProvider( context._currentValue2 = currentValue; } if (__DEV__) { - context._currentRenderer2 = null; + const currentRenderer2 = renderer2CursorDEV.current; + pop(renderer2CursorDEV, providerFiber); + context._currentRenderer2 = currentRenderer2; } } + + pop(valueCursor, providerFiber); } export function scheduleContextWorkOnParentPath( From 4e9a32ea95426a9dc9636964554dd85fd60053a4 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Wed, 11 Jan 2023 09:07:52 +0100 Subject: [PATCH 5/5] Update packages/react-reconciler/src/__tests__/ReactNewContext-test.js --- packages/react-reconciler/src/__tests__/ReactNewContext-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/__tests__/ReactNewContext-test.js b/packages/react-reconciler/src/__tests__/ReactNewContext-test.js index d49ccf9315dc4..7aacafab8c3af 100644 --- a/packages/react-reconciler/src/__tests__/ReactNewContext-test.js +++ b/packages/react-reconciler/src/__tests__/ReactNewContext-test.js @@ -874,7 +874,7 @@ describe('ReactNewContext', () => { } }); - it('warns not if multiple renderers finished rendering the same context', () => { + it('does not warn if multiple renderers use the same context sequentially', () => { spyOnDev(console, 'error'); const Context = React.createContext(0);