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/3] 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/3] 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 d94334a83cfc9ecbe63ba0e625ed43c3a821f744 Mon Sep 17 00:00:00 2001 From: Sebastian Sebbie Silbermann Date: Thu, 19 Dec 2024 16:23:54 +0100 Subject: [PATCH 3/3] Ensure passive effects are flushed after a suspended commit resolves --- packages/react-reconciler/src/ReactFiberWorkLoop.js | 3 +++ .../src/__tests__/ReactSuspenseyCommitPhase-test.js | 2 -- packages/shared/ReactFeatureFlags.js | 3 +-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index a71eb543cfe5d..28a8c733d105a 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -3278,6 +3278,9 @@ function commitRootImpl( // We don't schedule a separate task for flushing passive effects. // Instead, we just rely on ensureRootIsScheduled below to schedule // a callback for us to flush the passive effects. + // We do need to clear this callback. + // Otherwise, we'll still think the commit is suspended. + root.cancelPendingCommit = null; } else { // So we can clear these now to allow a new callback to be scheduled. root.callbackNode = null; diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js index 2b5707b8c554d..d946a473d67a0 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js @@ -492,8 +492,6 @@ describe('ReactSuspenseyCommitPhase', () => { ); }); - // FIXME: Should pass with `enableYieldingBeforePassive` - // @gate !enableYieldingBeforePassive it('runs passive effects after suspended commit resolves', async () => { function Effect() { React.useEffect(() => { diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 9f25afe55e9f3..8540d6e14721a 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -78,8 +78,7 @@ export const enableLegacyFBSupport = false; // ----------------------------------------------------------------------------- // Yield to the browser event loop and not just the scheduler event loop before passive effects. -// Fix gated tests that fail with this flag enabled before turning it back on. -export const enableYieldingBeforePassive = false; +export const enableYieldingBeforePassive = __EXPERIMENTAL__; export const enableLegacyCache = __EXPERIMENTAL__;