From 32a2f86f7553daa9a9049bd033bf88f090e26772 Mon Sep 17 00:00:00 2001 From: Sebastian Sebbie Silbermann Date: Thu, 19 Dec 2024 15:22:54 +0100 Subject: [PATCH 1/4] Turn off `enableYieldingBeforePassive` --- packages/shared/ReactFeatureFlags.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 8540d6e14721a..a5363afa2d059 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -78,7 +78,8 @@ export const enableLegacyFBSupport = false; // ----------------------------------------------------------------------------- // Yield to the browser event loop and not just the scheduler event loop before passive effects. -export const enableYieldingBeforePassive = __EXPERIMENTAL__; +// Introduced a regression by not flushing passive effects with Suspensey CSS: https://codesandbox.io/p/sandbox/suspensey-css-and-passive-effects-mww52c +export const enableYieldingBeforePassive = false; export const enableLegacyCache = __EXPERIMENTAL__; From a8efb5b695aea6335cb72103703c358855cdb8a2 Mon Sep 17 00:00:00 2001 From: Sebastian Sebbie Silbermann Date: Thu, 19 Dec 2024 15:28:03 +0100 Subject: [PATCH 2/4] Add failing test --- .../ReactSuspenseyCommitPhase-test.js | 42 +++++++++++++++++++ packages/shared/ReactFeatureFlags.js | 2 +- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js index 4dbba1bca21f0..2b5707b8c554d 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js @@ -491,4 +491,46 @@ describe('ReactSuspenseyCommitPhase', () => { , ); }); + + // FIXME: Should pass with `enableYieldingBeforePassive` + // @gate !enableYieldingBeforePassive + it('runs passive effects after suspended commit resolves', async () => { + function Effect() { + React.useEffect(() => { + Scheduler.log('flush effect'); + }); + return ; + } + + const root = ReactNoop.createRoot(); + + await act(() => { + root.render( + }> + + + , + ); + }); + + assertLog([ + 'render effect', + 'Image requested [A]', + 'Loading...', + 'render effect', + ]); + expect(root).toMatchRenderedOutput('Loading...'); + + await act(() => { + resolveSuspenseyThing('A'); + }); + + assertLog(['flush effect']); + expect(root).toMatchRenderedOutput( + <> + {'render effect'} + + , + ); + }); }); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index a5363afa2d059..9f25afe55e9f3 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -78,7 +78,7 @@ export const enableLegacyFBSupport = false; // ----------------------------------------------------------------------------- // Yield to the browser event loop and not just the scheduler event loop before passive effects. -// Introduced a regression by not flushing passive effects with Suspensey CSS: https://codesandbox.io/p/sandbox/suspensey-css-and-passive-effects-mww52c +// Fix gated tests that fail with this flag enabled before turning it back on. export const enableYieldingBeforePassive = false; export const enableLegacyCache = __EXPERIMENTAL__; From 9948ef542d3e3679d1c5e65b47b7aaff7d5e4633 Mon Sep 17 00:00:00 2001 From: Sebastian Sebbie Silbermann Date: Thu, 19 Dec 2024 19:52:38 +0100 Subject: [PATCH 3/4] Make dynamic --- packages/shared/forks/ReactFeatureFlags.www-dynamic.js | 3 +++ scripts/jest/setupTests.www.js | 3 +++ 2 files changed, 6 insertions(+) diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index 69da420d0422c..2a7d985a80a7a 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -39,6 +39,9 @@ export const enableSiblingPrerendering = __VARIANT__; export const enableUseResourceEffectHook = __VARIANT__; +// No GK yet. +export const enableYieldingBeforePassive = __VARIANT__; + // TODO: These flags are hard-coded to the default values used in open source. // Update the tests so that they pass in either mode, then set these // to __VARIANT__. diff --git a/scripts/jest/setupTests.www.js b/scripts/jest/setupTests.www.js index efee213861ca6..2e7c01e0ddcdc 100644 --- a/scripts/jest/setupTests.www.js +++ b/scripts/jest/setupTests.www.js @@ -8,6 +8,9 @@ jest.mock('shared/ReactFeatureFlags', () => { ); const actual = jest.requireActual('shared/forks/ReactFeatureFlags.www'); + // Can be removed once it becomes truly dynamic behind a GK. + actual.enableYieldingBeforePassive = __VARIANT__; + // Flags that aren't currently used, but we still want to force variants to keep the // code live. actual.disableInputAttributeSyncing = __VARIANT__; From 841e65808fdf0506eda0dcda63c2fee48773a747 Mon Sep 17 00:00:00 2001 From: Sebastian Sebbie Silbermann Date: Thu, 19 Dec 2024 20:06:23 +0100 Subject: [PATCH 4/4] Revert "Make dynamic" This reverts commit 9948ef542d3e3679d1c5e65b47b7aaff7d5e4633. A bunch of tests weren't gated on this behavior. --- packages/shared/forks/ReactFeatureFlags.www-dynamic.js | 3 --- scripts/jest/setupTests.www.js | 3 --- 2 files changed, 6 deletions(-) diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index 2a7d985a80a7a..69da420d0422c 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -39,9 +39,6 @@ export const enableSiblingPrerendering = __VARIANT__; export const enableUseResourceEffectHook = __VARIANT__; -// No GK yet. -export const enableYieldingBeforePassive = __VARIANT__; - // TODO: These flags are hard-coded to the default values used in open source. // Update the tests so that they pass in either mode, then set these // to __VARIANT__. diff --git a/scripts/jest/setupTests.www.js b/scripts/jest/setupTests.www.js index 2e7c01e0ddcdc..efee213861ca6 100644 --- a/scripts/jest/setupTests.www.js +++ b/scripts/jest/setupTests.www.js @@ -8,9 +8,6 @@ jest.mock('shared/ReactFeatureFlags', () => { ); const actual = jest.requireActual('shared/forks/ReactFeatureFlags.www'); - // Can be removed once it becomes truly dynamic behind a GK. - actual.enableYieldingBeforePassive = __VARIANT__; - // Flags that aren't currently used, but we still want to force variants to keep the // code live. actual.disableInputAttributeSyncing = __VARIANT__;