diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CollectReactiveIdentifiers.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CollectReactiveIdentifiers.ts index b4e262158a851..18e37f20f9798 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CollectReactiveIdentifiers.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CollectReactiveIdentifiers.ts @@ -9,7 +9,9 @@ import { IdentifierId, InstructionId, Place, + PrunedReactiveScopeBlock, ReactiveFunction, + isPrimitiveType, } from "../HIR/HIR"; import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors"; @@ -40,6 +42,19 @@ class Visitor extends ReactiveFunctionVisitor> { state.add(place.identifier.id); } } + + override visitPrunedScope( + scopeBlock: PrunedReactiveScopeBlock, + state: Set + ): void { + this.traversePrunedScope(scopeBlock, state); + + for (const [id, decl] of scopeBlock.scope.declarations) { + if (!isPrimitiveType(decl.identifier)) { + state.add(id); + } + } + } } /* diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-pruned-scope-leaks-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-pruned-scope-leaks-value.expect.md deleted file mode 100644 index d490c6e8f32f5..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-pruned-scope-leaks-value.expect.md +++ /dev/null @@ -1,119 +0,0 @@ - -## Input - -```javascript -import invariant from "invariant"; -import { - makeObject_Primitives, - mutate, - sum, - useIdentity, -} from "shared-runtime"; - -/** - * Exposes fundamental issue with pruning 'non-reactive' dependencies + flattening - * those scopes. Here, `z`'s original memo block is removed due to the inner hook call. - * However, we also infer that `z` is non-reactive and does not need to be a memo - * dependency. - * - * Current evaluator error: - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] - * [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] - * [5,{"a":0,"b":"value1","c":true,"wat0":"joe"}] - * Forget: - * (kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] - * [[ (exception in render) Invariant Violation: oh no! ]] - * [5,{"a":0,"b":"value1","c":true,"wat0":"joe"}] - */ - -function MyApp({ count }) { - const z = makeObject_Primitives(); - const x = useIdentity(2); - const y = sum(x, count); - mutate(z); - const thing = [y, z]; - if (thing[1] !== z) { - invariant(false, "oh no!"); - } - return thing; -} - -export const FIXTURE_ENTRYPOINT = { - fn: MyApp, - params: [{ count: 2 }], - sequentialRenders: [{ count: 2 }, { count: 2 }, { count: 3 }], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; -import invariant from "invariant"; -import { - makeObject_Primitives, - mutate, - sum, - useIdentity, -} from "shared-runtime"; - -/** - * Exposes fundamental issue with pruning 'non-reactive' dependencies + flattening - * those scopes. Here, `z`'s original memo block is removed due to the inner hook call. - * However, we also infer that `z` is non-reactive and does not need to be a memo - * dependency. - * - * Current evaluator error: - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] - * [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] - * [5,{"a":0,"b":"value1","c":true,"wat0":"joe"}] - * Forget: - * (kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] - * [[ (exception in render) Invariant Violation: oh no! ]] - * [5,{"a":0,"b":"value1","c":true,"wat0":"joe"}] - */ - -function MyApp(t0) { - const $ = _c(5); - const { count } = t0; - const z = makeObject_Primitives(); - const x = useIdentity(2); - let t1; - if ($[0] !== x || $[1] !== count) { - t1 = sum(x, count); - $[0] = x; - $[1] = count; - $[2] = t1; - } else { - t1 = $[2]; - } - const y = t1; - mutate(z); - let t2; - if ($[3] !== y) { - t2 = [y, z]; - $[3] = y; - $[4] = t2; - } else { - t2 = $[4]; - } - const thing = t2; - if (thing[1] !== z) { - invariant(false, "oh no!"); - } - return thing; -} - -export const FIXTURE_ENTRYPOINT = { - fn: MyApp, - params: [{ count: 2 }], - sequentialRenders: [{ count: 2 }, { count: 2 }, { count: 3 }], -}; - -``` - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-pruned-scope-leaks-value.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-pruned-scope-leaks-value.ts deleted file mode 100644 index 5b9c2527ae6f2..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-pruned-scope-leaks-value.ts +++ /dev/null @@ -1,43 +0,0 @@ -import invariant from "invariant"; -import { - makeObject_Primitives, - mutate, - sum, - useIdentity, -} from "shared-runtime"; - -/** - * Exposes fundamental issue with pruning 'non-reactive' dependencies + flattening - * those scopes. Here, `z`'s original memo block is removed due to the inner hook call. - * However, we also infer that `z` is non-reactive and does not need to be a memo - * dependency. - * - * Current evaluator error: - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] - * [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] - * [5,{"a":0,"b":"value1","c":true,"wat0":"joe"}] - * Forget: - * (kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] - * [[ (exception in render) Invariant Violation: oh no! ]] - * [5,{"a":0,"b":"value1","c":true,"wat0":"joe"}] - */ - -function MyApp({ count }) { - const z = makeObject_Primitives(); - const x = useIdentity(2); - const y = sum(x, count); - mutate(z); - const thing = [y, z]; - if (thing[1] !== z) { - invariant(false, "oh no!"); - } - return thing; -} - -export const FIXTURE_ENTRYPOINT = { - fn: MyApp, - params: [{ count: 2 }], - sequentialRenders: [{ count: 2 }, { count: 2 }, { count: 3 }], -}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-from-interleaved-reactivity-for-of.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-from-interleaved-reactivity-for-of.expect.md index e0441ff680e5c..a8b2da0491eae 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-from-interleaved-reactivity-for-of.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-from-interleaved-reactivity-for-of.expect.md @@ -39,7 +39,7 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; function Component(props) { - const $ = _c(1); + const $ = _c(2); const a = []; const b = []; @@ -53,11 +53,12 @@ function Component(props) { x = 1; } let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + if ($[0] !== x) { t0 = [x]; - $[0] = t0; + $[0] = x; + $[1] = t0; } else { - t0 = $[0]; + t0 = $[1]; } return t0; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-dont-memoize-array-with-mutable-map-after-hook.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-dont-memoize-array-with-mutable-map-after-hook.expect.md index 1c96a67ea9b07..e481a72bf434b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-dont-memoize-array-with-mutable-map-after-hook.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-dont-memoize-array-with-mutable-map-after-hook.expect.md @@ -39,7 +39,7 @@ import { useEffect, useState } from "react"; import { mutate } from "shared-runtime"; function Component(props) { - const $ = _c(6); + const $ = _c(8); const x = [{ ...props.value }]; let t0; let t1; @@ -64,25 +64,27 @@ function Component(props) { return {item.text}; }); let t3; - if ($[2] === Symbol.for("react.memo_cache_sentinel")) { + if ($[2] !== y) { t3 = mutate(y); - $[2] = t3; + $[2] = y; + $[3] = t3; } else { - t3 = $[2]; + t3 = $[3]; } let t4; - if ($[3] !== onClick || $[4] !== t2) { + if ($[4] !== onClick || $[5] !== t2 || $[6] !== t3) { t4 = (
{t2} {t3}
); - $[3] = onClick; - $[4] = t2; - $[5] = t4; + $[4] = onClick; + $[5] = t2; + $[6] = t3; + $[7] = t4; } else { - t4 = $[5]; + t4 = $[7]; } return t4; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-pruned-scope-leaks-value-via-alias.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-pruned-scope-leaks-value-via-alias.expect.md new file mode 100644 index 0000000000000..ebd9f413f7d98 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-pruned-scope-leaks-value-via-alias.expect.md @@ -0,0 +1,112 @@ + +## Input + +```javascript +import invariant from "invariant"; +import { + makeObject_Primitives, + mutate, + sum, + useIdentity, +} from "shared-runtime"; + +/** + * Here, `z`'s original memo block is removed due to the inner hook call. + * However, we also infer that `z` is non-reactive, so by default we would create + * the memo block for `thing = [y, z]` as only depending on `y`. + * + * This could then mean that `thing[1]` and `z` may not refer to the same value, + * since z recreates every time but `thing` doesn't correspondingly invalidate. + * + * The fix is to consider pruned memo block outputs as reactive, since they will + * recreate on every render. This means `thing` depends on both y and z. + */ +function MyApp({ count }) { + const z = makeObject_Primitives(); + const x = useIdentity(2); + const y = sum(x, count); + mutate(z); + const z2 = z; + const thing = [y, z2]; + if (thing[1] !== z) { + invariant(false, "oh no!"); + } + return thing; +} + +export const FIXTURE_ENTRYPOINT = { + fn: MyApp, + params: [{ count: 2 }], + sequentialRenders: [{ count: 2 }, { count: 2 }, { count: 3 }], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import invariant from "invariant"; +import { + makeObject_Primitives, + mutate, + sum, + useIdentity, +} from "shared-runtime"; + +/** + * Here, `z`'s original memo block is removed due to the inner hook call. + * However, we also infer that `z` is non-reactive, so by default we would create + * the memo block for `thing = [y, z]` as only depending on `y`. + * + * This could then mean that `thing[1]` and `z` may not refer to the same value, + * since z recreates every time but `thing` doesn't correspondingly invalidate. + * + * The fix is to consider pruned memo block outputs as reactive, since they will + * recreate on every render. This means `thing` depends on both y and z. + */ +function MyApp(t0) { + const $ = _c(6); + const { count } = t0; + const z = makeObject_Primitives(); + const x = useIdentity(2); + let t1; + if ($[0] !== x || $[1] !== count) { + t1 = sum(x, count); + $[0] = x; + $[1] = count; + $[2] = t1; + } else { + t1 = $[2]; + } + const y = t1; + mutate(z); + const z2 = z; + let t2; + if ($[3] !== y || $[4] !== z2) { + t2 = [y, z2]; + $[3] = y; + $[4] = z2; + $[5] = t2; + } else { + t2 = $[5]; + } + const thing = t2; + if (thing[1] !== z) { + invariant(false, "oh no!"); + } + return thing; +} + +export const FIXTURE_ENTRYPOINT = { + fn: MyApp, + params: [{ count: 2 }], + sequentialRenders: [{ count: 2 }, { count: 2 }, { count: 3 }], +}; + +``` + +### Eval output +(kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] +[4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] +[5,{"a":0,"b":"value1","c":true,"wat0":"joe"}] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-pruned-scope-leaks-value-via-alias.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-pruned-scope-leaks-value-via-alias.ts new file mode 100644 index 0000000000000..aabd79b6320c4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-pruned-scope-leaks-value-via-alias.ts @@ -0,0 +1,37 @@ +import invariant from "invariant"; +import { + makeObject_Primitives, + mutate, + sum, + useIdentity, +} from "shared-runtime"; + +/** + * Here, `z`'s original memo block is removed due to the inner hook call. + * However, we also infer that `z` is non-reactive, so by default we would create + * the memo block for `thing = [y, z]` as only depending on `y`. + * + * This could then mean that `thing[1]` and `z` may not refer to the same value, + * since z recreates every time but `thing` doesn't correspondingly invalidate. + * + * The fix is to consider pruned memo block outputs as reactive, since they will + * recreate on every render. This means `thing` depends on both y and z. + */ +function MyApp({ count }) { + const z = makeObject_Primitives(); + const x = useIdentity(2); + const y = sum(x, count); + mutate(z); + const z2 = z; + const thing = [y, z2]; + if (thing[1] !== z) { + invariant(false, "oh no!"); + } + return thing; +} + +export const FIXTURE_ENTRYPOINT = { + fn: MyApp, + params: [{ count: 2 }], + sequentialRenders: [{ count: 2 }, { count: 2 }, { count: 3 }], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-pruned-scope-leaks-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-pruned-scope-leaks-value.expect.md new file mode 100644 index 0000000000000..22c69221af621 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-pruned-scope-leaks-value.expect.md @@ -0,0 +1,110 @@ + +## Input + +```javascript +import invariant from "invariant"; +import { + makeObject_Primitives, + mutate, + sum, + useIdentity, +} from "shared-runtime"; + +/** + * Here, `z`'s original memo block is removed due to the inner hook call. + * However, we also infer that `z` is non-reactive, so by default we would create + * the memo block for `thing = [y, z]` as only depending on `y`. + * + * This could then mean that `thing[1]` and `z` may not refer to the same value, + * since z recreates every time but `thing` doesn't correspondingly invalidate. + * + * The fix is to consider pruned memo block outputs as reactive, since they will + * recreate on every render. This means `thing` depends on both y and z. + */ +function MyApp({ count }) { + const z = makeObject_Primitives(); + const x = useIdentity(2); + const y = sum(x, count); + mutate(z); + const thing = [y, z]; + if (thing[1] !== z) { + invariant(false, "oh no!"); + } + return thing; +} + +export const FIXTURE_ENTRYPOINT = { + fn: MyApp, + params: [{ count: 2 }], + sequentialRenders: [{ count: 2 }, { count: 2 }, { count: 3 }], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import invariant from "invariant"; +import { + makeObject_Primitives, + mutate, + sum, + useIdentity, +} from "shared-runtime"; + +/** + * Here, `z`'s original memo block is removed due to the inner hook call. + * However, we also infer that `z` is non-reactive, so by default we would create + * the memo block for `thing = [y, z]` as only depending on `y`. + * + * This could then mean that `thing[1]` and `z` may not refer to the same value, + * since z recreates every time but `thing` doesn't correspondingly invalidate. + * + * The fix is to consider pruned memo block outputs as reactive, since they will + * recreate on every render. This means `thing` depends on both y and z. + */ +function MyApp(t0) { + const $ = _c(6); + const { count } = t0; + const z = makeObject_Primitives(); + const x = useIdentity(2); + let t1; + if ($[0] !== x || $[1] !== count) { + t1 = sum(x, count); + $[0] = x; + $[1] = count; + $[2] = t1; + } else { + t1 = $[2]; + } + const y = t1; + mutate(z); + let t2; + if ($[3] !== y || $[4] !== z) { + t2 = [y, z]; + $[3] = y; + $[4] = z; + $[5] = t2; + } else { + t2 = $[5]; + } + const thing = t2; + if (thing[1] !== z) { + invariant(false, "oh no!"); + } + return thing; +} + +export const FIXTURE_ENTRYPOINT = { + fn: MyApp, + params: [{ count: 2 }], + sequentialRenders: [{ count: 2 }, { count: 2 }, { count: 3 }], +}; + +``` + +### Eval output +(kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] +[4,{"a":0,"b":"value1","c":true,"wat0":"joe"}] +[5,{"a":0,"b":"value1","c":true,"wat0":"joe"}] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-pruned-scope-leaks-value.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-pruned-scope-leaks-value.ts new file mode 100644 index 0000000000000..1940a2ffca135 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-pruned-scope-leaks-value.ts @@ -0,0 +1,36 @@ +import invariant from "invariant"; +import { + makeObject_Primitives, + mutate, + sum, + useIdentity, +} from "shared-runtime"; + +/** + * Here, `z`'s original memo block is removed due to the inner hook call. + * However, we also infer that `z` is non-reactive, so by default we would create + * the memo block for `thing = [y, z]` as only depending on `y`. + * + * This could then mean that `thing[1]` and `z` may not refer to the same value, + * since z recreates every time but `thing` doesn't correspondingly invalidate. + * + * The fix is to consider pruned memo block outputs as reactive, since they will + * recreate on every render. This means `thing` depends on both y and z. + */ +function MyApp({ count }) { + const z = makeObject_Primitives(); + const x = useIdentity(2); + const y = sum(x, count); + mutate(z); + const thing = [y, z]; + if (thing[1] !== z) { + invariant(false, "oh no!"); + } + return thing; +} + +export const FIXTURE_ENTRYPOINT = { + fn: MyApp, + params: [{ count: 2 }], + sequentialRenders: [{ count: 2 }, { count: 2 }, { count: 3 }], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-reactivity-value-block.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-reactivity-value-block.expect.md similarity index 52% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-reactivity-value-block.expect.md rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-reactivity-value-block.expect.md index baba119a4b09b..b4a158a7acff4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-reactivity-value-block.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-reactivity-value-block.expect.md @@ -8,17 +8,16 @@ import { makeObject_Primitives, useNoAlias, } from "shared-runtime"; + /** - * BUG - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) [{"a":0,"b":"value1","c":true},"[[ cyclic ref *1 ]]"] - * [{"a":0,"b":"value1","c":true},"[[ cyclic ref *1 ]]"] - * Forget: - * (kind: ok) [{"a":0,"b":"value1","c":true},"[[ cyclic ref *1 ]]"] - * [[ (exception in render) Error: Oh no! ]] + * Here the scope for `obj` is pruned because it spans the `useNoAlias()` hook call. + * Because `obj` is non-reactive, it would by default be excluded as dependency for + * `result = [...identity(obj)..., obj]`, but this could then cause the values in + * `result` to be out of sync with `obj`. + * + * The fix is to consider pruned memo block outputs as reactive, since they will + * recreate on every render. This means `thing` depends on both y and z. */ - function Foo() { const obj = makeObject_Primitives(); // hook calls keeps the next two lines as its own reactive scope @@ -53,19 +52,18 @@ import { makeObject_Primitives, useNoAlias, } from "shared-runtime"; + /** - * BUG - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) [{"a":0,"b":"value1","c":true},"[[ cyclic ref *1 ]]"] - * [{"a":0,"b":"value1","c":true},"[[ cyclic ref *1 ]]"] - * Forget: - * (kind: ok) [{"a":0,"b":"value1","c":true},"[[ cyclic ref *1 ]]"] - * [[ (exception in render) Error: Oh no! ]] + * Here the scope for `obj` is pruned because it spans the `useNoAlias()` hook call. + * Because `obj` is non-reactive, it would by default be excluded as dependency for + * `result = [...identity(obj)..., obj]`, but this could then cause the values in + * `result` to be out of sync with `obj`. + * + * The fix is to consider pruned memo block outputs as reactive, since they will + * recreate on every render. This means `thing` depends on both y and z. */ - function Foo() { - const $ = _c(1); + const $ = _c(3); const obj = makeObject_Primitives(); useNoAlias(); @@ -73,11 +71,13 @@ function Foo() { const shouldCaptureObj = obj != null && CONST_TRUE; const t0 = shouldCaptureObj ? identity(obj) : null; let t1; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + if ($[0] !== t0 || $[1] !== obj) { t1 = [t0, obj]; - $[0] = t1; + $[0] = t0; + $[1] = obj; + $[2] = t1; } else { - t1 = $[0]; + t1 = $[2]; } const result = t1; @@ -95,4 +95,7 @@ export const FIXTURE_ENTRYPOINT = { }; ``` - \ No newline at end of file + +### Eval output +(kind: ok) [{"a":0,"b":"value1","c":true},"[[ cyclic ref *1 ]]"] +[{"a":0,"b":"value1","c":true},"[[ cyclic ref *1 ]]"] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-reactivity-value-block.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-reactivity-value-block.ts similarity index 57% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-reactivity-value-block.ts rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-reactivity-value-block.ts index 0205270757187..2aebe594ef4e7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-reactivity-value-block.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-invalid-reactivity-value-block.ts @@ -4,17 +4,16 @@ import { makeObject_Primitives, useNoAlias, } from "shared-runtime"; + /** - * BUG - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) [{"a":0,"b":"value1","c":true},"[[ cyclic ref *1 ]]"] - * [{"a":0,"b":"value1","c":true},"[[ cyclic ref *1 ]]"] - * Forget: - * (kind: ok) [{"a":0,"b":"value1","c":true},"[[ cyclic ref *1 ]]"] - * [[ (exception in render) Error: Oh no! ]] + * Here the scope for `obj` is pruned because it spans the `useNoAlias()` hook call. + * Because `obj` is non-reactive, it would by default be excluded as dependency for + * `result = [...identity(obj)..., obj]`, but this could then cause the values in + * `result` to be out of sync with `obj`. + * + * The fix is to consider pruned memo block outputs as reactive, since they will + * recreate on every render. This means `thing` depends on both y and z. */ - function Foo() { const obj = makeObject_Primitives(); // hook calls keeps the next two lines as its own reactive scope diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/unmemoized-nonreactive-dependency-is-pruned-as-dependency.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/unmemoized-nonreactive-dependency-is-pruned-as-dependency.expect.md index cc829c9190fa3..96ac447902bd5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/unmemoized-nonreactive-dependency-is-pruned-as-dependency.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/unmemoized-nonreactive-dependency-is-pruned-as-dependency.expect.md @@ -25,23 +25,13 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; import { mutate, useNoAlias } from "shared-runtime"; function Component(props) { - const $ = _c(1); - const x = []; useNoAlias(); mutate(x); - let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - t0 =
{x}
; - $[0] = t0; - } else { - t0 = $[0]; - } - return t0; + return
{x}
; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useCallback-call-second-function-which-captures-maybe-mutable-value-dont-preserve-memoization.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useCallback-call-second-function-which-captures-maybe-mutable-value-dont-preserve-memoization.expect.md index b43bac8f7d431..797ffda2812a4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useCallback-call-second-function-which-captures-maybe-mutable-value-dont-preserve-memoization.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useCallback-call-second-function-which-captures-maybe-mutable-value-dont-preserve-memoization.expect.md @@ -49,7 +49,7 @@ import { } from "shared-runtime"; function Component(props) { - const $ = _c(1); + const $ = _c(2); const object = makeObject_Primitives(); useHook(); @@ -64,11 +64,12 @@ function Component(props) { identity(object); let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + if ($[0] !== onClick) { t0 =
; - $[0] = t0; + $[0] = onClick; + $[1] = t0; } else { - t0 = $[0]; + t0 = $[1]; } return t0; } diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index 0bfa03c3971e5..4972eee5f51e5 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -485,8 +485,6 @@ const skipFilter = new Set([ "rules-of-hooks/rules-of-hooks-69521d94fa03", // bugs - "bug-invalid-reactivity-value-block", - "bug-invalid-pruned-scope-leaks-value", "bug-invalid-hoisting-functionexpr", "original-reactive-scopes-fork/bug-nonmutating-capture-in-unsplittable-memo-block", "original-reactive-scopes-fork/bug-hoisted-declaration-with-scope",