From 5255e75df44319bdbfb09112b271c0241cb1f965 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 30 Apr 2025 11:15:21 +0900 Subject: [PATCH] [compiler] Improve source map coverage for variable declarations/reassignments We were missing source location information for the lvalue variable name, and for reassignment expressions. Note that we don't track source location for patterns, so we only handle cases where the declaration is a simple identifier. [ghstack-poisoned] --- .../ReactiveScopes/CodegenReactiveFunction.ts | 17 ++++++++++--- ...ined-assignment-context-variable.expect.md | 1 - .../codegen-inline-iife-reassign.expect.md | 1 - .../codegen-inline-iife-storeprop.expect.md | 1 - .../compiler/codegen-inline-iife.expect.md | 1 - ...text-variable-as-jsx-element-tag.expect.md | 1 - ...nverted-optionals-parallel-paths.expect.md | 1 - ...nverted-optionals-parallel-paths.expect.md | 1 - .../compiler/sourcemaps-simple.expect.md | 25 +++++++++++-------- .../fixtures/compiler/sourcemaps-simple.js | 5 ++-- ...-preserve-memoization-guarantees.expect.md | 1 - .../compiler/useMemo-return-empty.expect.md | 1 - 12 files changed, 32 insertions(+), 24 deletions(-) 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 1349817201..54e07dce83 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -1277,7 +1277,11 @@ function codegenInstructionNullable( suggestions: null, }); return createVariableDeclaration(instr.loc, 'const', [ - t.variableDeclarator(codegenLValue(cx, lvalue), value), + createVariableDeclarator( + lvalue.kind === 'Identifier' ? lvalue.identifier.loc : null, + codegenLValue(cx, lvalue), + value, + ), ]); } case InstructionKind.Function: { @@ -1317,7 +1321,11 @@ function codegenInstructionNullable( suggestions: null, }); return createVariableDeclaration(instr.loc, 'let', [ - t.variableDeclarator(codegenLValue(cx, lvalue), value), + createVariableDeclarator( + lvalue.kind === 'Identifier' ? lvalue.identifier.loc : null, + codegenLValue(cx, lvalue), + value, + ), ]); } case InstructionKind.Reassign: { @@ -1327,7 +1335,8 @@ function codegenInstructionNullable( loc: instr.value.loc, suggestions: null, }); - const expr = t.assignmentExpression( + const expr = createAssignmentExpression( + instr.loc, '=', codegenLValue(cx, lvalue), value, @@ -1530,6 +1539,8 @@ const createBinaryExpression = withLoc(t.binaryExpression); const createExpressionStatement = withLoc(t.expressionStatement); const _createLabelledStatement = withLoc(t.labeledStatement); const createVariableDeclaration = withLoc(t.variableDeclaration); +const createVariableDeclarator = withLoc(t.variableDeclarator); +const createAssignmentExpression = withLoc(t.assignmentExpression); const createFunctionDeclaration = withLoc(t.functionDeclaration); const _createWhileStatement = withLoc(t.whileStatement); const createTaggedTemplateExpression = withLoc(t.taggedTemplateExpression); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/chained-assignment-context-variable.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/chained-assignment-context-variable.expect.md index 13b0af22b9..796b75b2dd 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/chained-assignment-context-variable.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/chained-assignment-context-variable.expect.md @@ -33,7 +33,6 @@ function Component() { let y; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { y = x = {}; - const foo = () => { x = makeArray(); }; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife-reassign.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife-reassign.expect.md index cf85967682..81c3ff56d7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife-reassign.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife-reassign.expect.md @@ -39,7 +39,6 @@ function useTest() { const t1 = (w = 42); const t2 = w; let t3; - w = 999; t3 = 2; t0 = makeArray(t1, t2, t3); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife-storeprop.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife-storeprop.expect.md index 58c54ddaab..856b8e64b5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife-storeprop.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife-storeprop.expect.md @@ -38,7 +38,6 @@ function useTest() { const t1 = (w.x = 42); const t2 = w.x; let t3; - w.x = 999; t3 = 2; t0 = makeArray(t1, t2, t3); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife.expect.md index 25a08bc332..752a5774c2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife.expect.md @@ -33,7 +33,6 @@ function useTest() { if ($[0] === Symbol.for("react.memo_cache_sentinel")) { const t1 = print(1); let t2; - print(2); t2 = 2; t0 = makeArray(t1, t2); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/context-variable-as-jsx-element-tag.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/context-variable-as-jsx-element-tag.expect.md index da3bb94ed5..06d227bb59 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/context-variable-as-jsx-element-tag.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/context-variable-as-jsx-element-tag.expect.md @@ -35,7 +35,6 @@ function Component(props) { if ($[0] === Symbol.for("react.memo_cache_sentinel")) { Component = Stringify; let t0; - t0 = Component; Component = t0; $[0] = Component; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-inverted-optionals-parallel-paths.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-inverted-optionals-parallel-paths.expect.md index 98fcfbe7f0..1d46dadf59 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-inverted-optionals-parallel-paths.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-inverted-optionals-parallel-paths.expect.md @@ -24,7 +24,6 @@ import { ValidateMemoization } from "shared-runtime"; function Component(props) { const $ = _c(2); let t0; - const x$0 = []; x$0.push(props?.a.b?.c.d?.e); x$0.push(props.a?.b.c?.d.e); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/optional-member-expression-inverted-optionals-parallel-paths.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/optional-member-expression-inverted-optionals-parallel-paths.expect.md index 60ae4e49d3..20b704e914 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/optional-member-expression-inverted-optionals-parallel-paths.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/optional-member-expression-inverted-optionals-parallel-paths.expect.md @@ -24,7 +24,6 @@ import { ValidateMemoization } from "shared-runtime"; function Component(props) { const $ = _c(2); let t0; - const x$0 = []; x$0.push(props?.a.b?.c.d?.e); x$0.push(props.a?.b.c?.d.e); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/sourcemaps-simple.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/sourcemaps-simple.expect.md index 590b3d92f4..429d7c796f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/sourcemaps-simple.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/sourcemaps-simple.expect.md @@ -3,8 +3,9 @@ ```javascript // @sourceMaps -export const Button = () => { - return ; +export const Button = name => { + const greeting = `Hello, ${name}`; + return ; }; ``` @@ -13,14 +14,16 @@ export const Button = () => { ```javascript import { c as _c } from "react/compiler-runtime"; // @sourceMaps -export const Button = () => { - const $ = _c(1); +export const Button = (name) => { + const $ = _c(2); + const greeting = `Hello, ${name}`; let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - t0 = ; - $[0] = t0; + if ($[0] !== greeting) { + t0 = ; + $[0] = greeting; + $[1] = t0; } else { - t0 = $[0]; + t0 = $[1]; } return t0; }; @@ -34,15 +37,17 @@ export const Button = () => { "version": 3, "names": [ "Button", + "name", + "greeting", "t0" ], "sources": [ "sourcemaps-simple.ts" ], "sourcesContent": [ - "// @sourceMaps\nexport const Button = () => {\n return ;\n};\n" + "// @sourceMaps\nexport const Button = name => {\n const greeting = `Hello, ${name}`;\n return ;\n};\n" ], - "mappings": "kDAAA;AACA,OAAO,MAAMA,MAAM,GAAGA,CAAA,K;SACb,OAAyB,CAAjB,QAAQ,EAAhB,MAAyB,C,qCAAzBC,EAAyB,C,CACjC", + "mappings": "kDAAA;AACA,OAAO,MAAMA,MAAM,GAAGA,CAAAC,IAAA,K;EACpB,M,WAAiBC,UAAUD,IAAI,EAAjB,C;SACP,OAA2B,CAAlBC,SAAO,CAAE,EAAlB,MAA2B,C,qDAA3BC,EAA2B,C,CACnC", "ignoreList": [] } ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/sourcemaps-simple.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/sourcemaps-simple.js index dd68df6a86..4b2aa83ad5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/sourcemaps-simple.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/sourcemaps-simple.js @@ -1,4 +1,5 @@ // @sourceMaps -export const Button = () => { - return ; +export const Button = name => { + const greeting = `Hello, ${name}`; + return ; }; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-mabye-modified-free-variable-dont-preserve-memoization-guarantees.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-mabye-modified-free-variable-dont-preserve-memoization-guarantees.expect.md index 666fa49376..03b88f9e57 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-mabye-modified-free-variable-dont-preserve-memoization-guarantees.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-mabye-modified-free-variable-dont-preserve-memoization-guarantees.expect.md @@ -52,7 +52,6 @@ function Component(props) { useHook(); let t0; - const x = makeObject_Primitives(); x.value = props.value; mutate(x, free, part); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-return-empty.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-return-empty.expect.md index b348ae34b6..9c7f0a75d2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-return-empty.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-return-empty.expect.md @@ -16,7 +16,6 @@ function component(a) { ```javascript function component(a) { let t0; - mutate(a); t0 = undefined; }