From 2ce112dbf2442c87f89b649e31dd132c170d64fd Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 22 Mar 2024 21:49:39 -0700 Subject: [PATCH] Repro for missing dep with while/if using externally declared "index" variable What happens here is that the phi node for `i` has its mutable range set to start at 0, because it has a back edge and we haven't initialized the mutable ranges of all its operands yet when we iterate the operands and set range.start = min(start of operand starts). Then the corresponding scope has its range set to start at 0 too. When PropagateScopeDeps runs it sees that `b` is from instruction 1, which is after the start of the scope (0), so it thinks `b` isn't a valid dependency. The fix is in InferMutableRanges, where we need to make sure that phis ignore their operand's ranges until those ranges are initialized. --- ...ssing-dependency-if-within-while.expect.md | 79 +++++++++++++++++++ ...epro-missing-dependency-if-within-while.js | 28 +++++++ .../packages/snap/src/SproutTodoFilter.ts | 1 + 3 files changed, 108 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-repro-missing-dependency-if-within-while.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-repro-missing-dependency-if-within-while.js diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-repro-missing-dependency-if-within-while.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-repro-missing-dependency-if-within-while.expect.md new file mode 100644 index 0000000000..cacf7df91a --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-repro-missing-dependency-if-within-while.expect.md @@ -0,0 +1,79 @@ + +## Input + +```javascript +const someGlobal = true; +export default function Component(props) { + const { b } = props; + const items = []; + let i = 0; + while (i < 10) { + if (someGlobal) { + items.push(
{b}
); + i++; + } + } + return <>{items}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ b: 42 }], + sequentialRenders: [ + { b: 0 }, + { b: 0 }, + { b: 42 }, + { b: 42 }, + { b: 0 }, + { b: 42 }, + { b: 0 }, + { b: 42 }, + ], +}; + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; +const someGlobal = true; +export default function Component(props) { + const $ = useMemoCache(1); + const { b } = props; + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + const items = []; + let i = 0; + while (i < 10) { + if (someGlobal) { + items.push(
{b}
); + i++; + } + } + + t0 = <>{items}; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ b: 42 }], + sequentialRenders: [ + { b: 0 }, + { b: 0 }, + { b: 42 }, + { b: 42 }, + { b: 0 }, + { b: 42 }, + { b: 0 }, + { b: 42 }, + ], +}; + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-repro-missing-dependency-if-within-while.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-repro-missing-dependency-if-within-while.js new file mode 100644 index 0000000000..9b07b42fbf --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-repro-missing-dependency-if-within-while.js @@ -0,0 +1,28 @@ +const someGlobal = true; +export default function Component(props) { + const { b } = props; + const items = []; + let i = 0; + while (i < 10) { + if (someGlobal) { + items.push(
{b}
); + i++; + } + } + return <>{items}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ b: 42 }], + sequentialRenders: [ + { b: 0 }, + { b: 0 }, + { b: 42 }, + { b: 42 }, + { b: 0 }, + { b: 42 }, + { b: 0 }, + { b: 42 }, + ], +}; diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index fef339c363..cbc2edd572 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -534,6 +534,7 @@ const skipFilter = new Set([ // bugs "bug-reduce-reactive-deps-return-in-scope", "bug-reduce-reactive-deps-break-in-scope", + "bug-repro-missing-dependency-if-within-while", // 'react-forget-runtime' not yet supported "flag-enable-emit-hook-guards",