From 5d04d73274a884ed53106677d56dd837ae668c45 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Mon, 12 May 2025 17:39:20 +0100 Subject: [PATCH 01/11] Add eager alternate.stateNode cleanup (#33161) This is a fix for a problem where React retains shadow nodes longer than it needs to. The behaviour is shown in React Native test: https://github.com/facebook/react-native/blob/main/packages/react-native/src/private/__tests__/utilities/__tests__/ShadowNodeReferenceCounter-itest.js#L169 # Problem When React commits a new shadow tree, old shadow nodes are stored inside `fiber.alternate.stateNode`. This is not cleared up until React clones the node again. This may be problematic if mutation deletes a subtree, in that case `fiber.alternate.stateNode` will retain entire subtree until next update. In case of image nodes, this means retaining entire images. So when React goes from revision A: `` to revision B: ``, `fiber.alternate.stateNode` will be pointing to Shadow Node that represents revision A.. ![image](https://github.com/user-attachments/assets/076b677e-d152-4763-8c9d-4f923212b424) # Fix To fix this, this PR adds a new feature flag `enableEagerAlternateStateNodeCleanup`. When enabled, `alternate.stateNode` is proactively pointed towards finishedWork's stateNode, releasing resources sooner. I have verified this fixes the issue [demonstrated by React Native tests](https://github.com/facebook/react-native/blob/main/packages/react-native/src/private/__tests__/utilities/__tests__/ShadowNodeReferenceCounter-itest.js#L169). All existing React tests pass when the flag is enabled. --- .../react-reconciler/src/ReactFiberCommitWork.js | 15 +++++++++++++++ packages/shared/ReactFeatureFlags.js | 2 ++ .../forks/ReactFeatureFlags.native-fb-dynamic.js | 1 + .../shared/forks/ReactFeatureFlags.native-fb.js | 1 + .../shared/forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + .../ReactFeatureFlags.test-renderer.native-fb.js | 1 + .../forks/ReactFeatureFlags.test-renderer.www.js | 1 + packages/shared/forks/ReactFeatureFlags.www.js | 2 ++ 9 files changed, 25 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 93f2a476bd8bc..b7b17a080b172 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -59,6 +59,7 @@ import { enableComponentPerformanceTrack, enableViewTransition, enableFragmentRefs, + enableEagerAlternateStateNodeCleanup, } from 'shared/ReactFeatureFlags'; import { FunctionComponent, @@ -2170,6 +2171,20 @@ function commitMutationEffectsOnFiber( } } } + } else { + if (enableEagerAlternateStateNodeCleanup) { + if (supportsPersistence) { + if (finishedWork.alternate !== null) { + // `finishedWork.alternate.stateNode` is pointing to a stale shadow + // node at this point, retaining it and its subtree. To reclaim + // memory, point `alternate.stateNode` to new shadow node. This + // prevents shadow node from staying in memory longer than it + // needs to. The correct behaviour of this is checked by test in + // React Native: ShadowNodeReferenceCounter-itest.js#L150 + finishedWork.alternate.stateNode = finishedWork.stateNode; + } + } + } } break; } diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 217b095a29aa2..13c1121c95eaa 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -141,6 +141,8 @@ export const enablePersistedModeClonedFlag = false; export const enableShallowPropDiffing = false; +export const enableEagerAlternateStateNodeCleanup = true; + /** * Enables an expiration time for retry lanes to avoid starvation. */ diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js index 29a49ae366c81..aa413984e8f38 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js @@ -22,6 +22,7 @@ export const enableObjectFiber = __VARIANT__; export const enableHiddenSubtreeInsertionEffectCleanup = __VARIANT__; export const enablePersistedModeClonedFlag = __VARIANT__; export const enableShallowPropDiffing = __VARIANT__; +export const enableEagerAlternateStateNodeCleanup = __VARIANT__; export const passChildrenWhenCloningPersistedNodes = __VARIANT__; export const enableFastAddPropertiesInDiffing = __VARIANT__; export const enableLazyPublicInstanceInFabric = __VARIANT__; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 8383c0dc351b5..d885f9f71642e 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -24,6 +24,7 @@ export const { enableObjectFiber, enablePersistedModeClonedFlag, enableShallowPropDiffing, + enableEagerAlternateStateNodeCleanup, passChildrenWhenCloningPersistedNodes, enableFastAddPropertiesInDiffing, enableLazyPublicInstanceInFabric, diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index acc04dfb055bb..301552857f870 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -49,6 +49,7 @@ export const enableSchedulingProfiler = __PROFILE__; export const enableComponentPerformanceTrack = false; export const enableScopeAPI = false; export const enableShallowPropDiffing = false; +export const enableEagerAlternateStateNodeCleanup = false; export const enableSuspenseAvoidThisFallback = false; export const enableSuspenseCallback = false; export const enableTaint = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 4e51e7260e901..9d8a1808d9923 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -62,6 +62,7 @@ export const enableInfiniteRenderLoopDetection = false; export const renameElementSymbol = true; export const enableShallowPropDiffing = false; +export const enableEagerAlternateStateNodeCleanup = false; export const enableYieldingBeforePassive = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index c9bd058f1fef8..4a9fb9737ac04 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -47,6 +47,7 @@ export const enableSchedulingProfiler = __PROFILE__; export const enableComponentPerformanceTrack = false; export const enableScopeAPI = false; export const enableShallowPropDiffing = false; +export const enableEagerAlternateStateNodeCleanup = false; export const enableSuspenseAvoidThisFallback = false; export const enableSuspenseCallback = false; export const enableTaint = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index baceb1e6b727c..2858799494842 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -71,6 +71,7 @@ export const renameElementSymbol = false; export const enableObjectFiber = false; export const enableShallowPropDiffing = false; +export const enableEagerAlternateStateNodeCleanup = false; export const enableHydrationLaneScheduling = true; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 5ecd0073edd3f..4869d77741d05 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -107,6 +107,8 @@ export const disableLegacyMode = true; export const enableShallowPropDiffing = false; +export const enableEagerAlternateStateNodeCleanup = false; + export const enableLazyPublicInstanceInFabric = false; export const enableGestureTransition = false; From 2bcf06b69254cad6f7e702bf7d65c4f30478668c Mon Sep 17 00:00:00 2001 From: Jenny Steele Date: Mon, 12 May 2025 18:16:15 -0700 Subject: [PATCH 02/11] [ReactFlightWebpackPlugin] Add support for .mjs file extension (#33028) ## Summary Our builds generate files with a `.mjs` file extension. These are currently filtered out by `ReactFlightWebpackPlugin` so I am updating it to support this file extension. This fixes https://github.com/facebook/react/issues/33155 ## How did you test this change? I built the plugin with this change and used `yalc` to test it in my project. I confirmed the expected files now show up in `react-client-manifest.json` --- .../src/ReactFlightWebpackPlugin.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/react-server-dom-webpack/src/ReactFlightWebpackPlugin.js b/packages/react-server-dom-webpack/src/ReactFlightWebpackPlugin.js index 816b9f2a73185..59723c903f642 100644 --- a/packages/react-server-dom-webpack/src/ReactFlightWebpackPlugin.js +++ b/packages/react-server-dom-webpack/src/ReactFlightWebpackPlugin.js @@ -277,8 +277,14 @@ export default class ReactFlightWebpackPlugin { chunkGroup.chunks.forEach(function (c) { // eslint-disable-next-line no-for-of-loops/no-for-of-loops for (const file of c.files) { - if (!file.endsWith('.js')) return; - if (file.endsWith('.hot-update.js')) return; + if (!(file.endsWith('.js') || file.endsWith('.mjs'))) { + return; + } + if ( + file.endsWith('.hot-update.js') || + file.endsWith('.hot-update.mjs') + ) + return; chunks.push(c.id, file); break; } From b94603b95504130aec72f61e02d7b66d48f33653 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 13 May 2025 10:17:53 -0400 Subject: [PATCH 03/11] [Fizz] Gate rel="expect" behind enableFizzBlockingRender (#33183) Enabled in experimental channel. We know this is critical semantics to enforce at the HTML level since if you don't then you can't add explicit boundaries after the fact. However, this might have to go in a major release to allow for upgrading. --- .../src/server/ReactFizzConfigDOM.js | 38 +++++++++------- .../src/__tests__/ReactDOMFizzServer-test.js | 23 ++++++++-- .../ReactDOMFizzServerBrowser-test.js | 22 ++++++++-- .../__tests__/ReactDOMFizzServerEdge-test.js | 12 +++-- .../__tests__/ReactDOMFizzServerNode-test.js | 12 +++-- .../ReactDOMFizzStaticBrowser-test.js | 23 +++++++--- .../__tests__/ReactDOMFizzStaticNode-test.js | 12 +++-- .../src/__tests__/ReactDOMFloat-test.js | 10 ++++- .../src/__tests__/ReactDOMLegacyFloat-test.js | 11 ++++- .../src/__tests__/ReactRenderDocument-test.js | 44 +++++++++++++++---- .../src/__tests__/ReactFlightDOM-test.js | 22 ++++++++-- .../__tests__/ReactFlightDOMBrowser-test.js | 12 ++++- packages/shared/ReactFeatureFlags.js | 2 + .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + ...actFeatureFlags.test-renderer.native-fb.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 1 + 19 files changed, 194 insertions(+), 55 deletions(-) diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index a54a623bb3954..85ec4ae736465 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -34,6 +34,7 @@ import {Children} from 'react'; import { enableFizzExternalRuntime, enableSrcObject, + enableFizzBlockingRender, } from 'shared/ReactFeatureFlags'; import type { @@ -4146,16 +4147,21 @@ export function writeCompletedRoot( // we need to track the paint time of the shell so we know how much to throttle the reveal. writeShellTimeInstruction(destination, resumableState, renderState); } - const preamble = renderState.preamble; - if (preamble.htmlChunks || preamble.headChunks) { - // If we rendered the whole document, then we emitted a rel="expect" that needs a - // matching target. Normally we use one of the bootstrap scripts for this but if - // there are none, then we need to emit a tag to complete the shell. - if ((resumableState.instructions & SentCompletedShellId) === NothingSent) { - writeChunk(destination, startChunkForTag('template')); - writeCompletedShellIdAttribute(destination, resumableState); - writeChunk(destination, endOfStartTag); - writeChunk(destination, endChunkForTag('template')); + if (enableFizzBlockingRender) { + const preamble = renderState.preamble; + if (preamble.htmlChunks || preamble.headChunks) { + // If we rendered the whole document, then we emitted a rel="expect" that needs a + // matching target. Normally we use one of the bootstrap scripts for this but if + // there are none, then we need to emit a tag to complete the shell. + if ( + (resumableState.instructions & SentCompletedShellId) === + NothingSent + ) { + writeChunk(destination, startChunkForTag('template')); + writeCompletedShellIdAttribute(destination, resumableState); + writeChunk(destination, endOfStartTag); + writeChunk(destination, endChunkForTag('template')); + } } } return writeBootstrap(destination, renderState); @@ -5040,11 +5046,13 @@ function writeBlockingRenderInstruction( resumableState: ResumableState, renderState: RenderState, ): void { - const idPrefix = resumableState.idPrefix; - const shellId = '\u00AB' + idPrefix + 'R\u00BB'; - writeChunk(destination, blockingRenderChunkStart); - writeChunk(destination, stringToChunk(escapeTextForBrowser(shellId))); - writeChunk(destination, blockingRenderChunkEnd); + if (enableFizzBlockingRender) { + const idPrefix = resumableState.idPrefix; + const shellId = '\u00AB' + idPrefix + 'R\u00BB'; + writeChunk(destination, blockingRenderChunkStart); + writeChunk(destination, stringToChunk(escapeTextForBrowser(shellId))); + writeChunk(destination, blockingRenderChunkEnd); + } } const completedShellIdAttributeStart = stringToPrecomputedChunk(' id="'); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 1be040bfe36c9..8bb3e2f4b74b9 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -3590,7 +3590,9 @@ describe('ReactDOMFizzServer', () => { (gate(flags => flags.shouldUseFizzExternalRuntime) ? '' : '') + - '', + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : ''), ); }); @@ -4523,7 +4525,15 @@ describe('ReactDOMFizzServer', () => { // the html should be as-is expect(document.documentElement.innerHTML).toEqual( - '

hello world!

', + '' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '

hello world!

' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '', ); }); @@ -6512,7 +6522,14 @@ describe('ReactDOMFizzServer', () => { (gate(flags => flags.shouldUseFizzExternalRuntime) ? '' : '') + - '', + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '', ); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js index f5b01d2462403..7c0aa14c2607d 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js @@ -84,9 +84,15 @@ describe('ReactDOMFizzServerBrowser', () => { ), ); const result = await readResult(stream); - expect(result).toMatchInlineSnapshot( - `"hello world"`, - ); + if (gate(flags => flags.enableFizzBlockingRender)) { + expect(result).toMatchInlineSnapshot( + `"hello world"`, + ); + } else { + expect(result).toMatchInlineSnapshot( + `"hello world"`, + ); + } }); it('should emit bootstrap script src at the end', async () => { @@ -529,7 +535,15 @@ describe('ReactDOMFizzServerBrowser', () => { const result = await readResult(stream); expect(result).toEqual( - 'foobar', + '' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + 'foobar' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '', ); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerEdge-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerEdge-test.js index 1eefe1a4082e6..801baa9678af4 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerEdge-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerEdge-test.js @@ -71,8 +71,14 @@ describe('ReactDOMFizzServerEdge', () => { setTimeout(resolve, 1); }); - expect(result).toMatchInlineSnapshot( - `"
hello
"`, - ); + if (gate(flags => flags.enableFizzBlockingRender)) { + expect(result).toMatchInlineSnapshot( + `"
hello
"`, + ); + } else { + expect(result).toMatchInlineSnapshot( + `"
hello
"`, + ); + } }); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js index 2704c243eba48..d4f3486a1dbed 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js @@ -78,9 +78,15 @@ describe('ReactDOMFizzServerNode', () => { pipe(writable); }); // with Float, we emit empty heads if they are elided when rendering - expect(output.result).toMatchInlineSnapshot( - `"hello world"`, - ); + if (gate(flags => flags.enableFizzBlockingRender)) { + expect(output.result).toMatchInlineSnapshot( + `"hello world"`, + ); + } else { + expect(output.result).toMatchInlineSnapshot( + `"hello world"`, + ); + } }); it('should emit bootstrap script src at the end', async () => { diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js index 578b2bf916f61..55a89bc525a3c 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js @@ -195,9 +195,15 @@ describe('ReactDOMFizzStaticBrowser', () => { ), ); const prelude = await readContent(result.prelude); - expect(prelude).toMatchInlineSnapshot( - `"hello world"`, - ); + if (gate(flags => flags.enableFizzBlockingRender)) { + expect(prelude).toMatchInlineSnapshot( + `"hello world"`, + ); + } else { + expect(prelude).toMatchInlineSnapshot( + `"hello world"`, + ); + } }); it('should emit bootstrap script src at the end', async () => { @@ -1438,8 +1444,15 @@ describe('ReactDOMFizzStaticBrowser', () => { expect(await readContent(content)).toBe( '' + '' + - '' + - 'Hello', + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '' + + 'Hello' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '', ); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js index d9bd7c70db0b4..3aa7a70cb80df 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzStaticNode-test.js @@ -63,9 +63,15 @@ describe('ReactDOMFizzStaticNode', () => { , ); const prelude = await readContent(result.prelude); - expect(prelude).toMatchInlineSnapshot( - `"hello world"`, - ); + if (gate(flags => flags.enableFizzBlockingRender)) { + expect(prelude).toMatchInlineSnapshot( + `"hello world"`, + ); + } else { + expect(prelude).toMatchInlineSnapshot( + `"hello world"`, + ); + } }); // @gate experimental diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index b7f42bcf8dbef..2d3f1f0b8b621 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -704,8 +704,14 @@ describe('ReactDOMFloat', () => { (gate(flags => flags.shouldUseFizzExternalRuntime) ? '' : '') + - 'foo' + - 'bar', + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + 'foo' + + 'bar' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : ''), '', ]); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMLegacyFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMLegacyFloat-test.js index f2cabafc9f575..f6d868c84135a 100644 --- a/packages/react-dom/src/__tests__/ReactDOMLegacyFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMLegacyFloat-test.js @@ -34,8 +34,15 @@ describe('ReactDOMFloat', () => { ); expect(result).toEqual( - '' + - 'title', + '' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + 'title' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '', ); }); }); diff --git a/packages/react-dom/src/__tests__/ReactRenderDocument-test.js b/packages/react-dom/src/__tests__/ReactRenderDocument-test.js index 2b54bc90090e4..3501f1bd92af7 100644 --- a/packages/react-dom/src/__tests__/ReactRenderDocument-test.js +++ b/packages/react-dom/src/__tests__/ReactRenderDocument-test.js @@ -78,14 +78,20 @@ describe('rendering React components at document', () => { root = ReactDOMClient.hydrateRoot(testDocument, ); }); expect(testDocument.body.innerHTML).toBe( - 'Hello world' + '', + 'Hello world' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : ''), ); await act(() => { root.render(); }); expect(testDocument.body.innerHTML).toBe( - 'Hello moon' + '', + 'Hello moon' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : ''), ); expect(body === testDocument.body).toBe(true); @@ -112,7 +118,10 @@ describe('rendering React components at document', () => { root = ReactDOMClient.hydrateRoot(testDocument, ); }); expect(testDocument.body.innerHTML).toBe( - 'Hello world' + '', + 'Hello world' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : ''), ); const originalDocEl = testDocument.documentElement; @@ -124,9 +133,15 @@ describe('rendering React components at document', () => { expect(testDocument.firstChild).toBe(originalDocEl); expect(testDocument.head).toBe(originalHead); expect(testDocument.body).toBe(originalBody); - expect(originalBody.innerHTML).toBe(''); + expect(originalBody.innerHTML).toBe( + gate(flags => flags.enableFizzBlockingRender) + ? '' + : '', + ); expect(originalHead.innerHTML).toBe( - '', + gate(flags => flags.enableFizzBlockingRender) + ? '' + : '', ); }); @@ -166,7 +181,10 @@ describe('rendering React components at document', () => { }); expect(testDocument.body.innerHTML).toBe( - 'Hello world' + '', + 'Hello world' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : ''), ); await act(() => { @@ -174,7 +192,9 @@ describe('rendering React components at document', () => { }); expect(testDocument.body.innerHTML).toBe( - '' + 'Goodbye world', + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + 'Goodbye world', ); }); @@ -205,7 +225,10 @@ describe('rendering React components at document', () => { }); expect(testDocument.body.innerHTML).toBe( - 'Hello world' + '', + 'Hello world' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : ''), ); }); @@ -341,7 +364,10 @@ describe('rendering React components at document', () => { expect(testDocument.body.innerHTML).toBe( favorSafetyOverHydrationPerf ? 'Hello world' - : 'Goodbye world', + : 'Goodbye world' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : ''), ); }); diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js index 80562624eb173..05a6a227c2b50 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js @@ -1921,14 +1921,28 @@ describe('ReactFlightDOM', () => { expect(content1).toEqual( '' + '' + - '' + - '

hello world

', + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '' + + '

hello world

' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '', ); expect(content2).toEqual( '' + '' + - '' + - '

hello world

', + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '' + + '

hello world

' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '', ); }); diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js index 4313c379b70bd..9d668ea3c3e01 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js @@ -1899,8 +1899,16 @@ describe('ReactFlightDOMBrowser', () => { } expect(content).toEqual( - '' + - '

hello world

', + '' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '' + + '

hello world

' + + (gate(flags => flags.enableFizzBlockingRender) + ? '' + : '') + + '', ); }); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 13c1121c95eaa..216b6d668a95f 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -98,6 +98,8 @@ export const enableScrollEndPolyfill = __EXPERIMENTAL__; export const enableSuspenseyImages = false; +export const enableFizzBlockingRender = __EXPERIMENTAL__; // rel="expect" + export const enableSrcObject = __EXPERIMENTAL__; export const enableHydrationChangeEvent = __EXPERIMENTAL__; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index d885f9f71642e..4298a267a36ff 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -83,6 +83,7 @@ export const enableViewTransition = false; export const enableGestureTransition = false; export const enableScrollEndPolyfill = true; export const enableSuspenseyImages = false; +export const enableFizzBlockingRender = true; export const enableSrcObject = false; export const enableHydrationChangeEvent = true; export const enableDefaultTransitionIndicator = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 301552857f870..8a7a59ebb2654 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -73,6 +73,7 @@ export const enableFastAddPropertiesInDiffing = false; export const enableLazyPublicInstanceInFabric = false; export const enableScrollEndPolyfill = true; export const enableSuspenseyImages = false; +export const enableFizzBlockingRender = true; export const enableSrcObject = false; export const enableHydrationChangeEvent = false; export const enableDefaultTransitionIndicator = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 9d8a1808d9923..6de2578838ff6 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -73,6 +73,7 @@ export const enableFastAddPropertiesInDiffing = true; export const enableLazyPublicInstanceInFabric = false; export const enableScrollEndPolyfill = true; export const enableSuspenseyImages = false; +export const enableFizzBlockingRender = true; export const enableSrcObject = false; export const enableHydrationChangeEvent = false; export const enableDefaultTransitionIndicator = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index 4a9fb9737ac04..643626d6f9731 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -70,6 +70,7 @@ export const enableFastAddPropertiesInDiffing = false; export const enableLazyPublicInstanceInFabric = false; export const enableScrollEndPolyfill = true; export const enableSuspenseyImages = false; +export const enableFizzBlockingRender = true; export const enableSrcObject = false; export const enableHydrationChangeEvent = false; export const enableDefaultTransitionIndicator = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 2858799494842..d22d5cadd744d 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -84,6 +84,7 @@ export const enableFastAddPropertiesInDiffing = false; export const enableLazyPublicInstanceInFabric = false; export const enableScrollEndPolyfill = true; export const enableSuspenseyImages = false; +export const enableFizzBlockingRender = true; export const enableSrcObject = false; export const enableHydrationChangeEvent = false; export const enableDefaultTransitionIndicator = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 4869d77741d05..839248e2e7b04 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -114,6 +114,7 @@ export const enableLazyPublicInstanceInFabric = false; export const enableGestureTransition = false; export const enableSuspenseyImages = false; +export const enableFizzBlockingRender = true; export const enableSrcObject = false; export const enableHydrationChangeEvent = false; export const enableDefaultTransitionIndicator = false; From 997c7bc930304142b3af37bcb21599181124aeb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 13 May 2025 12:39:10 -0400 Subject: [PATCH 04/11] [DevTools] Get source location from structured callsites in prepareStackTrace (#33143) When we get the source location for "View source for this element" we should be using the enclosing function of the callsite of the child. So that we don't just point to some random line within the component. This is similar to the technique in #33136. This technique is now really better than the fake throw technique, when available. So I now favor the owner technique. The only problem it's only available in DEV and only if it has a child that's owned (and not filtered). We could implement this same technique for the error that's thrown in the fake throwing solution. However, we really shouldn't need that at all because for client components we should be able to call `inspect(fn)` at least in Chrome which is even better. --- .../src/backend/fiber/renderer.js | 26 +++---- .../src/backend/shared/DevToolsOwnerStack.js | 6 +- .../src/backend/utils/index.js | 73 +++++++++++++++++++ 3 files changed, 90 insertions(+), 15 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 711d76c92d075..4fc59a24d9272 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -50,6 +50,7 @@ import { gt, gte, parseSourceFromComponentStack, + parseSourceFromOwnerStack, serializeToString, } from 'react-devtools-shared/src/backend/utils'; import { @@ -5805,15 +5806,13 @@ export function attach( function getSourceForFiberInstance( fiberInstance: FiberInstance, ): Source | null { - const unresolvedSource = fiberInstance.source; - if ( - unresolvedSource !== null && - typeof unresolvedSource === 'object' && - !isError(unresolvedSource) - ) { - // $FlowFixMe: isError should have refined it. - return unresolvedSource; + // Favor the owner source if we have one. + const ownerSource = getSourceForInstance(fiberInstance); + if (ownerSource !== null) { + return ownerSource; } + + // Otherwise fallback to the throwing trick. const dispatcherRef = getDispatcherRef(renderer); const stackFrame = dispatcherRef == null @@ -5824,10 +5823,7 @@ export function attach( dispatcherRef, ); if (stackFrame === null) { - // If we don't find a source location by throwing, try to get one - // from an owned child if possible. This is the same branch as - // for virtual instances. - return getSourceForInstance(fiberInstance); + return null; } const source = parseSourceFromComponentStack(stackFrame); fiberInstance.source = source; @@ -5835,7 +5831,7 @@ export function attach( } function getSourceForInstance(instance: DevToolsInstance): Source | null { - let unresolvedSource = instance.source; + const unresolvedSource = instance.source; if (unresolvedSource === null) { // We don't have any source yet. We can try again later in case an owned child mounts later. // TODO: We won't have any information here if the child is filtered. @@ -5848,7 +5844,9 @@ export function attach( // any intermediate utility functions. This won't point to the top of the component function // but it's at least somewhere within it. if (isError(unresolvedSource)) { - unresolvedSource = formatOwnerStack((unresolvedSource: any)); + return (instance.source = parseSourceFromOwnerStack( + (unresolvedSource: any), + )); } if (typeof unresolvedSource === 'string') { const idx = unresolvedSource.lastIndexOf('\n'); diff --git a/packages/react-devtools-shared/src/backend/shared/DevToolsOwnerStack.js b/packages/react-devtools-shared/src/backend/shared/DevToolsOwnerStack.js index e03d948a45d3a..7d4cfa65ce030 100644 --- a/packages/react-devtools-shared/src/backend/shared/DevToolsOwnerStack.js +++ b/packages/react-devtools-shared/src/backend/shared/DevToolsOwnerStack.js @@ -13,8 +13,12 @@ export function formatOwnerStack(error: Error): string { const prevPrepareStackTrace = Error.prepareStackTrace; // $FlowFixMe[incompatible-type] It does accept undefined. Error.prepareStackTrace = undefined; - let stack = error.stack; + const stack = error.stack; Error.prepareStackTrace = prevPrepareStackTrace; + return formatOwnerStackString(stack); +} + +export function formatOwnerStackString(stack: string): string { if (stack.startsWith('Error: react-stack-top-frame\n')) { // V8's default formatting prefixes with the error message which we // don't want/need. diff --git a/packages/react-devtools-shared/src/backend/utils/index.js b/packages/react-devtools-shared/src/backend/utils/index.js index 977683ef9a208..950161a020b21 100644 --- a/packages/react-devtools-shared/src/backend/utils/index.js +++ b/packages/react-devtools-shared/src/backend/utils/index.js @@ -18,6 +18,8 @@ import type {DehydratedData} from 'react-devtools-shared/src/frontend/types'; export {default as formatWithStyles} from './formatWithStyles'; export {default as formatConsoleArguments} from './formatConsoleArguments'; +import {formatOwnerStackString} from '../shared/DevToolsOwnerStack'; + // TODO: update this to the first React version that has a corresponding DevTools backend const FIRST_DEVTOOLS_BACKEND_LOCKSTEP_VER = '999.9.9'; export function hasAssignedBackend(version?: string): boolean { @@ -345,6 +347,77 @@ export function parseSourceFromComponentStack( return parseSourceFromFirefoxStack(componentStack); } +let collectedLocation: Source | null = null; + +function collectStackTrace( + error: Error, + structuredStackTrace: CallSite[], +): string { + let result: null | Source = null; + // Collect structured stack traces from the callsites. + // We mirror how V8 serializes stack frames and how we later parse them. + for (let i = 0; i < structuredStackTrace.length; i++) { + const callSite = structuredStackTrace[i]; + if (callSite.getFunctionName() === 'react-stack-bottom-frame') { + // We pick the last frame that matches before the bottom frame since + // that will be immediately inside the component as opposed to some helper. + // If we don't find a bottom frame then we bail to string parsing. + collectedLocation = result; + // Skip everything after the bottom frame since it'll be internals. + break; + } else { + const sourceURL = callSite.getScriptNameOrSourceURL(); + const line = + // $FlowFixMe[prop-missing] + typeof callSite.getEnclosingLineNumber === 'function' + ? (callSite: any).getEnclosingLineNumber() + : callSite.getLineNumber(); + const col = + // $FlowFixMe[prop-missing] + typeof callSite.getEnclosingColumnNumber === 'function' + ? (callSite: any).getEnclosingColumnNumber() + : callSite.getLineNumber(); + if (!sourceURL || !line || !col) { + // Skip eval etc. without source url. They don't have location. + continue; + } + result = { + sourceURL, + line: line, + column: col, + }; + } + } + // At the same time we generate a string stack trace just in case someone + // else reads it. + const name = error.name || 'Error'; + const message = error.message || ''; + let stack = name + ': ' + message; + for (let i = 0; i < structuredStackTrace.length; i++) { + stack += '\n at ' + structuredStackTrace[i].toString(); + } + return stack; +} + +export function parseSourceFromOwnerStack(error: Error): Source | null { + // First attempt to collected the structured data using prepareStackTrace. + collectedLocation = null; + const previousPrepare = Error.prepareStackTrace; + Error.prepareStackTrace = collectStackTrace; + let stack; + try { + stack = error.stack; + } finally { + Error.prepareStackTrace = previousPrepare; + } + if (collectedLocation !== null) { + return collectedLocation; + } + // Fallback to parsing the string form. + const componentStack = formatOwnerStackString(stack); + return parseSourceFromComponentStack(componentStack); +} + // 0.123456789 => 0.123 // Expects high-resolution timestamp in milliseconds, like from performance.now() // Mainly used for optimizing the size of serialized profiling payload From 676f0879f315130309262ff3532707029f0288bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 13 May 2025 15:18:02 -0400 Subject: [PATCH 05/11] Reset currentEventTransitionLane after flushing sync work (#33159) This keeps track of the transition lane allocated for this event. I want to be able to use the current one within sync work flushing to know which lane needs its loading indicator cleared. It's also a bit weird that transition work scheduled inside sync updates in the same event aren't entangled with other transitions in that event when `flushSync` is. Therefore this moves it to reset after flushing. It should have no impact. Just splitting it out into a separate PR for an abundance of caution. The only thing this might affect would be if the React internals throws and it doesn't reset after. But really it doesn't really have to reset and they're all entangled anyway. --- packages/react-reconciler/src/ReactFiberRootScheduler.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index f7c26580bb95a..4dfa1975a250b 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -257,7 +257,6 @@ function processRootScheduleInMicrotask() { // preserve the scroll position of the previous page. syncTransitionLanes = currentEventTransitionLane; } - currentEventTransitionLane = NoLane; } const currentTime = now(); @@ -315,6 +314,9 @@ function processRootScheduleInMicrotask() { if (!hasPendingCommitEffects()) { flushSyncWorkAcrossRoots_impl(syncTransitionLanes, false); } + + // Reset Event Transition Lane so that we allocate a new one next time. + currentEventTransitionLane = NoLane; } function scheduleTaskForRootDuringMicrotask( From 0cac32d60dd4482b27fe8a54dffbabceb22c6272 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 13 May 2025 15:20:59 -0400 Subject: [PATCH 06/11] [Fiber] Stash the entangled async action lane on currentEventTransitionLane (#33188) When we're entangled with an async action lane we use that lane instead of the currentEventTransitionLane. Conversely, if we start a new async action lane we reuse the currentEventTransitionLane. So they're basically supposed to be in sync but they're not if you resolve the async action and then schedule new stuff in the same event. Then you end up with two transitions in the same event with different lanes. By stashing it like this we fix that but it also gives us an opportunity to check just the currentEventTransitionLane to see if this event scheduled any regular Transition updates or Async Transitions. --- .../react-reconciler/src/ReactFiberRootScheduler.js | 11 ++++++++++- packages/react-reconciler/src/ReactFiberWorkLoop.js | 10 +--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index 4dfa1975a250b..9fc6728cf9931 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -78,6 +78,7 @@ import { resetNestedUpdateFlag, syncNestedUpdateFlag, } from './ReactProfilerTimer'; +import {peekEntangledActionLane} from './ReactFiberAsyncAction'; // A linked list of all the roots with pending work. In an idiomatic app, // there's only a single root, but we do support multi root apps, hence this @@ -647,7 +648,15 @@ export function requestTransitionLane( // over. Our heuristic for that is whenever we enter a concurrent work loop. if (currentEventTransitionLane === NoLane) { // All transitions within the same event are assigned the same lane. - currentEventTransitionLane = claimNextTransitionLane(); + const actionScopeLane = peekEntangledActionLane(); + currentEventTransitionLane = + actionScopeLane !== NoLane + ? // We're inside an async action scope. Reuse the same lane. + actionScopeLane + : // We may or may not be inside an async action scope. If we are, this + // is the first update in that scope. Either way, we need to get a + // fresh transition lane. + claimNextTransitionLane(); } return currentEventTransitionLane; } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 95285c936eb93..c62691aa1f8c4 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -356,7 +356,6 @@ import { requestTransitionLane, } from './ReactFiberRootScheduler'; import {getMaskedContext, getUnmaskedContext} from './ReactFiberContext'; -import {peekEntangledActionLane} from './ReactFiberAsyncAction'; import {logUncaughtError} from './ReactFiberErrorLogger'; import { deleteScheduledGesture, @@ -779,14 +778,7 @@ export function requestUpdateLane(fiber: Fiber): Lane { transition._updatedFibers.add(fiber); } - const actionScopeLane = peekEntangledActionLane(); - return actionScopeLane !== NoLane - ? // We're inside an async action scope. Reuse the same lane. - actionScopeLane - : // We may or may not be inside an async action scope. If we are, this - // is the first update in that scope. Either way, we need to get a - // fresh transition lane. - requestTransitionLane(transition); + return requestTransitionLane(transition); } return eventPriorityToLane(resolveUpdatePriority()); From 62d3f36ea79fc0a10b514d4bbcc4ba3f21b3206e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 13 May 2025 15:45:11 -0400 Subject: [PATCH 07/11] [Fiber] Trigger default transition indicator if needed (#33160) Stacked on #33159. This implements `onDefaultTransitionIndicator`. The sequence is: 1) In `markRootUpdated` we schedule Transition updates as needing `indicatorLanes` on the root. This tracks the lanes that currently need an indicator to either start or remain going until this lane commits. 2) Track mutations during any commit. We use the same hook that view transitions use here but instead of tracking it just per view transition scope, we also track a global boolean for the whole root. 3) If a sync/default commit had any mutations, then we clear the indicator lane for the `currentEventTransitionLane`. This requires that the lane is still active while we do these commits. See #33159. In other words, a sync update gets associated with the current transition and it is assumed to be rendering the loading state for that corresponding transition so we don't need a default indicator for this lane. 4) At the end of `processRootScheduleInMicrotask`, right before we're about to enter a new "event transition lane" scope, it is no longer possible to render any more loading states for the current transition lane. That's when we invoke `onDefaultTransitionIndicator` for any roots that have new indicator lanes. 5) When we commit, we remove the finished lanes from `indicatorLanes` and once that reaches zero again, then we can clean up the default indicator. This approach means that you can start multiple different transitions while an indicator is still going but it won't stop/restart each time. Instead, it'll wait until all are done before stopping. Follow ups: - [x] Default updates are currently not enough to cancel because those aren't flush in the same microtask. That's unfortunate. #33186 - [x] Handle async actions before the setState. Since these don't necessarily have a root this is tricky. #33190 - [x] Disable for `useDeferredValue`. ~Since it also goes through `markRootUpdated` and schedules a Transition lane it'll get a default indicator even though it probably shouldn't have one.~ EDIT: Turns out this just works because it doesn't go through `markRootUpdated` when work is left behind. - [x] Implement built-in DOM version by default. #33162 --- .../src/createReactNoop.js | 4 +- .../src/ReactFiberCommitWork.js | 18 + .../react-reconciler/src/ReactFiberLane.js | 13 + .../src/ReactFiberMutationTracking.js | 24 +- .../react-reconciler/src/ReactFiberRoot.js | 4 + .../src/ReactFiberRootScheduler.js | 42 +- .../src/ReactFiberWorkLoop.js | 30 ++ .../src/ReactInternalTypes.js | 3 + .../ReactDefaultTransitionIndicator-test.js | 358 ++++++++++++++++++ 9 files changed, 490 insertions(+), 6 deletions(-) create mode 100644 packages/react-reconciler/src/__tests__/ReactDefaultTransitionIndicator-test.js diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index dd5173cd83720..6fbad9adac195 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -1142,9 +1142,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { // TODO: Turn this on once tests are fixed // console.error(error); } - function onDefaultTransitionIndicator(): void | (() => void) { - // TODO: Allow this as an option. - } + function onDefaultTransitionIndicator(): void | (() => void) {} let idCounter = 0; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index b7b17a080b172..03d78545d4ea5 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -20,6 +20,7 @@ import type { import type {Fiber, FiberRoot} from './ReactInternalTypes'; import type {Lanes} from './ReactFiberLane'; import { + includesLoadingIndicatorLanes, includesOnlySuspenseyCommitEligibleLanes, includesOnlyViewTransitionEligibleLanes, } from './ReactFiberLane'; @@ -60,6 +61,7 @@ import { enableViewTransition, enableFragmentRefs, enableEagerAlternateStateNodeCleanup, + enableDefaultTransitionIndicator, } from 'shared/ReactFeatureFlags'; import { FunctionComponent, @@ -268,13 +270,16 @@ import { } from './ReactFiberCommitViewTransitions'; import { viewTransitionMutationContext, + pushRootMutationContext, pushMutationContext, popMutationContext, + rootMutationContext, } from './ReactFiberMutationTracking'; import { trackNamedViewTransition, untrackNamedViewTransition, } from './ReactFiberDuplicateViewTransitions'; +import {markIndicatorHandled} from './ReactFiberRootScheduler'; // Used during the commit phase to track the state of the Offscreen component stack. // Allows us to avoid traversing the return path to find the nearest Offscreen ancestor. @@ -2216,6 +2221,7 @@ function commitMutationEffectsOnFiber( case HostRoot: { const prevProfilerEffectDuration = pushNestedEffectDurations(); + pushRootMutationContext(); if (supportsResources) { prepareToCommitHoistables(); @@ -2265,6 +2271,18 @@ function commitMutationEffectsOnFiber( ); } + popMutationContext(false); + + if ( + enableDefaultTransitionIndicator && + rootMutationContext && + includesLoadingIndicatorLanes(lanes) + ) { + // This root had a mutation. Mark this root as having rendered a manual + // loading state. + markIndicatorHandled(root); + } + break; } case HostPortal: { diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index fdada6b065a29..bd7f3267efd2d 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -27,6 +27,7 @@ import { transitionLaneExpirationMs, retryLaneExpirationMs, disableLegacyMode, + enableDefaultTransitionIndicator, } from 'shared/ReactFeatureFlags'; import {isDevToolsPresent} from './ReactFiberDevToolsHook'; import {clz32} from './clz32'; @@ -640,6 +641,10 @@ export function includesOnlySuspenseyCommitEligibleLanes( ); } +export function includesLoadingIndicatorLanes(lanes: Lanes): boolean { + return (lanes & (SyncLane | DefaultLane)) !== NoLanes; +} + export function includesBlockingLane(lanes: Lanes): boolean { const SyncDefaultLanes = InputContinuousHydrationLane | @@ -766,6 +771,10 @@ export function createLaneMap(initial: T): LaneMap { export function markRootUpdated(root: FiberRoot, updateLane: Lane) { root.pendingLanes |= updateLane; + if (enableDefaultTransitionIndicator) { + // Mark that this lane might need a loading indicator to be shown. + root.indicatorLanes |= updateLane & TransitionLanes; + } // If there are any suspended transitions, it's possible this new update // could unblock them. Clear the suspended lanes so that we can try rendering @@ -847,6 +856,10 @@ export function markRootFinished( root.pingedLanes = NoLanes; root.warmLanes = NoLanes; + if (enableDefaultTransitionIndicator) { + root.indicatorLanes &= remainingLanes; + } + root.expiredLanes &= remainingLanes; root.entangledLanes &= remainingLanes; diff --git a/packages/react-reconciler/src/ReactFiberMutationTracking.js b/packages/react-reconciler/src/ReactFiberMutationTracking.js index 164ec2c6edd7d..cb439bd68fb36 100644 --- a/packages/react-reconciler/src/ReactFiberMutationTracking.js +++ b/packages/react-reconciler/src/ReactFiberMutationTracking.js @@ -7,10 +7,23 @@ * @flow */ -import {enableViewTransition} from 'shared/ReactFeatureFlags'; +import { + enableDefaultTransitionIndicator, + enableViewTransition, +} from 'shared/ReactFeatureFlags'; +export let rootMutationContext: boolean = false; export let viewTransitionMutationContext: boolean = false; +export function pushRootMutationContext(): void { + if (enableDefaultTransitionIndicator) { + rootMutationContext = false; + } + if (enableViewTransition) { + viewTransitionMutationContext = false; + } +} + export function pushMutationContext(): boolean { if (!enableViewTransition) { return false; @@ -22,12 +35,21 @@ export function pushMutationContext(): boolean { export function popMutationContext(prev: boolean): void { if (enableViewTransition) { + if (viewTransitionMutationContext) { + rootMutationContext = true; + } viewTransitionMutationContext = prev; } } export function trackHostMutation(): void { + // This is extremely hot function that must be inlined. Don't add more stuff. if (enableViewTransition) { viewTransitionMutationContext = true; + } else if (enableDefaultTransitionIndicator) { + // We only set this if enableViewTransition is not on. Otherwise we track + // it on the viewTransitionMutationContext and collect it when we pop + // to avoid more than a single operation in this hot path. + rootMutationContext = true; } } diff --git a/packages/react-reconciler/src/ReactFiberRoot.js b/packages/react-reconciler/src/ReactFiberRoot.js index cc2a528010e77..e9d107bcb899c 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.js +++ b/packages/react-reconciler/src/ReactFiberRoot.js @@ -79,6 +79,9 @@ function FiberRootNode( this.pingedLanes = NoLanes; this.warmLanes = NoLanes; this.expiredLanes = NoLanes; + if (enableDefaultTransitionIndicator) { + this.indicatorLanes = NoLanes; + } this.errorRecoveryDisabledLanes = NoLanes; this.shellSuspendCounter = 0; @@ -94,6 +97,7 @@ function FiberRootNode( if (enableDefaultTransitionIndicator) { this.onDefaultTransitionIndicator = onDefaultTransitionIndicator; + this.pendingIndicator = null; } this.pooledCache = null; diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index 9fc6728cf9931..7daed077eee57 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -20,6 +20,7 @@ import { enableComponentPerformanceTrack, enableYieldingBeforePassive, enableGestureTransition, + enableDefaultTransitionIndicator, } from 'shared/ReactFeatureFlags'; import { NoLane, @@ -80,6 +81,9 @@ import { } from './ReactProfilerTimer'; import {peekEntangledActionLane} from './ReactFiberAsyncAction'; +import noop from 'shared/noop'; +import reportGlobalError from 'shared/reportGlobalError'; + // A linked list of all the roots with pending work. In an idiomatic app, // there's only a single root, but we do support multi root apps, hence this // extra complexity. But this module is optimized for the single root case. @@ -316,8 +320,33 @@ function processRootScheduleInMicrotask() { flushSyncWorkAcrossRoots_impl(syncTransitionLanes, false); } - // Reset Event Transition Lane so that we allocate a new one next time. - currentEventTransitionLane = NoLane; + if (currentEventTransitionLane !== NoLane) { + // Reset Event Transition Lane so that we allocate a new one next time. + currentEventTransitionLane = NoLane; + startDefaultTransitionIndicatorIfNeeded(); + } +} + +function startDefaultTransitionIndicatorIfNeeded() { + if (!enableDefaultTransitionIndicator) { + return; + } + // Check all the roots if there are any new indicators needed. + let root = firstScheduledRoot; + while (root !== null) { + if (root.indicatorLanes !== NoLanes && root.pendingIndicator === null) { + // We have new indicator lanes that requires a loading state. Start the + // default transition indicator. + try { + const onDefaultTransitionIndicator = root.onDefaultTransitionIndicator; + root.pendingIndicator = onDefaultTransitionIndicator() || noop; + } catch (x) { + root.pendingIndicator = noop; + reportGlobalError(x); + } + } + root = root.next; + } } function scheduleTaskForRootDuringMicrotask( @@ -664,3 +693,12 @@ export function requestTransitionLane( export function didCurrentEventScheduleTransition(): boolean { return currentEventTransitionLane !== NoLane; } + +export function markIndicatorHandled(root: FiberRoot): void { + if (enableDefaultTransitionIndicator) { + // The current transition event rendered a synchronous loading state. + // Clear it from the indicator lanes. We don't need to show a separate + // loading state for this lane. + root.indicatorLanes &= ~currentEventTransitionLane; + } +} diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index c62691aa1f8c4..cd5c1c1468523 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -52,11 +52,14 @@ import { enableThrottledScheduling, enableViewTransition, enableGestureTransition, + enableDefaultTransitionIndicator, } from 'shared/ReactFeatureFlags'; import {resetOwnerStackLimit} from 'shared/ReactOwnerStackReset'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import is from 'shared/objectIs'; +import reportGlobalError from 'shared/reportGlobalError'; + import { // Aliased because `act` will override and push to an internal queue scheduleCallback as Scheduler_scheduleCallback, @@ -3593,6 +3596,33 @@ function flushLayoutEffects(): void { const finishedWork = pendingFinishedWork; const lanes = pendingEffectsLanes; + if (enableDefaultTransitionIndicator) { + const cleanUpIndicator = root.pendingIndicator; + if (cleanUpIndicator !== null && root.indicatorLanes === NoLanes) { + // We have now committed all Transitions that needed the default indicator + // so we can now run the clean up function. We do this in the layout phase + // so it has the same semantics as if you did it with a useLayoutEffect or + // if it was reset automatically with useOptimistic. + const prevTransition = ReactSharedInternals.T; + ReactSharedInternals.T = null; + const previousPriority = getCurrentUpdatePriority(); + setCurrentUpdatePriority(DiscreteEventPriority); + const prevExecutionContext = executionContext; + executionContext |= CommitContext; + root.pendingIndicator = null; + try { + cleanUpIndicator(); + } catch (x) { + reportGlobalError(x); + } finally { + // Reset the priority to the previous non-sync value. + executionContext = prevExecutionContext; + setCurrentUpdatePriority(previousPriority); + ReactSharedInternals.T = prevTransition; + } + } + } + const subtreeHasLayoutEffects = (finishedWork.subtreeFlags & LayoutMask) !== NoFlags; const rootHasLayoutEffect = (finishedWork.flags & LayoutMask) !== NoFlags; diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index b364d4ec47abb..25840749a1adf 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -248,6 +248,7 @@ type BaseFiberRootProperties = { pingedLanes: Lanes, warmLanes: Lanes, expiredLanes: Lanes, + indicatorLanes: Lanes, // enableDefaultTransitionIndicator only errorRecoveryDisabledLanes: Lanes, shellSuspendCounter: number, @@ -280,7 +281,9 @@ type BaseFiberRootProperties = { errorInfo: {+componentStack?: ?string}, ) => void, + // enableDefaultTransitionIndicator only onDefaultTransitionIndicator: () => void | (() => void), + pendingIndicator: null | (() => void), formState: ReactFormState | null, diff --git a/packages/react-reconciler/src/__tests__/ReactDefaultTransitionIndicator-test.js b/packages/react-reconciler/src/__tests__/ReactDefaultTransitionIndicator-test.js new file mode 100644 index 0000000000000..bc5c3ec669512 --- /dev/null +++ b/packages/react-reconciler/src/__tests__/ReactDefaultTransitionIndicator-test.js @@ -0,0 +1,358 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + * @jest-environment node + */ + +'use strict'; + +let React; +let ReactNoop; +let Scheduler; +let act; +let use; +let useOptimistic; +let useState; +let useTransition; +let useDeferredValue; +let assertLog; +let waitForPaint; + +describe('ReactDefaultTransitionIndicator', () => { + beforeEach(() => { + jest.resetModules(); + + React = require('react'); + ReactNoop = require('react-noop-renderer'); + Scheduler = require('scheduler'); + const InternalTestUtils = require('internal-test-utils'); + act = InternalTestUtils.act; + assertLog = InternalTestUtils.assertLog; + waitForPaint = InternalTestUtils.waitForPaint; + use = React.use; + useOptimistic = React.useOptimistic; + useState = React.useState; + useTransition = React.useTransition; + useDeferredValue = React.useDeferredValue; + }); + + // @gate enableDefaultTransitionIndicator + it('triggers the default indicator while a transition is on-going', async () => { + let resolve; + const promise = new Promise(r => (resolve = r)); + function App() { + return use(promise); + } + + const root = ReactNoop.createRoot({ + onDefaultTransitionIndicator() { + Scheduler.log('start'); + return () => { + Scheduler.log('stop'); + }; + }, + }); + await act(() => { + React.startTransition(() => { + root.render(); + }); + }); + + assertLog(['start']); + + await act(async () => { + await resolve('Hello'); + }); + + assertLog(['stop']); + + expect(root).toMatchRenderedOutput('Hello'); + }); + + // @gate enableDefaultTransitionIndicator + it('does not trigger the default indicator if there is a sync mutation', async () => { + const promiseA = Promise.resolve('Hi'); + let resolveB; + const promiseB = new Promise(r => (resolveB = r)); + let update; + function App({children}) { + const [state, setState] = useState(''); + update = setState; + return ( +
+ {state} + {children} +
+ ); + } + + const root = ReactNoop.createRoot({ + onDefaultTransitionIndicator() { + Scheduler.log('start'); + return () => { + Scheduler.log('stop'); + }; + }, + }); + await act(() => { + React.startTransition(() => { + root.render({promiseA}); + }); + }); + + assertLog(['start', 'stop']); + + expect(root).toMatchRenderedOutput(
Hi
); + + await act(() => { + // TODO: This should not require a discrete update ideally but work for default too. + ReactNoop.discreteUpdates(() => { + update('Loading...'); + }); + React.startTransition(() => { + update(''); + root.render({promiseB}); + }); + }); + + assertLog([]); + + expect(root).toMatchRenderedOutput(
Loading...Hi
); + + await act(async () => { + await resolveB('Hello'); + }); + + assertLog([]); + + expect(root).toMatchRenderedOutput(
Hello
); + }); + + // @gate enableDefaultTransitionIndicator + it('does not trigger the default indicator if there is an optimistic update', async () => { + const promiseA = Promise.resolve('Hi'); + let resolveB; + const promiseB = new Promise(r => (resolveB = r)); + let update; + function App({children}) { + const [state, setOptimistic] = useOptimistic(''); + update = setOptimistic; + return ( +
+ {state} + {children} +
+ ); + } + + const root = ReactNoop.createRoot({ + onDefaultTransitionIndicator() { + Scheduler.log('start'); + return () => { + Scheduler.log('stop'); + }; + }, + }); + await act(() => { + React.startTransition(() => { + root.render({promiseA}); + }); + }); + + assertLog(['start', 'stop']); + + expect(root).toMatchRenderedOutput(
Hi
); + + await act(() => { + React.startTransition(() => { + update('Loading...'); + root.render({promiseB}); + }); + }); + + assertLog([]); + + expect(root).toMatchRenderedOutput(
Loading...Hi
); + + await act(async () => { + await resolveB('Hello'); + }); + + assertLog([]); + + expect(root).toMatchRenderedOutput(
Hello
); + }); + + // @gate enableDefaultTransitionIndicator + it('does not trigger the default indicator if there is an isPending update', async () => { + const promiseA = Promise.resolve('Hi'); + let resolveB; + const promiseB = new Promise(r => (resolveB = r)); + let start; + function App({children}) { + const [isPending, startTransition] = useTransition(); + start = startTransition; + return ( +
+ {isPending ? 'Loading...' : ''} + {children} +
+ ); + } + + const root = ReactNoop.createRoot({ + onDefaultTransitionIndicator() { + Scheduler.log('start'); + return () => { + Scheduler.log('stop'); + }; + }, + }); + await act(() => { + React.startTransition(() => { + root.render({promiseA}); + }); + }); + + assertLog(['start', 'stop']); + + expect(root).toMatchRenderedOutput(
Hi
); + + await act(() => { + start(() => { + root.render({promiseB}); + }); + }); + + assertLog([]); + + expect(root).toMatchRenderedOutput(
Loading...Hi
); + + await act(async () => { + await resolveB('Hello'); + }); + + assertLog([]); + + expect(root).toMatchRenderedOutput(
Hello
); + }); + + // @gate enableDefaultTransitionIndicator + it('triggers the default indicator while an async transition is ongoing', async () => { + let resolve; + const promise = new Promise(r => (resolve = r)); + let start; + function App() { + const [, startTransition] = useTransition(); + start = startTransition; + return 'Hi'; + } + + const root = ReactNoop.createRoot({ + onDefaultTransitionIndicator() { + Scheduler.log('start'); + return () => { + Scheduler.log('stop'); + }; + }, + }); + await act(() => { + root.render(); + }); + + assertLog([]); + + await act(() => { + // Start an async action but we haven't called setState yet + // TODO: This should ideally work with React.startTransition too but we don't know the root. + start(() => promise); + }); + + assertLog(['start']); + + await act(async () => { + await resolve('Hello'); + }); + + assertLog(['stop']); + + expect(root).toMatchRenderedOutput('Hi'); + }); + + it('should not trigger for useDeferredValue (sync)', async () => { + function Text({text}) { + Scheduler.log(text); + return text; + } + function App({value}) { + const deferredValue = useDeferredValue(value, 'Hi'); + return ; + } + + const root = ReactNoop.createRoot({ + onDefaultTransitionIndicator() { + Scheduler.log('start'); + return () => { + Scheduler.log('stop'); + }; + }, + }); + await act(async () => { + root.render(); + await waitForPaint(['Hi']); + expect(root).toMatchRenderedOutput('Hi'); + }); + + assertLog(['Hello']); + + expect(root).toMatchRenderedOutput('Hello'); + + assertLog([]); + + await act(async () => { + root.render(); + await waitForPaint(['Hello']); + expect(root).toMatchRenderedOutput('Hello'); + }); + + assertLog(['Bye']); + + expect(root).toMatchRenderedOutput('Bye'); + }); + + // @gate enableDefaultTransitionIndicator + it('should not trigger for useDeferredValue (transition)', async () => { + function Text({text}) { + Scheduler.log(text); + return text; + } + function App({value}) { + const deferredValue = useDeferredValue(value, 'Hi'); + return ; + } + + const root = ReactNoop.createRoot({ + onDefaultTransitionIndicator() { + Scheduler.log('start'); + return () => { + Scheduler.log('stop'); + }; + }, + }); + await act(async () => { + React.startTransition(() => { + root.render(); + }); + await waitForPaint(['start', 'Hi', 'stop']); + expect(root).toMatchRenderedOutput('Hi'); + }); + + assertLog(['Hello']); + + expect(root).toMatchRenderedOutput('Hello'); + }); +}); From b480865db0babfcad602a1a1909775069b5779f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 13 May 2025 15:52:44 -0400 Subject: [PATCH 08/11] [Fiber] Always flush Default priority in the microtask if a Transition was scheduled (#33186) Stacked on #33160. The purpose of this is to avoid calling `onDefaultTransitionIndicator` when a Default priority update acts as the loading indicator, but still call it when unrelated Default updates happens nearby. When we schedule Default priority work that gets batched with other events in the same frame more or less. This helps optimize by doing less work. However, that batching means that we can't separate work from one setState from another. If we would consider all Default priority work in a frame when determining whether to show the default we might never show it in cases like when you have a recurring timer updating something. This instead flushes the Default priority work eagerly along with the sync work at the end of the event, if this event scheduled any Transition work. This is then used to determine if the default indicator needs to be shown. --- packages/react-reconciler/src/ReactFiberRootScheduler.js | 8 ++++++++ .../src/__tests__/ReactDefaultTransitionIndicator-test.js | 5 +---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index 7daed077eee57..0bcf1d580c69a 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -26,6 +26,7 @@ import { NoLane, NoLanes, SyncLane, + DefaultLane, getHighestPriorityLane, getNextLanes, includesSyncLane, @@ -261,6 +262,13 @@ function processRootScheduleInMicrotask() { // render it synchronously anyway. We do this during a popstate event to // preserve the scroll position of the previous page. syncTransitionLanes = currentEventTransitionLane; + } else if (enableDefaultTransitionIndicator) { + // If we have a Transition scheduled by this event it might be paired + // with Default lane scheduled loading indicators. To unbatch it from + // other events later on, flush it early to determine whether it + // rendered an indicator. This ensures that setState in default priority + // event doesn't trigger onDefaultTransitionIndicator. + syncTransitionLanes = DefaultLane; } } diff --git a/packages/react-reconciler/src/__tests__/ReactDefaultTransitionIndicator-test.js b/packages/react-reconciler/src/__tests__/ReactDefaultTransitionIndicator-test.js index bc5c3ec669512..718480560344f 100644 --- a/packages/react-reconciler/src/__tests__/ReactDefaultTransitionIndicator-test.js +++ b/packages/react-reconciler/src/__tests__/ReactDefaultTransitionIndicator-test.js @@ -109,10 +109,7 @@ describe('ReactDefaultTransitionIndicator', () => { expect(root).toMatchRenderedOutput(
Hi
); await act(() => { - // TODO: This should not require a discrete update ideally but work for default too. - ReactNoop.discreteUpdates(() => { - update('Loading...'); - }); + update('Loading...'); React.startTransition(() => { update(''); root.render({promiseB}); From 59440424d05360cca32ca6f46ae33661f70d43e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 13 May 2025 16:00:38 -0400 Subject: [PATCH 09/11] Implement Navigation API backed default indicator for DOM renderer (#33162) Stacked on #33160. By default, if `onDefaultTransitionIndicator` is not overridden, this will trigger a fake Navigation event using the Navigation API. This is intercepted to create an on-going navigation until we complete the Transition. Basically each default Transition is simulated as a Navigation. This triggers the native browser loading state (in Chrome at least). So now by default the browser spinner spins during a Transition if no other loading state is provided. Firefox and Safari hasn't shipped Navigation API yet and even in the flag Safari has, it doesn't actually trigger the native loading state. To ensures that you can still use other Navigations concurrently, we don't start our fake Navigation if there's one on-going already. Similarly if our fake Navigation gets interrupted by another. We wait for on-going ones to finish and then start a new fake one if we're supposed to be still pending. There might be other routers on the page that might listen to intercept Navigation Events. Typically you'd expect them not to trigger a refetch when navigating to the same state. However, if they want to detect this we provide the `"react-transition"` string in the `info` field for this purpose. --- .eslintrc.js | 2 + fixtures/flight/src/actions.js | 8 ++ .../view-transition/src/components/Page.js | 12 +- .../ReactDOMDefaultTransitionIndicator.js | 89 +++++++++++++ packages/react-dom/src/client/ReactDOMRoot.js | 6 +- scripts/flow/environment.js | 124 ++++++++++++++++++ scripts/rollup/validate/eslintrc.cjs.js | 1 + scripts/rollup/validate/eslintrc.cjs2015.js | 1 + scripts/rollup/validate/eslintrc.esm.js | 1 + scripts/rollup/validate/eslintrc.fb.js | 1 + scripts/rollup/validate/eslintrc.rn.js | 1 + 11 files changed, 240 insertions(+), 6 deletions(-) create mode 100644 packages/react-dom/src/client/ReactDOMDefaultTransitionIndicator.js diff --git a/.eslintrc.js b/.eslintrc.js index 49846c1f5e9bc..c1eb5b34ebe82 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -579,6 +579,7 @@ module.exports = { JSONValue: 'readonly', JSResourceReference: 'readonly', MouseEventHandler: 'readonly', + NavigateEvent: 'readonly', PropagationPhases: 'readonly', PropertyDescriptor: 'readonly', React$AbstractComponent: 'readonly', @@ -634,5 +635,6 @@ module.exports = { AsyncLocalStorage: 'readonly', async_hooks: 'readonly', globalThis: 'readonly', + navigation: 'readonly', }, }; diff --git a/fixtures/flight/src/actions.js b/fixtures/flight/src/actions.js index aa19871a9dcbb..0b9b9c315d647 100644 --- a/fixtures/flight/src/actions.js +++ b/fixtures/flight/src/actions.js @@ -2,7 +2,13 @@ import {setServerState} from './ServerState.js'; +async function sleep(ms) { + return new Promise(resolve => setTimeout(resolve, ms)); +} + export async function like() { + // Test loading state + await sleep(1000); setServerState('Liked!'); return new Promise((resolve, reject) => resolve('Liked')); } @@ -20,5 +26,7 @@ export async function greet(formData) { } export async function increment(n) { + // Test loading state + await sleep(1000); return n + 1; } diff --git a/fixtures/view-transition/src/components/Page.js b/fixtures/view-transition/src/components/Page.js index 9744313c4f5ea..d7f7bc0983601 100644 --- a/fixtures/view-transition/src/components/Page.js +++ b/fixtures/view-transition/src/components/Page.js @@ -18,6 +18,10 @@ import './Page.css'; import transitions from './Transitions.module.css'; +async function sleep(ms) { + return new Promise(resolve => setTimeout(resolve, ms)); +} + const a = (
@@ -106,7 +110,13 @@ export default function Page({url, navigate}) { document.body ) ) : ( - ); diff --git a/packages/react-dom/src/client/ReactDOMDefaultTransitionIndicator.js b/packages/react-dom/src/client/ReactDOMDefaultTransitionIndicator.js new file mode 100644 index 0000000000000..8f1a32d826c1a --- /dev/null +++ b/packages/react-dom/src/client/ReactDOMDefaultTransitionIndicator.js @@ -0,0 +1,89 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +export function defaultOnDefaultTransitionIndicator(): void | (() => void) { + if (typeof navigation !== 'object') { + // If the Navigation API is not available, then this is a noop. + return; + } + + let isCancelled = false; + let pendingResolve: null | (() => void) = null; + + function handleNavigate(event: NavigateEvent) { + if (event.canIntercept && event.info === 'react-transition') { + event.intercept({ + handler() { + return new Promise(resolve => (pendingResolve = resolve)); + }, + focusReset: 'manual', + scroll: 'manual', + }); + } + } + + function handleNavigateComplete() { + if (pendingResolve !== null) { + // If this was not our navigation completing, we were probably cancelled. + // We'll start a new one below. + pendingResolve(); + pendingResolve = null; + } + if (!isCancelled) { + // Some other navigation completed but we should still be running. + // Start another fake one to keep the loading indicator going. + startFakeNavigation(); + } + } + + // $FlowFixMe + navigation.addEventListener('navigate', handleNavigate); + // $FlowFixMe + navigation.addEventListener('navigatesuccess', handleNavigateComplete); + // $FlowFixMe + navigation.addEventListener('navigateerror', handleNavigateComplete); + + function startFakeNavigation() { + if (isCancelled) { + // We already stopped this Transition. + return; + } + if (navigation.transition) { + // There is an on-going Navigation already happening. Let's wait for it to + // finish before starting our fake one. + return; + } + // Trigger a fake navigation to the same page + const currentEntry = navigation.currentEntry; + if (currentEntry && currentEntry.url != null) { + navigation.navigate(currentEntry.url, { + state: currentEntry.getState(), + info: 'react-transition', // indicator to routers to ignore this navigation + history: 'replace', + }); + } + } + + // Delay the start a bit in case this is a fast navigation. + setTimeout(startFakeNavigation, 100); + + return function () { + isCancelled = true; + // $FlowFixMe + navigation.removeEventListener('navigate', handleNavigate); + // $FlowFixMe + navigation.removeEventListener('navigatesuccess', handleNavigateComplete); + // $FlowFixMe + navigation.removeEventListener('navigateerror', handleNavigateComplete); + if (pendingResolve !== null) { + pendingResolve(); + pendingResolve = null; + } + }; +} diff --git a/packages/react-dom/src/client/ReactDOMRoot.js b/packages/react-dom/src/client/ReactDOMRoot.js index ef2c9ddf193eb..97f4c83515364 100644 --- a/packages/react-dom/src/client/ReactDOMRoot.js +++ b/packages/react-dom/src/client/ReactDOMRoot.js @@ -95,13 +95,9 @@ import { defaultOnCaughtError, defaultOnRecoverableError, } from 'react-reconciler/src/ReactFiberReconciler'; +import {defaultOnDefaultTransitionIndicator} from './ReactDOMDefaultTransitionIndicator'; import {ConcurrentRoot} from 'react-reconciler/src/ReactRootTags'; -function defaultOnDefaultTransitionIndicator(): void | (() => void) { - // TODO: Implement the default - return function () {}; -} - // $FlowFixMe[missing-this-annot] function ReactDOMRoot(internalRoot: FiberRoot) { this._internalRoot = internalRoot; diff --git a/scripts/flow/environment.js b/scripts/flow/environment.js index c3fe40eeef302..d66ef65d9d318 100644 --- a/scripts/flow/environment.js +++ b/scripts/flow/environment.js @@ -429,3 +429,127 @@ declare const Bun: { input: string | $TypedArray | DataView | ArrayBuffer | SharedArrayBuffer, ): number, }; + +// Navigation API + +declare const navigation: Navigation; + +interface NavigationResult { + committed: Promise; + finished: Promise; +} + +declare class Navigation extends EventTarget { + entries(): NavigationHistoryEntry[]; + +currentEntry: NavigationHistoryEntry | null; + updateCurrentEntry(options: NavigationUpdateCurrentEntryOptions): void; + +transition: NavigationTransition | null; + + +canGoBack: boolean; + +canGoForward: boolean; + + navigate(url: string, options?: NavigationNavigateOptions): NavigationResult; + reload(options?: NavigationReloadOptions): NavigationResult; + + traverseTo(key: string, options?: NavigationOptions): NavigationResult; + back(options?: NavigationOptions): NavigationResult; + forward(options?: NavigationOptions): NavigationResult; + + onnavigate: ((this: Navigation, ev: NavigateEvent) => any) | null; + onnavigatesuccess: ((this: Navigation, ev: Event) => any) | null; + onnavigateerror: ((this: Navigation, ev: ErrorEvent) => any) | null; + oncurrententrychange: + | ((this: Navigation, ev: NavigationCurrentEntryChangeEvent) => any) + | null; + + // TODO: Implement addEventListener overrides. Doesn't seem like Flow supports this. +} + +declare class NavigationTransition { + +navigationType: NavigationTypeString; + +from: NavigationHistoryEntry; + +finished: Promise; +} + +interface NavigationHistoryEntryEventMap { + dispose: Event; +} + +interface NavigationHistoryEntry extends EventTarget { + +key: string; + +id: string; + +url: string | null; + +index: number; + +sameDocument: boolean; + + getState(): mixed; + + ondispose: ((this: NavigationHistoryEntry, ev: Event) => any) | null; + + // TODO: Implement addEventListener overrides. Doesn't seem like Flow supports this. +} + +declare var NavigationHistoryEntry: { + prototype: NavigationHistoryEntry, + new(): NavigationHistoryEntry, +}; + +type NavigationTypeString = 'reload' | 'push' | 'replace' | 'traverse'; + +interface NavigationUpdateCurrentEntryOptions { + state: mixed; +} + +interface NavigationOptions { + info?: mixed; +} + +interface NavigationNavigateOptions extends NavigationOptions { + state?: mixed; + history?: 'auto' | 'push' | 'replace'; +} + +interface NavigationReloadOptions extends NavigationOptions { + state?: mixed; +} + +declare class NavigationCurrentEntryChangeEvent extends Event { + constructor(type: string, eventInit?: any): void; + + +navigationType: NavigationTypeString | null; + +from: NavigationHistoryEntry; +} + +declare class NavigateEvent extends Event { + constructor(type: string, eventInit?: any): void; + + +navigationType: NavigationTypeString; + +canIntercept: boolean; + +userInitiated: boolean; + +hashChange: boolean; + +hasUAVisualTransition: boolean; + +destination: NavigationDestination; + +signal: AbortSignal; + +formData: FormData | null; + +downloadRequest: string | null; + +info?: mixed; + + intercept(options?: NavigationInterceptOptions): void; + scroll(): void; +} + +interface NavigationInterceptOptions { + handler?: () => Promise; + focusReset?: 'after-transition' | 'manual'; + scroll?: 'after-transition' | 'manual'; +} + +declare class NavigationDestination { + +url: string; + +key: string | null; + +id: string | null; + +index: number; + +sameDocument: boolean; + + getState(): mixed; +} diff --git a/scripts/rollup/validate/eslintrc.cjs.js b/scripts/rollup/validate/eslintrc.cjs.js index 88d17772d7b21..65fd6129904e6 100644 --- a/scripts/rollup/validate/eslintrc.cjs.js +++ b/scripts/rollup/validate/eslintrc.cjs.js @@ -35,6 +35,7 @@ module.exports = { FinalizationRegistry: 'readonly', ScrollTimeline: 'readonly', + navigation: 'readonly', // Vendor specific MSApp: 'readonly', diff --git a/scripts/rollup/validate/eslintrc.cjs2015.js b/scripts/rollup/validate/eslintrc.cjs2015.js index 8e87c8dbe0203..fa0b471330f4a 100644 --- a/scripts/rollup/validate/eslintrc.cjs2015.js +++ b/scripts/rollup/validate/eslintrc.cjs2015.js @@ -33,6 +33,7 @@ module.exports = { globalThis: 'readonly', FinalizationRegistry: 'readonly', ScrollTimeline: 'readonly', + navigation: 'readonly', // Vendor specific MSApp: 'readonly', __REACT_DEVTOOLS_GLOBAL_HOOK__: 'readonly', diff --git a/scripts/rollup/validate/eslintrc.esm.js b/scripts/rollup/validate/eslintrc.esm.js index 8b4bba35796fa..a5ea7afb972e7 100644 --- a/scripts/rollup/validate/eslintrc.esm.js +++ b/scripts/rollup/validate/eslintrc.esm.js @@ -35,6 +35,7 @@ module.exports = { FinalizationRegistry: 'readonly', ScrollTimeline: 'readonly', + navigation: 'readonly', // Vendor specific MSApp: 'readonly', diff --git a/scripts/rollup/validate/eslintrc.fb.js b/scripts/rollup/validate/eslintrc.fb.js index f0602e79e5074..afee2f1199eb1 100644 --- a/scripts/rollup/validate/eslintrc.fb.js +++ b/scripts/rollup/validate/eslintrc.fb.js @@ -35,6 +35,7 @@ module.exports = { FinalizationRegistry: 'readonly', ScrollTimeline: 'readonly', + navigation: 'readonly', // Vendor specific MSApp: 'readonly', diff --git a/scripts/rollup/validate/eslintrc.rn.js b/scripts/rollup/validate/eslintrc.rn.js index 052edabdc0f92..2420898bebecd 100644 --- a/scripts/rollup/validate/eslintrc.rn.js +++ b/scripts/rollup/validate/eslintrc.rn.js @@ -35,6 +35,7 @@ module.exports = { FinalizationRegistry: 'readonly', ScrollTimeline: 'readonly', + navigation: 'readonly', // Vendor specific MSApp: 'readonly', From 3a5b326d8180f005a10e34a07ded6d5632efe337 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 13 May 2025 16:10:28 -0400 Subject: [PATCH 10/11] [Fiber] Trigger default indicator for isomorphic async actions with no root associated (#33190) Stacked on #33160, #33162, #33186 and #33188. We have a special case that's awkward for default indicators. When you start a new async Transition from `React.startTransition` then there's not yet any associated root with the Transition because you haven't necessarily `setState` on anything yet until the promise resolves. That's what `entangleAsyncAction` handles by creating a lane that everything entangles with until all async actions are done. If there are no sync updates before the end of the event, we should trigger a default indicator until either the async action completes without update or if it gets entangled with some roots we should keep it going until those roots are done. --- .../view-transition/src/components/Page.js | 2 +- .../src/ReactFiberAsyncAction.js | 102 +++++++++++++- .../src/ReactFiberReconciler.js | 7 +- .../src/ReactFiberRootScheduler.js | 56 +++++--- .../ReactDefaultTransitionIndicator-test.js | 127 +++++++++++++++++- 5 files changed, 274 insertions(+), 20 deletions(-) diff --git a/fixtures/view-transition/src/components/Page.js b/fixtures/view-transition/src/components/Page.js index d7f7bc0983601..db2cd0aff08c7 100644 --- a/fixtures/view-transition/src/components/Page.js +++ b/fixtures/view-transition/src/components/Page.js @@ -113,8 +113,8 @@ export default function Page({url, navigate}) {