From 096e3431e440553217d2fffa7380b04abcba6ede Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Mon, 30 Sep 2024 15:56:58 -0400 Subject: [PATCH] [compiler] Test fixture: non-reactive phi creates 'dangling ref' scope ghstack-source-id: 5dba5c0b1d2e17c1d8d8a88a363f28ad7ac4746a Pull Request resolved: https://github.com/facebook/react/pull/31103 --- .../bug-invalid-phi-as-dependency.expect.md | 73 +++++++++++++++++++ .../bug-invalid-phi-as-dependency.tsx | 30 ++++++++ .../packages/snap/src/SproutTodoFilter.ts | 1 + 3 files changed, 104 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-phi-as-dependency.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-phi-as-dependency.tsx diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-phi-as-dependency.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-phi-as-dependency.expect.md new file mode 100644 index 0000000000000..8114b8a3738bc --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-phi-as-dependency.expect.md @@ -0,0 +1,73 @@ + +## Input + +```javascript +import { CONST_TRUE, Stringify, setProperty } from "shared-runtime"; + +function Component({arg}) { + const obj = CONST_TRUE ? {inner: {value: "hello"}} : null; + const boxedInner = [obj?.inner]; + useHook(); + setProperty(obj, arg); + if (boxedInner[0] !== obj?.inner) { + throw new Error("invariant broken"); + } + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{arg: 0}], + sequentialRenders: [{arg: 0}, {arg: 1}] +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { CONST_TRUE, Stringify, setProperty } from "shared-runtime"; + +function Component(t0) { + const $ = _c(5); + const { arg } = t0; + const obj = CONST_TRUE ? { inner: { value: "hello" } } : null; + const t1 = obj?.inner; + let t2; + if ($[0] !== t1) { + t2 = [t1]; + $[0] = t1; + $[1] = t2; + } else { + t2 = $[1]; + } + const boxedInner = t2; + useHook(); + setProperty(obj, arg); + if (boxedInner[0] !== obj?.inner) { + throw new Error("invariant broken"); + } + let t3; + if ($[2] !== obj || $[3] !== boxedInner) { + t3 = ; + $[2] = obj; + $[3] = boxedInner; + $[4] = t3; + } else { + t3 = $[4]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ arg: 0 }], + sequentialRenders: [{ arg: 0 }, { arg: 1 }], +}; + +``` + +### Eval output +(kind: ok) [[ (exception in render) ReferenceError: useHook is not defined ]] +[[ (exception in render) ReferenceError: useHook is not defined ]] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-phi-as-dependency.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-phi-as-dependency.tsx new file mode 100644 index 0000000000000..f3be6660623af --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-phi-as-dependency.tsx @@ -0,0 +1,30 @@ +import { CONST_TRUE, Stringify, mutate, useIdentity } from "shared-runtime"; + +/** + * Fixture showing an edge case for ReactiveScope variable propagation. + * + * Found differences in evaluator results + * Non-forget (expected): + *
{"obj":{"inner":{"value":"hello"},"wat0":"joe"},"inner":["[[ cyclic ref *2 ]]"]}
+ *
{"obj":{"inner":{"value":"hello"},"wat0":"joe"},"inner":["[[ cyclic ref *2 ]]"]}
+ * Forget: + *
{"obj":{"inner":{"value":"hello"},"wat0":"joe"},"inner":["[[ cyclic ref *2 ]]"]}
+ * [[ (exception in render) Error: invariant broken ]] + * + */ +function Component() { + const obj = CONST_TRUE ? {inner: {value: "hello"}} : null; + const boxedInner = [obj?.inner]; + useIdentity(null); + mutate(obj); + if (boxedInner[0] !== obj?.inner) { + throw new Error("invariant broken"); + } + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{arg: 0}], + sequentialRenders: [{arg: 0}, {arg: 1}] +} diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index 8597f66dbd788..5baca4b5dcd62 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -479,6 +479,7 @@ const skipFilter = new Set([ 'fbt/bug-fbt-plural-multiple-mixed-call-tag', 'bug-invalid-hoisting-functionexpr', 'bug-try-catch-maybe-null-dependency', + 'bug-invalid-phi-as-dependency', 'reduce-reactive-deps/bug-infer-function-cond-access-not-hoisted', 'reduce-reactive-deps/bug-merge-uncond-optional-chain-and-cond', 'original-reactive-scopes-fork/bug-nonmutating-capture-in-unsplittable-memo-block',