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 5dccb5ffd2..b2ee773c26 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts @@ -1934,7 +1934,6 @@ function lowerExpression( switch (leftNode.type) { case 'Identifier': { const leftExpr = left as NodePath; - const identifier = lowerIdentifier(builder, leftExpr); const leftPlace = lowerExpressionToTemporary(builder, leftExpr); const right = lowerExpressionToTemporary(builder, expr.get('right')); const binaryPlace = lowerValueToTemporary(builder, { @@ -1944,30 +1943,42 @@ function lowerExpression( right, loc: exprLoc, }); - const kind = getStoreKind(builder, leftExpr); - if (kind === 'StoreLocal') { - lowerValueToTemporary(builder, { - kind: 'StoreLocal', - lvalue: { - place: {...identifier}, - kind: InstructionKind.Reassign, - }, - value: {...binaryPlace}, - type: null, - loc: exprLoc, - }); - return {kind: 'LoadLocal', place: identifier, loc: exprLoc}; + const binding = builder.resolveIdentifier(leftExpr); + if (binding.kind === 'Identifier') { + const identifier = lowerIdentifier(builder, leftExpr); + const kind = getStoreKind(builder, leftExpr); + if (kind === 'StoreLocal') { + lowerValueToTemporary(builder, { + kind: 'StoreLocal', + lvalue: { + place: {...identifier}, + kind: InstructionKind.Reassign, + }, + value: {...binaryPlace}, + type: null, + loc: exprLoc, + }); + return {kind: 'LoadLocal', place: identifier, loc: exprLoc}; + } else { + lowerValueToTemporary(builder, { + kind: 'StoreContext', + lvalue: { + place: {...identifier}, + kind: InstructionKind.Reassign, + }, + value: {...binaryPlace}, + loc: exprLoc, + }); + return {kind: 'LoadContext', place: identifier, loc: exprLoc}; + } } else { - lowerValueToTemporary(builder, { - kind: 'StoreContext', - lvalue: { - place: {...identifier}, - kind: InstructionKind.Reassign, - }, + const temporary = lowerValueToTemporary(builder, { + kind: 'StoreGlobal', + name: leftExpr.node.name, value: {...binaryPlace}, loc: exprLoc, }); - return {kind: 'LoadContext', place: identifier, loc: exprLoc}; + return {kind: 'LoadLocal', place: temporary, loc: temporary.loc}; } } case 'MemberExpression': { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-update-global-should-bailout.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-update-global-should-bailout.expect.md deleted file mode 100644 index d8804cad09..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-update-global-should-bailout.expect.md +++ /dev/null @@ -1,32 +0,0 @@ - -## Input - -```javascript -let renderCount = 0; -function useFoo() { - renderCount += 1; - return renderCount; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [], -}; - -``` - -## Code - -```javascript -let renderCount = 0; -function useFoo() { - return renderCount; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [], -}; - -``` - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.update-global-should-bailout.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.update-global-should-bailout.expect.md new file mode 100644 index 0000000000..ca299dbdec --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.update-global-should-bailout.expect.md @@ -0,0 +1,31 @@ + +## Input + +```javascript +let renderCount = 0; +function useFoo() { + renderCount += 1; + return renderCount; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [], +}; + +``` + + +## Error + +``` + 1 | let renderCount = 0; + 2 | function useFoo() { +> 3 | renderCount += 1; + | ^^^^^^^^^^^^^^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (3:3) + 4 | return renderCount; + 5 | } + 6 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-update-global-should-bailout.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.update-global-should-bailout.tsx similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-update-global-should-bailout.tsx rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.update-global-should-bailout.tsx diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-update-global-in-callback.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/update-global-in-callback.expect.md similarity index 85% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-update-global-in-callback.expect.md rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/update-global-in-callback.expect.md index 682259ca3a..a250ce4a10 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-update-global-in-callback.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/update-global-in-callback.expect.md @@ -40,6 +40,7 @@ function Foo() { return t0; } function _temp() { + renderCount = renderCount + 1; return renderCount; } @@ -49,4 +50,6 @@ export const FIXTURE_ENTRYPOINT = { }; ``` - \ No newline at end of file + +### Eval output +(kind: ok)
{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-update-global-in-callback.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/update-global-in-callback.tsx similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-update-global-in-callback.tsx rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/update-global-in-callback.tsx diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index 9bc39d72b1..ff542dfb97 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -484,8 +484,6 @@ const skipFilter = new Set([ 'rules-of-hooks/rules-of-hooks-69521d94fa03', // bugs - 'bug-invalid-update-global-should-bailout', - 'bug-update-global-in-callback', 'bug-invalid-hoisting-functionexpr', 'original-reactive-scopes-fork/bug-nonmutating-capture-in-unsplittable-memo-block', 'original-reactive-scopes-fork/bug-hoisted-declaration-with-scope',