Skip to content

Commit

Permalink
[compiler] Repro for nested function local reassignment issue
Browse files Browse the repository at this point in the history
Summary: Additional repro demonstrating a case that still exists after #30106

[ghstack-poisoned]
  • Loading branch information
mvitousek committed Jun 26, 2024
1 parent 00775d9 commit 5bcc119
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@

## Input

```javascript
import { useEffect } from "react";
function Component() {
let local;
const mk_reassignlocal = () => {
// Create the reassignment function inside another function, then return it
const reassignLocal = (newValue) => {
local = newValue;
};
return reassignLocal;
}
const reassignLocal = mk_reassignlocal();
const onMount = (newValue) => {
reassignLocal("hello");
if (local === newValue) {
// Without React Compiler, `reassignLocal` is freshly created
// on each render, capturing a binding to the latest `local`,
// such that invoking reassignLocal will reassign the same
// binding that we are observing in the if condition, and
// we reach this branch
console.log("`local` was updated!");
} else {
// With React Compiler enabled, `reassignLocal` is only created
// once, capturing a binding to `local` in that render pass.
// Therefore, calling `reassignLocal` will reassign the wrong
// version of `local`, and not update the binding we are checking
// in the if condition.
//
// To protect against this, we disallow reassigning locals from
// functions that escape
throw new Error("`local` not updated!");
}
};
useEffect(() => {
onMount();
}, [onMount]);
return "ok";
}

```

## Code

```javascript
import { c as _c } from "react/compiler-runtime";
import { useEffect } from "react";
function Component() {
const $ = _c(4);
let local;
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
const mk_reassignlocal = () => {
const reassignLocal = (newValue) => {
local = newValue;
};
return reassignLocal;
};

t0 = mk_reassignlocal();
$[0] = t0;
} else {
t0 = $[0];
}
const reassignLocal_0 = t0;
let t1;
if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
t1 = (newValue_0) => {
reassignLocal_0("hello");
if (local === newValue_0) {
console.log("`local` was updated!");
} else {
throw new Error("`local` not updated!");
}
};
$[1] = t1;
} else {
t1 = $[1];
}
const onMount = t1;
let t2;
let t3;
if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
t2 = () => {
onMount();
};
t3 = [onMount];
$[2] = t2;
$[3] = t3;
} else {
t2 = $[2];
t3 = $[3];
}
useEffect(t2, t3);
return "ok";
}

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { useEffect } from "react";
function Component() {
let local;
const mk_reassignlocal = () => {
// Create the reassignment function inside another function, then return it
const reassignLocal = (newValue) => {
local = newValue;
};
return reassignLocal;
}
const reassignLocal = mk_reassignlocal();
const onMount = (newValue) => {
reassignLocal("hello");
if (local === newValue) {
// Without React Compiler, `reassignLocal` is freshly created
// on each render, capturing a binding to the latest `local`,
// such that invoking reassignLocal will reassign the same
// binding that we are observing in the if condition, and
// we reach this branch
console.log("`local` was updated!");
} else {
// With React Compiler enabled, `reassignLocal` is only created
// once, capturing a binding to `local` in that render pass.
// Therefore, calling `reassignLocal` will reassign the wrong
// version of `local`, and not update the binding we are checking
// in the if condition.
//
// To protect against this, we disallow reassigning locals from
// functions that escape
throw new Error("`local` not updated!");
}
};
useEffect(() => {
onMount();
}, [onMount]);
return "ok";
}
2 changes: 2 additions & 0 deletions compiler/packages/snap/src/SproutTodoFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,8 @@ const skipFilter = new Set([

// needs to be executed as a module
"meta-property",

"todo.invalid-nested-function-reassign-local-variable-in-effect",
]);

export default skipFilter;

0 comments on commit 5bcc119

Please sign in to comment.