diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts index ecc22365dd03c..4d9ce6becc17a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts @@ -1078,6 +1078,12 @@ function lowerStatement( const left = stmt.get('left'); const leftLoc = left.node.loc ?? GeneratedSource; let test: Place; + const advanceIterator = lowerValueToTemporary(builder, { + kind: 'IteratorNext', + loc: leftLoc, + iterator: {...iterator}, + collection: {...value}, + }); if (left.isVariableDeclaration()) { const declarations = left.get('declarations'); CompilerError.invariant(declarations.length === 1, { @@ -1087,12 +1093,6 @@ function lowerStatement( suggestions: null, }); const id = declarations[0].get('id'); - const advanceIterator = lowerValueToTemporary(builder, { - kind: 'IteratorNext', - loc: leftLoc, - iterator: {...iterator}, - collection: {...value}, - }); const assign = lowerAssignment( builder, leftLoc, @@ -1103,13 +1103,19 @@ function lowerStatement( ); test = lowerValueToTemporary(builder, assign); } else { - builder.errors.push({ - reason: `(BuildHIR::lowerStatement) Handle ${left.type} inits in ForOfStatement`, - severity: ErrorSeverity.Todo, - loc: left.node.loc ?? null, - suggestions: null, + CompilerError.invariant(left.isLVal(), { + loc: leftLoc, + reason: 'Expected ForOf init to be a variable declaration or lval', }); - return; + const assign = lowerAssignment( + builder, + leftLoc, + InstructionKind.Reassign, + left, + advanceIterator, + 'Assignment', + ); + test = lowerValueToTemporary(builder, assign); } builder.terminateWithContinuation( { @@ -1166,6 +1172,11 @@ function lowerStatement( const left = stmt.get('left'); const leftLoc = left.node.loc ?? GeneratedSource; let test: Place; + const nextPropertyTemp = lowerValueToTemporary(builder, { + kind: 'NextPropertyOf', + loc: leftLoc, + value, + }); if (left.isVariableDeclaration()) { const declarations = left.get('declarations'); CompilerError.invariant(declarations.length === 1, { @@ -1175,11 +1186,6 @@ function lowerStatement( suggestions: null, }); const id = declarations[0].get('id'); - const nextPropertyTemp = lowerValueToTemporary(builder, { - kind: 'NextPropertyOf', - loc: leftLoc, - value, - }); const assign = lowerAssignment( builder, leftLoc, @@ -1190,13 +1196,19 @@ function lowerStatement( ); test = lowerValueToTemporary(builder, assign); } else { - builder.errors.push({ - reason: `(BuildHIR::lowerStatement) Handle ${left.type} inits in ForInStatement`, - severity: ErrorSeverity.Todo, - loc: left.node.loc ?? null, - suggestions: null, + CompilerError.invariant(left.isLVal(), { + loc: leftLoc, + reason: 'Expected ForIn init to be a variable declaration or lval', }); - return; + const assign = lowerAssignment( + builder, + leftLoc, + InstructionKind.Reassign, + left, + nextPropertyTemp, + 'Assignment', + ); + test = lowerValueToTemporary(builder, assign); } builder.terminateWithContinuation( { diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts index 167db6dedeccc..b2f1b9e6d4edc 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -1354,20 +1354,6 @@ function codegenForInit( init: ReactiveValue, ): t.Expression | t.VariableDeclaration | null { if (init.kind === 'SequenceExpression') { - for (const instr of init.instructions) { - if (instr.value.kind === 'DeclareContext') { - CompilerError.throwTodo({ - reason: `Support for loops where the index variable is a context variable`, - loc: instr.loc, - description: - instr.value.lvalue.place.identifier.name != null - ? `\`${instr.value.lvalue.place.identifier.name.value}\` is a context variable` - : null, - suggestions: null, - }); - } - } - const body = codegenBlock( cx, init.instructions.map(instruction => ({ @@ -1378,20 +1364,33 @@ function codegenForInit( const declarators: Array = []; let kind: 'let' | 'const' = 'const'; body.forEach(instr => { - CompilerError.invariant( - instr.type === 'VariableDeclaration' && - (instr.kind === 'let' || instr.kind === 'const'), - { - reason: 'Expected a variable declaration', - loc: init.loc, - description: `Got ${instr.type}`, - suggestions: null, - }, - ); - if (instr.kind === 'let') { - kind = 'let'; + let top: undefined | t.VariableDeclarator = undefined; + if ( + instr.type === 'ExpressionStatement' && + instr.expression.type === 'AssignmentExpression' && + instr.expression.operator === '=' && + instr.expression.left.type === 'Identifier' && + (top = declarators.at(-1))?.id.type === 'Identifier' && + top?.id.name === instr.expression.left.name && + top?.init == null + ) { + top.init = instr.expression.right; + } else { + CompilerError.invariant( + instr.type === 'VariableDeclaration' && + (instr.kind === 'let' || instr.kind === 'const'), + { + reason: 'Expected a variable declaration', + loc: init.loc, + description: `Got ${instr.type}`, + suggestions: null, + }, + ); + if (instr.kind === 'let') { + kind = 'let'; + } + declarators.push(...instr.declarations); } - declarators.push(...instr.declarations); }); CompilerError.invariant(declarators.length > 0, { reason: 'Expected a variable declaration', diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-for-loop-with-context-variable-iterator.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-for-loop-with-context-variable-iterator.expect.md deleted file mode 100644 index fd03115be1021..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-for-loop-with-context-variable-iterator.expect.md +++ /dev/null @@ -1,31 +0,0 @@ - -## Input - -```javascript -function Component() { - const data = useData(); - const items = []; - // NOTE: `i` is a context variable because it's reassigned and also referenced - // within a closure, the `onClick` handler of each item - for (let i = MIN; i <= MAX; i += INCREMENT) { - items.push( data.set(i)} />); - } - return items; -} - -``` - - -## Error - -``` - 4 | // NOTE: `i` is a context variable because it's reassigned and also referenced - 5 | // within a closure, the `onClick` handler of each item -> 6 | for (let i = MIN; i <= MAX; i += INCREMENT) { - | ^^^^^^^^^^^ Todo: Support for loops where the index variable is a context variable. `i` is a context variable (6:6) - 7 | items.push( data.set(i)} />); - 8 | } - 9 | return items; -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-kitchensink.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-kitchensink.expect.md index d8360455bb303..776b16091327d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-kitchensink.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-kitchensink.expect.md @@ -98,12 +98,6 @@ Todo: (BuildHIR::lowerExpression) Handle tagged template with interpolations (30 Todo: (BuildHIR::lowerExpression) Handle tagged template where cooked value is different from raw value (34:34) -Todo: (BuildHIR::lowerStatement) Handle Identifier inits in ForOfStatement (36:36) - -Todo: (BuildHIR::lowerStatement) Handle ArrayPattern inits in ForOfStatement (38:38) - -Todo: (BuildHIR::lowerStatement) Handle ObjectPattern inits in ForOfStatement (40:40) - Todo: (BuildHIR::node.lowerReorderableExpression) Expression type `MemberExpression` cannot be safely reordered (57:57) Todo: (BuildHIR::node.lowerReorderableExpression) Expression type `BinaryExpression` cannot be safely reordered (53:53) diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/for-loop-with-context-variable-iterator.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/for-loop-with-context-variable-iterator.expect.md new file mode 100644 index 0000000000000..d92e1919625d6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/for-loop-with-context-variable-iterator.expect.md @@ -0,0 +1,78 @@ + +## Input + +```javascript +function Component() { + const data = useData(); + const items = []; + // NOTE: `i` is a context variable because it's reassigned and also referenced + // within a closure, the `onClick` handler of each item + for (let i = MIN; i <= MAX; i += INCREMENT) { + items.push(
data.set(i)} />); + } + return <>{items}; +} + +const MIN = 0; +const MAX = 3; +const INCREMENT = 1; + +function useData() { + return new Map(); +} + +export const FIXTURE_ENTRYPOINT = { + params: [], + fn: Component, +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function Component() { + const $ = _c(2); + const data = useData(); + let t0; + if ($[0] !== data) { + const items = []; + for (let i = MIN; i <= MAX; i = i + INCREMENT, i) { + items.push(
data.set(i)} />); + } + + t0 = <>{items}; + $[0] = data; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +const MIN = 0; +const MAX = 3; +const INCREMENT = 1; + +function useData() { + const $ = _c(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = new Map(); + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + params: [], + fn: Component, +}; + +``` + +### Eval output +(kind: ok)
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-for-loop-with-context-variable-iterator.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/for-loop-with-context-variable-iterator.js similarity index 51% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-for-loop-with-context-variable-iterator.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/for-loop-with-context-variable-iterator.js index 5330874b89bca..86e222e9e05cc 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-for-loop-with-context-variable-iterator.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/for-loop-with-context-variable-iterator.js @@ -4,7 +4,20 @@ function Component() { // NOTE: `i` is a context variable because it's reassigned and also referenced // within a closure, the `onClick` handler of each item for (let i = MIN; i <= MAX; i += INCREMENT) { - items.push( data.set(i)} />); + items.push(
data.set(i)} />); } - return items; + return <>{items}; } + +const MIN = 0; +const MAX = 3; +const INCREMENT = 1; + +function useData() { + return new Map(); +} + +export const FIXTURE_ENTRYPOINT = { + params: [], + fn: Component, +}; diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 9f15eabe0d029..94eaf1e20e6e8 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -1105,6 +1105,143 @@ function callWithDebugContextInDEV( const voidHandler = () => {}; +function processServerComponentReturnValue( + request: Request, + task: Task, + Component: any, + result: any, +): any { + // A Server Component's return value has a few special properties due to being + // in the return position of a Component. We convert them here. + if ( + typeof result !== 'object' || + result === null || + isClientReference(result) + ) { + return result; + } + + if (typeof result.then === 'function') { + // When the return value is in children position we can resolve it immediately, + // to its value without a wrapper if it's synchronously available. + const thenable: Thenable = result; + if (__DEV__) { + // If the thenable resolves to an element, then it was in a static position, + // the return value of a Server Component. That doesn't need further validation + // of keys. The Server Component itself would have had a key. + thenable.then(resolvedValue => { + if ( + typeof resolvedValue === 'object' && + resolvedValue !== null && + resolvedValue.$$typeof === REACT_ELEMENT_TYPE + ) { + resolvedValue._store.validated = 1; + } + }, voidHandler); + } + if (thenable.status === 'fulfilled') { + return thenable.value; + } + // TODO: Once we accept Promises as children on the client, we can just return + // the thenable here. + return createLazyWrapperAroundWakeable(result); + } + + if (__DEV__) { + if ((result: any).$$typeof === REACT_ELEMENT_TYPE) { + // If the server component renders to an element, then it was in a static position. + // That doesn't need further validation of keys. The Server Component itself would + // have had a key. + (result: any)._store.validated = 1; + } + } + + // Normally we'd serialize an Iterator/AsyncIterator as a single-shot which is not compatible + // to be rendered as a React Child. However, because we have the function to recreate + // an iterable from rendering the element again, we can effectively treat it as multi- + // shot. Therefore we treat this as an Iterable/AsyncIterable, whether it was one or not, by + // adding a wrapper so that this component effectively renders down to an AsyncIterable. + const iteratorFn = getIteratorFn(result); + if (iteratorFn) { + const iterableChild = result; + const multiShot = { + [Symbol.iterator]: function () { + const iterator = iteratorFn.call(iterableChild); + if (__DEV__) { + // If this was an Iterator but not a GeneratorFunction we warn because + // it might have been a mistake. Technically you can make this mistake with + // GeneratorFunctions and even single-shot Iterables too but it's extra + // tempting to try to return the value from a generator. + if (iterator === iterableChild) { + const isGeneratorComponent = + // $FlowIgnore[method-unbinding] + Object.prototype.toString.call(Component) === + '[object GeneratorFunction]' && + // $FlowIgnore[method-unbinding] + Object.prototype.toString.call(iterableChild) === + '[object Generator]'; + if (!isGeneratorComponent) { + callWithDebugContextInDEV(request, task, () => { + console.error( + 'Returning an Iterator from a Server Component is not supported ' + + 'since it cannot be looped over more than once. ', + ); + }); + } + } + } + return (iterator: any); + }, + }; + if (__DEV__) { + (multiShot: any)._debugInfo = iterableChild._debugInfo; + } + return multiShot; + } + if ( + enableFlightReadableStream && + typeof (result: any)[ASYNC_ITERATOR] === 'function' && + (typeof ReadableStream !== 'function' || + !(result instanceof ReadableStream)) + ) { + const iterableChild = result; + const multishot = { + [ASYNC_ITERATOR]: function () { + const iterator = (iterableChild: any)[ASYNC_ITERATOR](); + if (__DEV__) { + // If this was an AsyncIterator but not an AsyncGeneratorFunction we warn because + // it might have been a mistake. Technically you can make this mistake with + // AsyncGeneratorFunctions and even single-shot AsyncIterables too but it's extra + // tempting to try to return the value from a generator. + if (iterator === iterableChild) { + const isGeneratorComponent = + // $FlowIgnore[method-unbinding] + Object.prototype.toString.call(Component) === + '[object AsyncGeneratorFunction]' && + // $FlowIgnore[method-unbinding] + Object.prototype.toString.call(iterableChild) === + '[object AsyncGenerator]'; + if (!isGeneratorComponent) { + callWithDebugContextInDEV(request, task, () => { + console.error( + 'Returning an AsyncIterator from a Server Component is not supported ' + + 'since it cannot be looped over more than once. ', + ); + }); + } + } + } + return iterator; + }, + }; + if (__DEV__) { + (multishot: any)._debugInfo = iterableChild._debugInfo; + } + return multishot; + } + return result; +} + function renderFunctionComponent( request: Request, task: Task, @@ -1231,123 +1368,9 @@ function renderFunctionComponent( throw null; } - if ( - typeof result === 'object' && - result !== null && - !isClientReference(result) - ) { - if (typeof result.then === 'function') { - // When the return value is in children position we can resolve it immediately, - // to its value without a wrapper if it's synchronously available. - const thenable: Thenable = result; - if (__DEV__) { - // If the thenable resolves to an element, then it was in a static position, - // the return value of a Server Component. That doesn't need further validation - // of keys. The Server Component itself would have had a key. - thenable.then(resolvedValue => { - if ( - typeof resolvedValue === 'object' && - resolvedValue !== null && - resolvedValue.$$typeof === REACT_ELEMENT_TYPE - ) { - resolvedValue._store.validated = 1; - } - }, voidHandler); - } - if (thenable.status === 'fulfilled') { - return thenable.value; - } - // TODO: Once we accept Promises as children on the client, we can just return - // the thenable here. - result = createLazyWrapperAroundWakeable(result); - } + // Apply special cases. + result = processServerComponentReturnValue(request, task, Component, result); - // Normally we'd serialize an Iterator/AsyncIterator as a single-shot which is not compatible - // to be rendered as a React Child. However, because we have the function to recreate - // an iterable from rendering the element again, we can effectively treat it as multi- - // shot. Therefore we treat this as an Iterable/AsyncIterable, whether it was one or not, by - // adding a wrapper so that this component effectively renders down to an AsyncIterable. - const iteratorFn = getIteratorFn(result); - if (iteratorFn) { - const iterableChild = result; - result = { - [Symbol.iterator]: function () { - const iterator = iteratorFn.call(iterableChild); - if (__DEV__) { - // If this was an Iterator but not a GeneratorFunction we warn because - // it might have been a mistake. Technically you can make this mistake with - // GeneratorFunctions and even single-shot Iterables too but it's extra - // tempting to try to return the value from a generator. - if (iterator === iterableChild) { - const isGeneratorComponent = - // $FlowIgnore[method-unbinding] - Object.prototype.toString.call(Component) === - '[object GeneratorFunction]' && - // $FlowIgnore[method-unbinding] - Object.prototype.toString.call(iterableChild) === - '[object Generator]'; - if (!isGeneratorComponent) { - callWithDebugContextInDEV(request, task, () => { - console.error( - 'Returning an Iterator from a Server Component is not supported ' + - 'since it cannot be looped over more than once. ', - ); - }); - } - } - } - return (iterator: any); - }, - }; - if (__DEV__) { - (result: any)._debugInfo = iterableChild._debugInfo; - } - } else if ( - enableFlightReadableStream && - typeof (result: any)[ASYNC_ITERATOR] === 'function' && - (typeof ReadableStream !== 'function' || - !(result instanceof ReadableStream)) - ) { - const iterableChild = result; - result = { - [ASYNC_ITERATOR]: function () { - const iterator = (iterableChild: any)[ASYNC_ITERATOR](); - if (__DEV__) { - // If this was an AsyncIterator but not an AsyncGeneratorFunction we warn because - // it might have been a mistake. Technically you can make this mistake with - // AsyncGeneratorFunctions and even single-shot AsyncIterables too but it's extra - // tempting to try to return the value from a generator. - if (iterator === iterableChild) { - const isGeneratorComponent = - // $FlowIgnore[method-unbinding] - Object.prototype.toString.call(Component) === - '[object AsyncGeneratorFunction]' && - // $FlowIgnore[method-unbinding] - Object.prototype.toString.call(iterableChild) === - '[object AsyncGenerator]'; - if (!isGeneratorComponent) { - callWithDebugContextInDEV(request, task, () => { - console.error( - 'Returning an AsyncIterator from a Server Component is not supported ' + - 'since it cannot be looped over more than once. ', - ); - }); - } - } - } - return iterator; - }, - }; - if (__DEV__) { - (result: any)._debugInfo = iterableChild._debugInfo; - } - } else if (__DEV__ && (result: any).$$typeof === REACT_ELEMENT_TYPE) { - // If the server component renders to an element, then it was in a static position. - // That doesn't need further validation of keys. The Server Component itself would - // have had a key. - (result: any)._store.validated = 1; - } - } // Track this element's key on the Server Component on the keyPath context.. const prevKeyPath = task.keyPath; const prevImplicitSlot = task.implicitSlot;