diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonReactiveDependencies.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonReactiveDependencies.ts index 4501487f1e..6e8c3b903a 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonReactiveDependencies.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonReactiveDependencies.ts @@ -5,7 +5,14 @@ * LICENSE file in the root directory of this source tree. */ -import { IdentifierId, ReactiveFunction, ReactiveScopeBlock } from "../HIR"; +import { + IdentifierId, + ReactiveFunction, + ReactiveInstruction, + ReactiveScopeBlock, + isSetStateType, +} from "../HIR"; +import { eachPatternOperand } from "../HIR/visitors"; import { collectReactiveIdentifiers } from "./CollectReactiveIdentifiers"; import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors"; @@ -23,27 +30,90 @@ export function pruneNonReactiveDependencies(fn: ReactiveFunction): void { type ReactiveIdentifiers = Set; class Visitor extends ReactiveFunctionVisitor { - override visitScope( - scope: ReactiveScopeBlock, + override visitInstruction( + instruction: ReactiveInstruction, state: ReactiveIdentifiers ): void { - this.traverseScope(scope, state); - for (const dep of scope.scope.dependencies) { - const isReactive = state.has(dep.identifier.id); - if (!isReactive) { - scope.scope.dependencies.delete(dep); + this.traverseInstruction(instruction, state); + + const { lvalue, value } = instruction; + switch (value.kind) { + case "LoadLocal": { + if (lvalue !== null && state.has(value.place.identifier.id)) { + state.add(lvalue.identifier.id); + } + break; + } + case "StoreLocal": { + if (state.has(value.value.identifier.id)) { + state.add(value.lvalue.place.identifier.id); + if (lvalue !== null) { + state.add(lvalue.identifier.id); + } + } + break; + } + case "Destructure": { + if (state.has(value.value.identifier.id)) { + for (const lvalue of eachPatternOperand(value.lvalue.pattern)) { + if (isSetStateType(lvalue.identifier)) { + continue; + } + state.add(lvalue.identifier.id); + } + if (lvalue !== null) { + state.add(lvalue.identifier.id); + } + } + break; + } + case "PropertyLoad": { + if ( + lvalue !== null && + state.has(value.object.identifier.id) && + !isSetStateType(lvalue.identifier) + ) { + state.add(lvalue.identifier.id); + } + break; + } + case "ComputedLoad": { + if ( + lvalue !== null && + (state.has(value.object.identifier.id) || + state.has(value.property.identifier.id)) + ) { + state.add(lvalue.identifier.id); + } + break; } } - if (scope.scope.dependencies.size === 0) { - // If a scope has no dependencies, then its declarations are all non-reactive - for (const [, declaration] of scope.scope.declarations) { - state.delete(declaration.identifier.id); + } + + override visitScope( + scopeBlock: ReactiveScopeBlock, + state: ReactiveIdentifiers + ): void { + this.traverseScope(scopeBlock, state); + for (const dep of scopeBlock.scope.dependencies) { + const isReactive = state.has(dep.identifier.id); + if (!isReactive) { + scopeBlock.scope.dependencies.delete(dep); } - } else { - // otherwise, all the scope's declarations are reactive - for (const [, declaration] of scope.scope.declarations) { + } + if (scopeBlock.scope.dependencies.size !== 0) { + /** + * If any of a scope's dependencies are reactive, then all of its + * outputs will re-evaluate whenever those dependencies change. + * Mark all of the outputs as reactive to reflect the fact that + * they may change in practice based on a reactive input. + */ + for (const [, declaration] of scopeBlock.scope.declarations) { state.add(declaration.identifier.id); } + for (const reassignment of scopeBlock.scope.reassignments) { + state.add(reassignment.id); + } } } } diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-from-interleaved-reactivity-if.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-from-interleaved-reactivity-if.expect.md new file mode 100644 index 0000000000..098077f59f --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-from-interleaved-reactivity-if.expect.md @@ -0,0 +1,77 @@ + +## Input + +```javascript +function Component(props) { + // a and b are independent but their mutations are interleaved, so + // they get grouped in a reactive scope. this means that a becomes + // reactive since it will effectively re-evaluate based on a reactive + // input + const a = []; + const b = []; + b.push(props.cond); + a.push(null); + + // Downstream consumer of a, which initially seems non-reactive except + // that a becomes reactive, per above + const c = [a]; + + let x; + if (c[0]) { + x = 1; + } else { + x = 2; + } + // The values assigned to `x` are non-reactive, but the value of `x` + // depends on the "control" value `c[0]` which becomes reactive via + // being interleaved with `b`. + // Therefore x should be treated as reactive too. + return [x]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ cond: true }], +}; + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; +function Component(props) { + const $ = useMemoCache(1); + + const a = []; + const b = []; + b.push(props.cond); + a.push(null); + + const c = [a]; + + let x; + if (c[0]) { + x = 1; + } else { + x = 2; + } + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = [x]; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ cond: true }], +}; + +``` + +### Eval output +(kind: ok) [1] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-from-interleaved-reactivity-if.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-from-interleaved-reactivity-if.js new file mode 100644 index 0000000000..9c07d33b12 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-from-interleaved-reactivity-if.js @@ -0,0 +1,31 @@ +function Component(props) { + // a and b are independent but their mutations are interleaved, so + // they get grouped in a reactive scope. this means that a becomes + // reactive since it will effectively re-evaluate based on a reactive + // input + const a = []; + const b = []; + b.push(props.cond); + a.push(null); + + // Downstream consumer of a, which initially seems non-reactive except + // that a becomes reactive, per above + const c = [a]; + + let x; + if (c[0]) { + x = 1; + } else { + x = 2; + } + // The values assigned to `x` are non-reactive, but the value of `x` + // depends on the "control" value `c[0]` which becomes reactive via + // being interleaved with `b`. + // Therefore x should be treated as reactive too. + return [x]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ cond: true }], +}; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactivity-analysis-interleaved-reactivity.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactivity-analysis-interleaved-reactivity.expect.md index 22d2109a07..7cdb73ba9d 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactivity-analysis-interleaved-reactivity.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactivity-analysis-interleaved-reactivity.expect.md @@ -35,7 +35,7 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { unstable_useMemoCache as useMemoCache } from "react"; function Component(props) { - const $ = useMemoCache(6); + const $ = useMemoCache(7); let a; if ($[0] !== props.b) { a = {}; @@ -57,12 +57,13 @@ function Component(props) { } const c = t0; let t1; - if ($[4] !== a) { + if ($[4] !== c || $[5] !== a) { t1 = [c, a]; - $[4] = a; - $[5] = t1; + $[4] = c; + $[5] = a; + $[6] = t1; } else { - t1 = $[5]; + t1 = $[6]; } return t1; }