From 4c6967be290fc31182c61cfdac19915fdb16aa60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 20 May 2025 09:39:25 -0400 Subject: [PATCH 1/3] [Fiber] Support AsyncIterable children in SuspenseList (#33299) We support AsyncIterable (more so when it's a cached form like in coming from Flight) as children. This fixes some warnings and bugs when passed to SuspenseList. Ideally SuspenseList with `tail="hidden"` should support unblocking before the full result has resolved but that's an optimization on top. We also might want to change semantics for this for `revealOrder="backwards"` so it becomes possible to stream items in reverse order. --- .../react-reconciler/src/ReactChildFiber.js | 101 +++++++++ .../src/ReactFiberBeginWork.js | 89 ++------ .../src/__tests__/ReactSuspenseList-test.js | 193 ++++++++++++++++++ 3 files changed, 306 insertions(+), 77 deletions(-) diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index 6af8c1356f9ca..fcb2406552899 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -13,6 +13,7 @@ import type { Thenable, ReactContext, ReactDebugInfo, + SuspenseListRevealOrder, } from 'shared/ReactTypes'; import type {Fiber} from './ReactInternalTypes'; import type {Lanes} from './ReactFiberLane'; @@ -2057,3 +2058,103 @@ export function resetChildFibers(workInProgress: Fiber, lanes: Lanes): void { child = child.sibling; } } + +function validateSuspenseListNestedChild(childSlot: mixed, index: number) { + if (__DEV__) { + const isAnArray = isArray(childSlot); + const isIterable = + !isAnArray && typeof getIteratorFn(childSlot) === 'function'; + const isAsyncIterable = + enableAsyncIterableChildren && + typeof childSlot === 'object' && + childSlot !== null && + typeof (childSlot: any)[ASYNC_ITERATOR] === 'function'; + if (isAnArray || isIterable || isAsyncIterable) { + const type = isAnArray + ? 'array' + : isAsyncIterable + ? 'async iterable' + : 'iterable'; + console.error( + 'A nested %s was passed to row #%s in . Wrap it in ' + + 'an additional SuspenseList to configure its revealOrder: ' + + ' ... ' + + '{%s} ... ' + + '', + type, + index, + type, + ); + return false; + } + } + return true; +} + +export function validateSuspenseListChildren( + children: mixed, + revealOrder: SuspenseListRevealOrder, +) { + if (__DEV__) { + if ( + (revealOrder === 'forwards' || revealOrder === 'backwards') && + children !== undefined && + children !== null && + children !== false + ) { + if (isArray(children)) { + for (let i = 0; i < children.length; i++) { + if (!validateSuspenseListNestedChild(children[i], i)) { + return; + } + } + } else { + const iteratorFn = getIteratorFn(children); + if (typeof iteratorFn === 'function') { + const childrenIterator = iteratorFn.call(children); + if (childrenIterator) { + let step = childrenIterator.next(); + let i = 0; + for (; !step.done; step = childrenIterator.next()) { + if (!validateSuspenseListNestedChild(step.value, i)) { + return; + } + i++; + } + } + } else if ( + enableAsyncIterableChildren && + typeof (children: any)[ASYNC_ITERATOR] === 'function' + ) { + // TODO: Technically we should warn for nested arrays inside the + // async iterable but it would require unwrapping the array. + // However, this mistake is not as easy to make so it's ok not to warn. + } else if ( + enableAsyncIterableChildren && + children.$$typeof === REACT_ELEMENT_TYPE && + typeof children.type === 'function' && + // $FlowFixMe + (Object.prototype.toString.call(children.type) === + '[object GeneratorFunction]' || + // $FlowFixMe + Object.prototype.toString.call(children.type) === + '[object AsyncGeneratorFunction]') + ) { + console.error( + 'A generator Component was passed to a . ' + + 'This is not supported as a way to generate lists. Instead, pass an ' + + 'iterable as the children.', + revealOrder, + ); + } else { + console.error( + 'A single row was passed to a . ' + + 'This is not useful since it needs multiple rows. ' + + 'Did you mean to pass multiple children or an array?', + revealOrder, + ); + } + } + } + } +} diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 69bc84038dac9..7b86962f778fe 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -123,7 +123,6 @@ import { enableViewTransition, enableFragmentRefs, } from 'shared/ReactFeatureFlags'; -import isArray from 'shared/isArray'; import shallowEqual from 'shared/shallowEqual'; import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; import getComponentNameFromType from 'shared/getComponentNameFromType'; @@ -132,7 +131,6 @@ import { REACT_LAZY_TYPE, REACT_FORWARD_REF_TYPE, REACT_MEMO_TYPE, - getIteratorFn, } from 'shared/ReactSymbols'; import {setCurrentFiber} from './ReactCurrentFiber'; import { @@ -145,6 +143,7 @@ import { mountChildFibers, reconcileChildFibers, cloneChildFibers, + validateSuspenseListChildren, } from './ReactChildFiber'; import { processUpdateQueue, @@ -3302,73 +3301,6 @@ function validateTailOptions( } } -function validateSuspenseListNestedChild(childSlot: mixed, index: number) { - if (__DEV__) { - const isAnArray = isArray(childSlot); - const isIterable = - !isAnArray && typeof getIteratorFn(childSlot) === 'function'; - if (isAnArray || isIterable) { - const type = isAnArray ? 'array' : 'iterable'; - console.error( - 'A nested %s was passed to row #%s in . Wrap it in ' + - 'an additional SuspenseList to configure its revealOrder: ' + - ' ... ' + - '{%s} ... ' + - '', - type, - index, - type, - ); - return false; - } - } - return true; -} - -function validateSuspenseListChildren( - children: mixed, - revealOrder: SuspenseListRevealOrder, -) { - if (__DEV__) { - if ( - (revealOrder === 'forwards' || revealOrder === 'backwards') && - children !== undefined && - children !== null && - children !== false - ) { - if (isArray(children)) { - for (let i = 0; i < children.length; i++) { - if (!validateSuspenseListNestedChild(children[i], i)) { - return; - } - } - } else { - const iteratorFn = getIteratorFn(children); - if (typeof iteratorFn === 'function') { - const childrenIterator = iteratorFn.call(children); - if (childrenIterator) { - let step = childrenIterator.next(); - let i = 0; - for (; !step.done; step = childrenIterator.next()) { - if (!validateSuspenseListNestedChild(step.value, i)) { - return; - } - i++; - } - } - } else { - console.error( - 'A single row was passed to a . ' + - 'This is not useful since it needs multiple rows. ' + - 'Did you mean to pass multiple children or an array?', - revealOrder, - ); - } - } - } - } -} - function initSuspenseListRenderState( workInProgress: Fiber, isBackwards: boolean, @@ -3415,12 +3347,6 @@ function updateSuspenseListComponent( const tailMode: SuspenseListTailMode = nextProps.tail; const newChildren = nextProps.children; - validateRevealOrder(revealOrder); - validateTailOptions(tailMode, revealOrder); - validateSuspenseListChildren(newChildren, revealOrder); - - reconcileChildren(current, workInProgress, newChildren, renderLanes); - let suspenseContext: SuspenseContext = suspenseStackCursor.current; const shouldForceFallback = hasSuspenseListContext( @@ -3434,6 +3360,17 @@ function updateSuspenseListComponent( ); workInProgress.flags |= DidCapture; } else { + suspenseContext = setDefaultShallowSuspenseListContext(suspenseContext); + } + pushSuspenseListContext(workInProgress, suspenseContext); + + validateRevealOrder(revealOrder); + validateTailOptions(tailMode, revealOrder); + validateSuspenseListChildren(newChildren, revealOrder); + + reconcileChildren(current, workInProgress, newChildren, renderLanes); + + if (!shouldForceFallback) { const didSuspendBefore = current !== null && (current.flags & DidCapture) !== NoFlags; if (didSuspendBefore) { @@ -3446,9 +3383,7 @@ function updateSuspenseListComponent( renderLanes, ); } - suspenseContext = setDefaultShallowSuspenseListContext(suspenseContext); } - pushSuspenseListContext(workInProgress, suspenseContext); if (!disableLegacyMode && (workInProgress.mode & ConcurrentMode) === NoMode) { // In legacy mode, SuspenseList doesn't work so we just diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js index 6faeae3acba0d..f9efb330cf891 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js @@ -3119,4 +3119,197 @@ describe('ReactSuspenseList', () => { ); }, ); + + // @gate enableSuspenseList && enableAsyncIterableChildren + it('warns for async generator components in "forwards" order', async () => { + async function* Generator() { + yield 'A'; + yield 'B'; + } + function Foo() { + return ( + + + + ); + } + + await act(() => { + React.startTransition(() => { + ReactNoop.render(); + }); + }); + assertConsoleErrorDev([ + 'A generator Component was passed to a . ' + + 'This is not supported as a way to generate lists. Instead, pass an ' + + 'iterable as the children.' + + '\n in SuspenseList (at **)' + + '\n in Foo (at **)', + ' is an async Client Component. ' + + 'Only Server Components can be async at the moment. ' + + "This error is often caused by accidentally adding `'use client'` " + + 'to a module that was originally written for the server.\n' + + ' in Foo (at **)', + // We get this warning because the generator's promise themselves are not cached. + 'A component was suspended by an uncached promise. ' + + 'Creating promises inside a Client Component or hook is not yet supported, ' + + 'except via a Suspense-compatible library or framework.\n' + + ' in Foo (at **)', + ]); + }); + + // @gate enableSuspenseList && enableAsyncIterableChildren + it('can display async iterable in "forwards" order', async () => { + const A = createAsyncText('A'); + const B = createAsyncText('B'); + + // We use Cached elements to avoid rerender. + const ASlot = ( + }> + + + ); + + const BSlot = ( + }> + + + ); + + const iterable = { + async *[Symbol.asyncIterator]() { + yield ASlot; + yield BSlot; + }, + }; + + function Foo() { + return {iterable}; + } + + await act(() => { + React.startTransition(() => { + ReactNoop.render(); + }); + }); + + assertLog([ + 'Suspend! [A]', + 'Loading A', + 'Loading B', + // pre-warming + 'Suspend! [A]', + ]); + + assertConsoleErrorDev([ + // We get this warning because the generator's promise themselves are not cached. + 'A component was suspended by an uncached promise. ' + + 'Creating promises inside a Client Component or hook is not yet supported, ' + + 'except via a Suspense-compatible library or framework.\n' + + ' in SuspenseList (at **)\n' + + ' in Foo (at **)', + 'A component was suspended by an uncached promise. ' + + 'Creating promises inside a Client Component or hook is not yet supported, ' + + 'except via a Suspense-compatible library or framework.\n' + + ' in SuspenseList (at **)\n' + + ' in Foo (at **)', + ]); + + expect(ReactNoop).toMatchRenderedOutput( + <> + Loading A + Loading B + , + ); + + await act(() => A.resolve()); + assertLog(['A', 'Suspend! [B]', 'Suspend! [B]']); + + assertConsoleErrorDev([ + // We get this warning because the generator's promise themselves are not cached. + 'A component was suspended by an uncached promise. ' + + 'Creating promises inside a Client Component or hook is not yet supported, ' + + 'except via a Suspense-compatible library or framework.\n' + + ' in SuspenseList (at **)\n' + + ' in Foo (at **)', + 'A component was suspended by an uncached promise. ' + + 'Creating promises inside a Client Component or hook is not yet supported, ' + + 'except via a Suspense-compatible library or framework.\n' + + ' in SuspenseList (at **)\n' + + ' in Foo (at **)', + ]); + + expect(ReactNoop).toMatchRenderedOutput( + <> + A + Loading B + , + ); + + await act(() => B.resolve()); + assertLog(['B']); + + assertConsoleErrorDev([ + // We get this warning because the generator's promise themselves are not cached. + 'A component was suspended by an uncached promise. ' + + 'Creating promises inside a Client Component or hook is not yet supported, ' + + 'except via a Suspense-compatible library or framework.\n' + + ' in SuspenseList (at **)\n' + + ' in Foo (at **)', + ]); + + expect(ReactNoop).toMatchRenderedOutput( + <> + A + B + , + ); + }); + + // @gate enableSuspenseList && enableAsyncIterableChildren + it('warns if a nested async iterable is passed to a "forwards" list', async () => { + function Foo({items}) { + return ( + + {items} +
Tail
+ + ); + } + + const iterable = { + async *[Symbol.asyncIterator]() { + yield ( + + A + + ); + yield ( + + B + + ); + }, + }; + + await act(() => { + React.startTransition(() => { + ReactNoop.render(); + }); + }); + assertConsoleErrorDev([ + 'A nested async iterable was passed to row #0 in . ' + + 'Wrap it in an additional SuspenseList to configure its revealOrder: ' + + ' ... ' + + '{async iterable} ... ' + + '' + + '\n in SuspenseList (at **)' + + '\n in Foo (at **)', + // We get this warning because the generator's promise themselves are not cached. + 'A component was suspended by an uncached promise. ' + + 'Creating promises inside a Client Component or hook is not yet supported, ' + + 'except via a Suspense-compatible library or framework.\n' + + ' in Foo (at **)', + ]); + }); }); From c4676e72a630f3e93634c2b004b3be07b17a79c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 20 May 2025 09:39:46 -0400 Subject: [PATCH 2/3] [Fizz] Handle nested SuspenseList (#33308) Follow up to #33306. If we're nested inside a SuspenseList and we have a row, then we can point our last row to block the parent row and unblock the parent when the last child unblocks. --- .../ReactDOMFizzSuspenseList-test.js | 73 +++++++++++++++++++ packages/react-server/src/ReactFizzServer.js | 16 +++- 2 files changed, 85 insertions(+), 4 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js index 90ced7d677dbb..3a2d854d33275 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js @@ -324,4 +324,77 @@ describe('ReactDOMFizSuspenseList', () => { , ); }); + + // @gate enableSuspenseList + it('waits for a nested SuspenseList to complete before resolving "forwards"', async () => { + const A = createAsyncText('A'); + const B = createAsyncText('B'); + const C = createAsyncText('C'); + + function Foo() { + return ( + + ); + } + + await C.resolve(); + + await serverAct(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(); + pipe(writable); + }); + + assertLog([ + 'Suspend! [B]', + 'Suspend! [A]', + 'C', + 'Loading B', + 'Loading A', + 'Loading C', + ]); + + expect(getVisibleChildren(container)).toEqual( +
+ Loading A + Loading B + Loading C +
, + ); + + await serverAct(() => A.resolve()); + assertLog(['A']); + + expect(getVisibleChildren(container)).toEqual( +
+ Loading A + Loading B + Loading C +
, + ); + + await serverAct(() => B.resolve()); + assertLog(['B']); + + expect(getVisibleChildren(container)).toEqual( +
+ A + B + C +
, + ); + }); }); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index b8f5c1be75db0..70e61f42c6e8e 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -1732,12 +1732,12 @@ function renderSuspenseListRows( const prevRow = task.row; const totalChildren = rows.length; + let previousSuspenseListRow: null | SuspenseListRow = null; if (task.replay !== null) { // Replay // First we need to check if we have any resume slots at this level. const resumeSlots = task.replay.slots; if (resumeSlots !== null && typeof resumeSlots === 'object') { - let previousSuspenseListRow: null | SuspenseListRow = null; for (let n = 0; n < totalChildren; n++) { // Since we are going to resume into a slot whose order was already // determined by the prerender, we can safely resume it even in reverse @@ -1763,7 +1763,6 @@ function renderSuspenseListRows( } } } else { - let previousSuspenseListRow: null | SuspenseListRow = null; for (let n = 0; n < totalChildren; n++) { // Since we are going to resume into a slot whose order was already // determined by the prerender, we can safely resume it even in reverse @@ -1787,7 +1786,6 @@ function renderSuspenseListRows( task = ((task: any): RenderTask); // Refined if (revealOrder !== 'backwards') { // Forwards direction - let previousSuspenseListRow: null | SuspenseListRow = null; for (let i = 0; i < totalChildren; i++) { const node = rows[i]; if (__DEV__) { @@ -1809,7 +1807,6 @@ function renderSuspenseListRows( const parentSegment = task.blockedSegment; const childIndex = parentSegment.children.length; const insertionIndex = parentSegment.chunks.length; - let previousSuspenseListRow: null | SuspenseListRow = null; for (let i = totalChildren - 1; i >= 0; i--) { const node = rows[i]; task.row = previousSuspenseListRow = createSuspenseListRow( @@ -1859,6 +1856,17 @@ function renderSuspenseListRows( } } + if ( + prevRow !== null && + previousSuspenseListRow !== null && + previousSuspenseListRow.pendingTasks > 0 + ) { + // If we are part of an outer SuspenseList and our last row is still pending, then that blocks + // the parent row from completing. We can continue the chain. + prevRow.pendingTasks++; + previousSuspenseListRow.next = prevRow; + } + // Because this context is always set right before rendering every child, we // only need to reset it to the previous value at the very end. task.treeContext = prevTreeContext; From d38c7e10d3625c550744ce36c623a73c15c2b5d8 Mon Sep 17 00:00:00 2001 From: Jan Kassens Date: Tue, 20 May 2025 12:20:51 -0400 Subject: [PATCH 3/3] Remove leftover Rust script (#33314) For now we removed Rust from the codebase, remove this leftover script. Also remove some dupes and Rust related files from `.gitignore`. --- compiler/.gitignore | 14 -------------- compiler/scripts/rustfmt.sh | 12 ------------ 2 files changed, 26 deletions(-) delete mode 100755 compiler/scripts/rustfmt.sh diff --git a/compiler/.gitignore b/compiler/.gitignore index d41f59333aa09..77e4c01bef707 100644 --- a/compiler/.gitignore +++ b/compiler/.gitignore @@ -1,28 +1,14 @@ .DS_Store .spr.yml -# Generated by Cargo -# will have compiled files and executables -debug/ -target/ - -# These are backup files generated by rustfmt -**/*.rs.bk - -# MSVC Windows builds of rustc generate these, which store debugging information -*.pdb - node_modules .watchmanconfig .watchman-cookie-* dist .vscode !packages/playground/.vscode -.spr.yml testfilter.txt -bundle-oss.sh - # forgive *.vsix .vscode-test diff --git a/compiler/scripts/rustfmt.sh b/compiler/scripts/rustfmt.sh deleted file mode 100755 index 2fb09ae324668..0000000000000 --- a/compiler/scripts/rustfmt.sh +++ /dev/null @@ -1,12 +0,0 @@ -#!/usr/bin/env bash -# 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. - -set -eo pipefail - -# Executes rustfmt using a nightly build of the compiler -# NOTE: this command must exactly match the Rust Lint command in .github/workflows/rust.yml -rustup toolchain list | grep -q nightly-2023-08-01 || (echo "Expected Rust version missing, try running: 'rustup toolchain install nightly-2023-08-01'" && exit 1) -grep -r --include "*.rs" --files-without-match "@generated" crates | xargs rustup run nightly-2023-08-01 rustfmt --config="skip_children=true" "$@"