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 1349817201169..54e07dce83d4c 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 13b0af22b9d22..796b75b2ddbe1 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 cf85967682607..81c3ff56d7aca 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 58c54ddaab891..856b8e64b56a3 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 25a08bc3329cb..752a5774c2dde 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 da3bb94ed5ed4..06d227bb591aa 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 98fcfbe7f0f6f..1d46dadf592f7 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 60ae4e49d328c..20b704e9149cf 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 590b3d92f4940..429d7c796f387 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 dd68df6a86026..4b2aa83ad5d50 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 666fa49376b8f..03b88f9e57ae5 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 b348ae34b6f56..9c7f0a75d23f6 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; }