diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts index af1cffe85fd31..5735358cecf2a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts @@ -11,6 +11,7 @@ import { BasicBlock, BlockId, Instruction, + InstructionKind, InstructionValue, makeInstructionId, Pattern, @@ -32,6 +33,32 @@ export function* eachInstructionLValue( yield* eachInstructionValueLValue(instr.value); } +export function* eachInstructionLValueWithKind( + instr: ReactiveInstruction, +): Iterable<[Place, InstructionKind]> { + switch (instr.value.kind) { + case 'DeclareContext': + case 'StoreContext': + case 'DeclareLocal': + case 'StoreLocal': { + yield [instr.value.lvalue.place, instr.value.lvalue.kind]; + break; + } + case 'Destructure': { + const kind = instr.value.lvalue.kind; + for (const place of eachPatternOperand(instr.value.lvalue.pattern)) { + yield [place, kind]; + } + break; + } + case 'PostfixUpdate': + case 'PrefixUpdate': { + yield [instr.value.lvalue, InstructionKind.Reassign]; + break; + } + } +} + export function* eachInstructionValueLValue( value: ReactiveValue, ): Iterable { 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 c497253a22fb5..f81c962edf867 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -1359,8 +1359,6 @@ function codegenInstructionNullable( value = null; } else { lvalue = instr.value.lvalue.pattern; - let hasReassign = false; - let hasDeclaration = false; for (const place of eachPatternOperand(lvalue)) { if ( kind !== InstructionKind.Reassign && @@ -1368,26 +1366,6 @@ function codegenInstructionNullable( ) { cx.temp.set(place.identifier.declarationId, null); } - const isDeclared = cx.hasDeclared(place.identifier); - hasReassign ||= isDeclared; - hasDeclaration ||= !isDeclared; - } - if (hasReassign && hasDeclaration) { - CompilerError.invariant(false, { - reason: - 'Encountered a destructuring operation where some identifiers are already declared (reassignments) but others are not (declarations)', - description: null, - details: [ - { - kind: 'error', - loc: instr.loc, - message: null, - }, - ], - suggestions: null, - }); - } else if (hasReassign) { - kind = InstructionKind.Reassign; } value = codegenPlaceToExpression(cx, instr.value.value); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/ExtractScopeDeclarationsFromDestructuring.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/ExtractScopeDeclarationsFromDestructuring.ts index 642b89747e6ea..f24861152aa08 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/ExtractScopeDeclarationsFromDestructuring.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/ExtractScopeDeclarationsFromDestructuring.ts @@ -19,7 +19,11 @@ import { promoteTemporary, } from '../HIR'; import {clonePlaceToTemporary} from '../HIR/HIRBuilder'; -import {eachPatternOperand, mapPatternOperands} from '../HIR/visitors'; +import { + eachInstructionLValueWithKind, + eachPatternOperand, + mapPatternOperands, +} from '../HIR/visitors'; import { ReactiveFunctionTransform, Transformed, @@ -113,6 +117,9 @@ class Visitor extends ReactiveFunctionTransform { ): Transformed { this.visitInstruction(instruction, state); + let instructionsToProcess: Array = [instruction]; + let result: Transformed = {kind: 'keep'}; + if (instruction.value.kind === 'Destructure') { const transformed = transformDestructuring( state, @@ -120,7 +127,8 @@ class Visitor extends ReactiveFunctionTransform { instruction.value, ); if (transformed) { - return { + instructionsToProcess = transformed; + result = { kind: 'replace-many', value: transformed.map(instruction => ({ kind: 'instruction', @@ -129,7 +137,17 @@ class Visitor extends ReactiveFunctionTransform { }; } } - return {kind: 'keep'}; + + // Update state.declared with declarations from the instruction(s) + for (const instr of instructionsToProcess) { + for (const [place, kind] of eachInstructionLValueWithKind(instr)) { + if (kind !== InstructionKind.Reassign) { + state.declared.add(place.identifier.declarationId); + } + } + } + + return result; } } @@ -144,10 +162,13 @@ function transformDestructuring( const isDeclared = state.declared.has(place.identifier.declarationId); if (isDeclared) { reassigned.add(place.identifier.id); + } else { + hasDeclaration = true; } - hasDeclaration ||= !isDeclared; } - if (reassigned.size === 0 || !hasDeclaration) { + if (!hasDeclaration) { + // all reassignments + destructure.lvalue.kind = InstructionKind.Reassign; return null; } /* diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-destructuring-reassignment-undefined-variable.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-destructuring-reassignment-undefined-variable.expect.md new file mode 100644 index 0000000000000..544366b1de6e5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-destructuring-reassignment-undefined-variable.expect.md @@ -0,0 +1,162 @@ + +## Input + +```javascript +// @flow @compilationMode:"infer" +'use strict'; + +function getWeekendDays(user) { + return [0, 6]; +} + +function getConfig(weekendDays) { + return [1, 5]; +} + +component Calendar(user, defaultFirstDay, currentDate, view) { + const weekendDays = getWeekendDays(user); + let firstDay = defaultFirstDay; + let daysToDisplay = 7; + if (view === 'week') { + let lastDay; + // this assignment produces invalid code + [firstDay, lastDay] = getConfig(weekendDays); + daysToDisplay = ((7 + lastDay - firstDay) % 7) + 1; + } else if (view === 'day') { + firstDay = currentDate.getDayOfWeek(); + daysToDisplay = 1; + } + + return [currentDate, firstDay, daysToDisplay]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Calendar, + params: [ + { + user: {}, + defaultFirstDay: 1, + currentDate: {getDayOfWeek: () => 3}, + view: 'week', + }, + ], + sequentialRenders: [ + { + user: {}, + defaultFirstDay: 1, + currentDate: {getDayOfWeek: () => 3}, + view: 'week', + }, + { + user: {}, + defaultFirstDay: 1, + currentDate: {getDayOfWeek: () => 3}, + view: 'day', + }, + ], +}; + +``` + +## Code + +```javascript +"use strict"; +import { c as _c } from "react/compiler-runtime"; + +function getWeekendDays(user) { + return [0, 6]; +} + +function getConfig(weekendDays) { + return [1, 5]; +} + +function Calendar(t0) { + const $ = _c(12); + const { user, defaultFirstDay, currentDate, view } = t0; + let daysToDisplay; + let firstDay; + if ( + $[0] !== currentDate || + $[1] !== defaultFirstDay || + $[2] !== user || + $[3] !== view + ) { + const weekendDays = getWeekendDays(user); + firstDay = defaultFirstDay; + daysToDisplay = 7; + if (view === "week") { + let lastDay; + + [firstDay, lastDay] = getConfig(weekendDays); + daysToDisplay = ((7 + lastDay - firstDay) % 7) + 1; + } else { + if (view === "day") { + let t1; + if ($[6] !== currentDate) { + t1 = currentDate.getDayOfWeek(); + $[6] = currentDate; + $[7] = t1; + } else { + t1 = $[7]; + } + firstDay = t1; + daysToDisplay = 1; + } + } + $[0] = currentDate; + $[1] = defaultFirstDay; + $[2] = user; + $[3] = view; + $[4] = daysToDisplay; + $[5] = firstDay; + } else { + daysToDisplay = $[4]; + firstDay = $[5]; + } + let t1; + if ($[8] !== currentDate || $[9] !== daysToDisplay || $[10] !== firstDay) { + t1 = [currentDate, firstDay, daysToDisplay]; + $[8] = currentDate; + $[9] = daysToDisplay; + $[10] = firstDay; + $[11] = t1; + } else { + t1 = $[11]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Calendar, + params: [ + { + user: {}, + defaultFirstDay: 1, + currentDate: { getDayOfWeek: () => 3 }, + view: "week", + }, + ], + + sequentialRenders: [ + { + user: {}, + defaultFirstDay: 1, + currentDate: { getDayOfWeek: () => 3 }, + view: "week", + }, + { + user: {}, + defaultFirstDay: 1, + currentDate: { getDayOfWeek: () => 3 }, + view: "day", + }, + ], +}; + +``` + +### Eval output +(kind: ok) [{"getDayOfWeek":"[[ function params=0 ]]"},1,5] +[{"getDayOfWeek":"[[ function params=0 ]]"},3,1] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-destructuring-reassignment-undefined-variable.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-destructuring-reassignment-undefined-variable.js new file mode 100644 index 0000000000000..461d6bf16dfb3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-destructuring-reassignment-undefined-variable.js @@ -0,0 +1,53 @@ +// @flow @compilationMode:"infer" +'use strict'; + +function getWeekendDays(user) { + return [0, 6]; +} + +function getConfig(weekendDays) { + return [1, 5]; +} + +component Calendar(user, defaultFirstDay, currentDate, view) { + const weekendDays = getWeekendDays(user); + let firstDay = defaultFirstDay; + let daysToDisplay = 7; + if (view === 'week') { + let lastDay; + // this assignment produces invalid code + [firstDay, lastDay] = getConfig(weekendDays); + daysToDisplay = ((7 + lastDay - firstDay) % 7) + 1; + } else if (view === 'day') { + firstDay = currentDate.getDayOfWeek(); + daysToDisplay = 1; + } + + return [currentDate, firstDay, daysToDisplay]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Calendar, + params: [ + { + user: {}, + defaultFirstDay: 1, + currentDate: {getDayOfWeek: () => 3}, + view: 'week', + }, + ], + sequentialRenders: [ + { + user: {}, + defaultFirstDay: 1, + currentDate: {getDayOfWeek: () => 3}, + view: 'week', + }, + { + user: {}, + defaultFirstDay: 1, + currentDate: {getDayOfWeek: () => 3}, + view: 'day', + }, + ], +};