@samitouri / QOS-React / commits / b315a0f713

[compiler] Fix for destructuring with mixed declaration/reassignment (#35144)

Destructing statements that start off as declarations can end up becoming reassignments if the variable is a scope declaration, so we have existing logic to handle cases where some parts of a destructure need to be converted into new locals, with a reassignment to the hoisted scope variable afterwards. However, there is an edge case where all of the values are reassigned, in which case we don't need to rewrite and can just set the instruction kind to reassign. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35144). * #35148 * #35147 * #35146 * __->__ #35144

Joseph Savona committed Nov 17, 2025 at 11:34 UTC b315a0f7133a3251f72970d56c2ad454bdd47003

5 files changed +268 -27

compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts

+27

@@ -11,6 +11,7 @@ import {

11 BasicBlock,

12 BlockId,

13 Instruction,

14 + InstructionKind,

15 InstructionValue,

16 makeInstructionId,

17 Pattern,

@@ -32,6 +33,32 @@ export function* eachInstructionLValue(

33 yield* eachInstructionValueLValue(instr.value);

34 }

36 +export function* eachInstructionLValueWithKind(

37 + instr: ReactiveInstruction,

38 +): Iterable<[Place, InstructionKind]> {

39 + switch (instr.value.kind) {

40 + case 'DeclareContext':

41 + case 'StoreContext':

42 + case 'DeclareLocal':

43 + case 'StoreLocal': {

44 + yield [instr.value.lvalue.place, instr.value.lvalue.kind];

45 + break;

46 + }

47 + case 'Destructure': {

48 + const kind = instr.value.lvalue.kind;

49 + for (const place of eachPatternOperand(instr.value.lvalue.pattern)) {

50 + yield [place, kind];

51 + }

52 + break;

53 + }

54 + case 'PostfixUpdate':

55 + case 'PrefixUpdate': {

56 + yield [instr.value.lvalue, InstructionKind.Reassign];

57 + break;

58 + }

59 + }

60 +}

61 +

62 export function* eachInstructionValueLValue(

63 value: ReactiveValue,

64 ): Iterable<Place> {

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts

-22

@@ -1359,8 +1359,6 @@ function codegenInstructionNullable(

1359 value = null;

1360 } else {

1361 lvalue = instr.value.lvalue.pattern;

1362 - let hasReassign = false;

1363 - let hasDeclaration = false;

1362 for (const place of eachPatternOperand(lvalue)) {

1363 if (

1364 kind !== InstructionKind.Reassign &&

@@ -1368,26 +1366,6 @@ function codegenInstructionNullable(

1366 ) {

1367 cx.temp.set(place.identifier.declarationId, null);

1368 }

1371 - const isDeclared = cx.hasDeclared(place.identifier);

1372 - hasReassign ||= isDeclared;

1373 - hasDeclaration ||= !isDeclared;

1374 - }

1375 - if (hasReassign && hasDeclaration) {

1376 - CompilerError.invariant(false, {

1377 - reason:

1378 - 'Encountered a destructuring operation where some identifiers are already declared (reassignments) but others are not (declarations)',

1379 - description: null,

1380 - details: [

1381 - {

1382 - kind: 'error',

1383 - loc: instr.loc,

1384 - message: null,

1385 - },

1386 - ],

1387 - suggestions: null,

1388 - });

1389 - } else if (hasReassign) {

1390 - kind = InstructionKind.Reassign;

1369 }

1370 value = codegenPlaceToExpression(cx, instr.value.value);

1371 }

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/ExtractScopeDeclarationsFromDestructuring.ts

+26 -5

@@ -19,7 +19,11 @@ import {

19 promoteTemporary,

20 } from '../HIR';

21 import {clonePlaceToTemporary} from '../HIR/HIRBuilder';

22 -import {eachPatternOperand, mapPatternOperands} from '../HIR/visitors';

22 +import {

23 + eachInstructionLValueWithKind,

24 + eachPatternOperand,

25 + mapPatternOperands,

26 +} from '../HIR/visitors';

27 import {

28 ReactiveFunctionTransform,

29 Transformed,

@@ -113,6 +117,9 @@ class Visitor extends ReactiveFunctionTransform<State> {

117 ): Transformed<ReactiveStatement> {

118 this.visitInstruction(instruction, state);

119

120 + let instructionsToProcess: Array<ReactiveInstruction> = [instruction];

121 + let result: Transformed<ReactiveStatement> = {kind: 'keep'};

122 +

123 if (instruction.value.kind === 'Destructure') {

124 const transformed = transformDestructuring(

125 state,

@@ -120,7 +127,8 @@ class Visitor extends ReactiveFunctionTransform<State> {

127 instruction.value,

128 );

129 if (transformed) {

123 - return {

130 + instructionsToProcess = transformed;

131 + result = {

132 kind: 'replace-many',

133 value: transformed.map(instruction => ({

134 kind: 'instruction',

@@ -129,7 +137,17 @@ class Visitor extends ReactiveFunctionTransform<State> {

137 };

138 }

139 }

132 - return {kind: 'keep'};

140 +

141 + // Update state.declared with declarations from the instruction(s)

142 + for (const instr of instructionsToProcess) {

143 + for (const [place, kind] of eachInstructionLValueWithKind(instr)) {

144 + if (kind !== InstructionKind.Reassign) {

145 + state.declared.add(place.identifier.declarationId);

146 + }

147 + }

148 + }

149 +

150 + return result;

151 }

152 }

153

@@ -144,10 +162,13 @@ function transformDestructuring(

162 const isDeclared = state.declared.has(place.identifier.declarationId);

163 if (isDeclared) {

164 reassigned.add(place.identifier.id);

165 + } else {

166 + hasDeclaration = true;

167 }

148 - hasDeclaration ||= !isDeclared;

168 }

150 - if (reassigned.size === 0 || !hasDeclaration) {

169 + if (!hasDeclaration) {

170 + // all reassignments

171 + destructure.lvalue.kind = InstructionKind.Reassign;

172 return null;

173 }

174 /*

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-destructuring-reassignment-undefined-variable.expect.md new

+162

@@ -0,0 +1,162 @@

1 +

2 +## Input

3 +

4 +```javascript

5 +// @flow @compilationMode:"infer"

6 +'use strict';

7 +

8 +function getWeekendDays(user) {

9 + return [0, 6];

10 +}

11 +

12 +function getConfig(weekendDays) {

13 + return [1, 5];

14 +}

15 +

16 +component Calendar(user, defaultFirstDay, currentDate, view) {

17 + const weekendDays = getWeekendDays(user);

18 + let firstDay = defaultFirstDay;

19 + let daysToDisplay = 7;

20 + if (view === 'week') {

21 + let lastDay;

22 + // this assignment produces invalid code

23 + [firstDay, lastDay] = getConfig(weekendDays);

24 + daysToDisplay = ((7 + lastDay - firstDay) % 7) + 1;

25 + } else if (view === 'day') {

26 + firstDay = currentDate.getDayOfWeek();

27 + daysToDisplay = 1;

28 + }

29 +

30 + return [currentDate, firstDay, daysToDisplay];

31 +}

32 +

33 +export const FIXTURE_ENTRYPOINT = {

34 + fn: Calendar,

35 + params: [

36 + {

37 + user: {},

38 + defaultFirstDay: 1,

39 + currentDate: {getDayOfWeek: () => 3},

40 + view: 'week',

41 + },

42 + ],

43 + sequentialRenders: [

44 + {

45 + user: {},

46 + defaultFirstDay: 1,

47 + currentDate: {getDayOfWeek: () => 3},

48 + view: 'week',

49 + },

50 + {

51 + user: {},

52 + defaultFirstDay: 1,

53 + currentDate: {getDayOfWeek: () => 3},

54 + view: 'day',

55 + },

56 + ],

57 +};

58 +

59 +```

60 +

61 +## Code

62 +

63 +```javascript

64 +"use strict";

65 +import { c as _c } from "react/compiler-runtime";

66 +

67 +function getWeekendDays(user) {

68 + return [0, 6];

69 +}

70 +

71 +function getConfig(weekendDays) {

72 + return [1, 5];

73 +}

74 +

75 +function Calendar(t0) {

76 + const $ = _c(12);

77 + const { user, defaultFirstDay, currentDate, view } = t0;

78 + let daysToDisplay;

79 + let firstDay;

80 + if (

81 + $[0] !== currentDate ||

82 + $[1] !== defaultFirstDay ||

83 + $[2] !== user ||

84 + $[3] !== view

85 + ) {

86 + const weekendDays = getWeekendDays(user);

87 + firstDay = defaultFirstDay;

88 + daysToDisplay = 7;

89 + if (view === "week") {

90 + let lastDay;

91 +

92 + [firstDay, lastDay] = getConfig(weekendDays);

93 + daysToDisplay = ((7 + lastDay - firstDay) % 7) + 1;

94 + } else {

95 + if (view === "day") {

96 + let t1;

97 + if ($[6] !== currentDate) {

98 + t1 = currentDate.getDayOfWeek();

99 + $[6] = currentDate;

100 + $[7] = t1;

101 + } else {

102 + t1 = $[7];

103 + }

104 + firstDay = t1;

105 + daysToDisplay = 1;

106 + }

107 + }

108 + $[0] = currentDate;

109 + $[1] = defaultFirstDay;

110 + $[2] = user;

111 + $[3] = view;

112 + $[4] = daysToDisplay;

113 + $[5] = firstDay;

114 + } else {

115 + daysToDisplay = $[4];

116 + firstDay = $[5];

117 + }

118 + let t1;

119 + if ($[8] !== currentDate || $[9] !== daysToDisplay || $[10] !== firstDay) {

120 + t1 = [currentDate, firstDay, daysToDisplay];

121 + $[8] = currentDate;

122 + $[9] = daysToDisplay;

123 + $[10] = firstDay;

124 + $[11] = t1;

125 + } else {

126 + t1 = $[11];

127 + }

128 + return t1;

129 +}

130 +

131 +export const FIXTURE_ENTRYPOINT = {

132 + fn: Calendar,

133 + params: [

134 + {

135 + user: {},

136 + defaultFirstDay: 1,

137 + currentDate: { getDayOfWeek: () => 3 },

138 + view: "week",

139 + },

140 + ],

141 +

142 + sequentialRenders: [

143 + {

144 + user: {},

145 + defaultFirstDay: 1,

146 + currentDate: { getDayOfWeek: () => 3 },

147 + view: "week",

148 + },

149 + {

150 + user: {},

151 + defaultFirstDay: 1,

152 + currentDate: { getDayOfWeek: () => 3 },

153 + view: "day",

154 + },

155 + ],

156 +};

157 +

158 +```

159 +

160 +### Eval output

161 +(kind: ok) [{"getDayOfWeek":"[[ function params=0 ]]"},1,5]

162 +[{"getDayOfWeek":"[[ function params=0 ]]"},3,1]

\ No newline at end of file

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-destructuring-reassignment-undefined-variable.js new

+53

@@ -0,0 +1,53 @@

1 +// @flow @compilationMode:"infer"

2 +'use strict';

3 +

4 +function getWeekendDays(user) {

5 + return [0, 6];

6 +}

7 +

8 +function getConfig(weekendDays) {

9 + return [1, 5];

10 +}

11 +

12 +component Calendar(user, defaultFirstDay, currentDate, view) {

13 + const weekendDays = getWeekendDays(user);

14 + let firstDay = defaultFirstDay;

15 + let daysToDisplay = 7;

16 + if (view === 'week') {

17 + let lastDay;

18 + // this assignment produces invalid code

19 + [firstDay, lastDay] = getConfig(weekendDays);

20 + daysToDisplay = ((7 + lastDay - firstDay) % 7) + 1;

21 + } else if (view === 'day') {

22 + firstDay = currentDate.getDayOfWeek();

23 + daysToDisplay = 1;

24 + }

25 +

26 + return [currentDate, firstDay, daysToDisplay];

27 +}

28 +

29 +export const FIXTURE_ENTRYPOINT = {

30 + fn: Calendar,

31 + params: [

32 + {

33 + user: {},

34 + defaultFirstDay: 1,

35 + currentDate: {getDayOfWeek: () => 3},

36 + view: 'week',

37 + },

38 + ],

39 + sequentialRenders: [

40 + {

41 + user: {},

42 + defaultFirstDay: 1,

43 + currentDate: {getDayOfWeek: () => 3},

44 + view: 'week',

45 + },

46 + {

47 + user: {},

48 + defaultFirstDay: 1,

49 + currentDate: {getDayOfWeek: () => 3},

50 + view: 'day',

51 + },

52 + ],

53 +};