From 4575c8a5f6cb2bc006d586a1a0c025d677a62c99 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 22 Mar 2024 13:49:50 -0700 Subject: [PATCH] Detect hoisting where the reference is a reassignment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Our logic to detect hoisting relies on Babel's `isReferencedIdentifier()` to determine whether a reference to an identifier is a reference or a declaration. The idea is that we want to find references to variables that may be hoistable, before the declaration — the definition of hoisting. But due to the bug in isReferencedIdentifier, we skipped over reassignments of hoisted variables. The hack here checks if an identifier is a direct child of an AssignmentExpression, ensuring we visit reassignments. --- .../packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts | 7 ++++++- ...ression-references-later-variable-declaration.expect.md | 2 +- ...xpression-references-variable-its-assigned-to.expect.md | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts index 9425030dac..33158cbbd6 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts @@ -366,7 +366,12 @@ function lowerStatement( ArrowFunctionExpression: withFunctionContext, ObjectMethod: withFunctionContext, Identifier(id: NodePath) { - if (!id.isReferencedIdentifier()) { + const id2 = id; + if ( + !id2.isReferencedIdentifier() && + // isReferencedIdentifier is broken and returns false for reassignments + id.parent.type !== "AssignmentExpression" + ) { return; } const binding = id.scope.getBinding(id.node.name); diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-function-expression-references-later-variable-declaration.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-function-expression-references-later-variable-declaration.expect.md index 797ea6a278..d674395e54 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-function-expression-references-later-variable-declaration.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-function-expression-references-later-variable-declaration.expect.md @@ -20,7 +20,7 @@ function Component() { 1 | function Component() { 2 | let callback = () => { > 3 | onClick = () => {}; - | ^^^^^^^ [ReactForget] Invariant: [hoisting] Expected value kind to be initialized. read onClick$0_@1 (3:3) + | ^^^^^^^^^^^^^^^^^^ [ReactForget] Todo: Handle non-const declarations for hoisting. variable "onClick" declared with let (3:3) 4 | }; 5 | let onClick; 6 | diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-function-expression-references-variable-its-assigned-to.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-function-expression-references-variable-its-assigned-to.expect.md index 209f6bf447..db960286f4 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-function-expression-references-variable-its-assigned-to.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-function-expression-references-variable-its-assigned-to.expect.md @@ -18,7 +18,7 @@ function Component() { 1 | function Component() { 2 | let callback = () => { > 3 | callback = null; - | ^^^^^^^^ [ReactForget] Invariant: [hoisting] Expected value kind to be initialized. read callback$0_@0 (3:3) + | ^^^^^^^^^^^^^^^ [ReactForget] Todo: Handle non-const declarations for hoisting. variable "callback" declared with let (3:3) 4 | }; 5 | return
; 6 | }