Detect hoisting where the reference is a reassignment

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.
This commit is contained in:
Joe Savona
2024-03-22 13:49:50 -07:00
parent fb0f7e5c39
commit 4575c8a5f6
3 changed files with 8 additions and 3 deletions
@@ -366,7 +366,12 @@ function lowerStatement(
ArrowFunctionExpression: withFunctionContext,
ObjectMethod: withFunctionContext,
Identifier(id: NodePath<t.Identifier>) {
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);
@@ -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 |
@@ -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 <div onClick={callback} />;
6 | }