From e7a580fd29cb2ec4ff01b6359e2811f1712be50e Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Thu, 21 Nov 2024 14:30:13 -0500 Subject: [PATCH] refactor to use scope dependency Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: --- .../src/Entrypoint/Pipeline.ts | 1 + .../src/Inference/InferEffectDependencies.ts | 189 ++++++++++++++++-- .../nonreactive-dep.expect.md | 80 ++++++++ .../infer-effect-deps/nonreactive-dep.js | 25 +++ .../nonreactive-ref.expect.md | 51 +++++ .../infer-effect-deps/nonreactive-ref.js | 14 ++ .../outlined-function.expect.md | 46 +++++ .../infer-effect-deps/outlined-function.js | 14 ++ .../pruned-nonreactive-obj.expect.md | 119 +++++++++++ .../pruned-nonreactive-obj.js | 48 +++++ .../reactive-memberexpr-merge.expect.md | 49 +++++ .../reactive-memberexpr-merge.js | 8 + .../reactive-memberexpr.expect.md | 49 +++++ .../infer-effect-deps/reactive-memberexpr.js | 8 + .../reactive-optional-chain.expect.md | 60 ++++++ .../reactive-optional-chain.js | 9 + .../reactive-variable.expect.md | 49 +++++ .../infer-effect-deps/reactive-variable.js | 8 + .../todo-import-namespace-useEffect.expect.md | 50 +++++ .../todo-import-namespace-useEffect.js | 10 + 20 files changed, 874 insertions(+), 13 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/nonreactive-dep.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/nonreactive-dep.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/nonreactive-ref.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/nonreactive-ref.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/outlined-function.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/outlined-function.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/pruned-nonreactive-obj.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/pruned-nonreactive-obj.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-memberexpr-merge.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-memberexpr-merge.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-memberexpr.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-memberexpr.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-optional-chain.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-optional-chain.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-variable.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-variable.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/todo-import-namespace-useEffect.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/todo-import-namespace-useEffect.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index bbd076e46a..42f6a89ca7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -21,6 +21,7 @@ import { lower, mergeConsecutiveBlocks, mergeOverlappingReactiveScopesHIR, + printHIR, pruneUnusedLabelsHIR, } from '../HIR'; import { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts index 55a252f53e..8bf5e09893 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts @@ -1,3 +1,4 @@ +import {CompilerError, SourceLocation} from '..'; import { ArrayExpression, Effect, @@ -9,8 +10,19 @@ import { Instruction, isUseEffectHookType, makeInstructionId, + TInstruction, + InstructionId, + ScopeId, + ReactiveScopeDependency, + Place, + ReactiveScopeDependencies, } from '../HIR'; -import {createTemporaryPlace} from '../HIR/HIRBuilder'; +import { + createTemporaryPlace, + fixScopeAndIdentifierRanges, + markInstructionIds, +} from '../HIR/HIRBuilder'; +import {eachInstructionOperand, eachTerminalOperand} from '../HIR/visitors'; /** * Infers reactive dependencies captured by useEffect lambdas and adds them as @@ -20,14 +32,48 @@ export function inferEffectDependencies( env: Environment, fn: HIRFunction, ): void { - const fnExpressions = new Map(); + let hasRewrite = false; + const fnExpressions = new Map< + IdentifierId, + TInstruction + >(); + const scopeInfos = new Map< + ScopeId, + {pruned: boolean; deps: ReactiveScopeDependencies; hasSingleInstr: boolean} + >(); + + /** + * When inserting LoadLocals, we need to retain the reactivity of the base + * identifier, as later passes e.g. PruneNonReactiveDeps take the reactivity of + * a base identifier as the "maximal" reactivity of all its references. + * Concretely, + * reactive(Identifier i) = Union_{reference of i}(reactive(reference)) + */ + const reactiveIds = inferReactiveIdentifiers(fn); for (const [, block] of fn.body.blocks) { - let rewriteInstrs = new Map(); + if ( + block.terminal.kind === 'scope' || + block.terminal.kind === 'pruned-scope' + ) { + const scopeBlock = fn.body.blocks.get(block.terminal.block)!; + scopeInfos.set(block.terminal.scope.id, { + pruned: block.terminal.kind === 'pruned-scope', + deps: block.terminal.scope.dependencies, + hasSingleInstr: + scopeBlock.instructions.length === 1 && + scopeBlock.terminal.kind === 'goto' && + scopeBlock.terminal.block === block.terminal.fallthrough, + }); + } + const rewriteInstrs = new Map>(); for (const instr of block.instructions) { const {value, lvalue} = instr; if (value.kind === 'FunctionExpression') { - fnExpressions.set(lvalue.identifier.id, value); + fnExpressions.set( + lvalue.identifier.id, + instr as TInstruction, + ); } else if ( /* * This check is not final. Right now we only look for useEffects without a dependency array. @@ -41,34 +87,73 @@ export function inferEffectDependencies( ) { const fnExpr = fnExpressions.get(value.args[0].identifier.id); if (fnExpr != null) { + const scopeInfo = + fnExpr.lvalue.identifier.scope != null + ? scopeInfos.get(fnExpr.lvalue.identifier.scope.id) + : null; + CompilerError.invariant(scopeInfo != null, { + reason: 'Expected function expression scope to exist', + loc: value.loc, + }); + if (scopeInfo.pruned || !scopeInfo.hasSingleInstr) { + // TODO: retry pipeline that ensures effect function expressions + // are placed into their own scope + CompilerError.throwTodo({ + reason: + '[InferEffectDependencies] Expected effect function to have non-pruned scope and its scope to have exactly one instruction', + loc: fnExpr.loc, + }); + } + + /** + * Step 1: write new instructions to insert a dependency array + */ + const effectDeps: Array = []; + const newInstructions: Array = []; + for (const dep of scopeInfo.deps) { + /** + * Invalid to prune non-reactive values in this pass, see the + * `infer-effect-deps/pruned-nonreactive-obj` fixture for an + * explanation + */ + const {place, instructions} = writeDependencyToInstructions( + dep, + reactiveIds.has(dep.identifier.id), + fn.env, + fnExpr.loc, + ); + newInstructions.push(...instructions); + effectDeps.push(place); + } const deps: ArrayExpression = { kind: 'ArrayExpression', - elements: fnExpr.loweredFunc.dependencies.filter( - place => place.reactive, - ), + elements: effectDeps, loc: GeneratedSource, }; const depsPlace = createTemporaryPlace(env, GeneratedSource); depsPlace.effect = Effect.Read; - value.args[1] = depsPlace; - const newInstruction: Instruction = { + newInstructions.push({ id: makeInstructionId(0), loc: GeneratedSource, - lvalue: depsPlace, + lvalue: {...depsPlace, effect: Effect.Mutate}, value: deps, - }; - rewriteInstrs.set(instr.id, newInstruction); + }); + + // Step 2: insert the deps array as an argument of the useEffect + value.args[1] = {...depsPlace, effect: Effect.Freeze}; + rewriteInstrs.set(instr.id, newInstructions); } } } if (rewriteInstrs.size > 0) { + hasRewrite = true; const newInstrs = []; for (const instr of block.instructions) { const newInstr = rewriteInstrs.get(instr.id); if (newInstr != null) { - newInstrs.push(newInstr, instr); + newInstrs.push(...newInstr, instr); } else { newInstrs.push(instr); } @@ -76,4 +161,82 @@ export function inferEffectDependencies( block.instructions = newInstrs; } } + if (hasRewrite) { + // Renumber instructions and fix scope ranges + markInstructionIds(fn.body); + fixScopeAndIdentifierRanges(fn.body); + } +} + +function writeDependencyToInstructions( + dep: ReactiveScopeDependency, + reactive: boolean, + env: Environment, + loc: SourceLocation, +): {place: Place; instructions: Array} { + const instructions: Array = []; + let currValue = createTemporaryPlace(env, GeneratedSource); + currValue.reactive = reactive; + instructions.push({ + id: makeInstructionId(0), + loc: GeneratedSource, + lvalue: {...currValue, effect: Effect.Mutate}, + value: { + kind: 'LoadLocal', + place: { + kind: 'Identifier', + identifier: dep.identifier, + effect: Effect.Capture, + reactive, + loc: loc, + }, + loc: loc, + }, + }); + for (const path of dep.path) { + if (path.optional) { + // TODO: instead of truncating optional paths, reuse + // instructions from hoisted dependencies block + break; + } + const nextValue = createTemporaryPlace(env, GeneratedSource); + nextValue.reactive = reactive; + instructions.push({ + id: makeInstructionId(0), + loc: GeneratedSource, + lvalue: {...nextValue, effect: Effect.Mutate}, + value: { + kind: 'PropertyLoad', + object: {...currValue, effect: Effect.Capture}, + property: path.property, + loc: loc, + }, + }); + currValue = nextValue; + } + currValue.effect = Effect.Freeze; + return {place: currValue, instructions}; +} + +function inferReactiveIdentifiers(fn: HIRFunction): Set { + const reactiveIds: Set = new Set(); + for (const [, block] of fn.body.blocks) { + for (const instr of block.instructions) { + // no need to traverse into nested functions as + // 1. Their effects are recorded in `LoweredFunction.dependencies` + // 2. we don't mark `reactive` in these anyways + for (const place of eachInstructionOperand(instr)) { + if (place.reactive) { + reactiveIds.add(place.identifier.id); + } + } + } + + for (const place of eachTerminalOperand(block.terminal)) { + if (place.reactive) { + reactiveIds.add(place.identifier.id); + } + } + } + return reactiveIds; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/nonreactive-dep.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/nonreactive-dep.expect.md new file mode 100644 index 0000000000..1cfbe71e78 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/nonreactive-dep.expect.md @@ -0,0 +1,80 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useEffect} from 'react'; +import {makeObject_Primitives, print} from 'shared-runtime'; + +/** + * Note that `obj` is currently added to the effect dependency array, even + * though it's non-reactive due to memoization. + * + * This is a TODO in effect dependency inference. Note that we cannot simply + * filter out non-reactive effect dependencies, as some non-reactive (by data + * flow) values become reactive due to scope pruning. See the + * `infer-effect-deps/pruned-nonreactive-obj` fixture for why this matters. + * + * Realizing that this `useEffect` should have an empty dependency array + * requires effect dependency inference to be structured similarly to memo + * dependency inference. + * Pass 1: add all potential dependencies regardless of dataflow reactivity + * Pass 2: (todo) prune non-reactive dependencies + * + * Note that instruction reordering should significantly reduce scope pruning + */ +function NonReactiveDepInEffect() { + const obj = makeObject_Primitives(); + useEffect(() => print(obj)); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import { useEffect } from "react"; +import { makeObject_Primitives, print } from "shared-runtime"; + +/** + * Note that `obj` is currently added to the effect dependency array, even + * though it's non-reactive due to memoization. + * + * This is a TODO in effect dependency inference. Note that we cannot simply + * filter out non-reactive effect dependencies, as some non-reactive (by data + * flow) values become reactive due to scope pruning. See the + * `infer-effect-deps/pruned-nonreactive-obj` fixture for why this matters. + * + * Realizing that this `useEffect` should have an empty dependency array + * requires effect dependency inference to be structured similarly to memo + * dependency inference. + * Pass 1: add all potential dependencies regardless of dataflow reactivity + * Pass 2: (todo) prune non-reactive dependencies + * + * Note that instruction reordering should significantly reduce scope pruning + */ +function NonReactiveDepInEffect() { + const $ = _c(2); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = makeObject_Primitives(); + $[0] = t0; + } else { + t0 = $[0]; + } + const obj = t0; + let t1; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + t1 = () => print(obj); + $[1] = t1; + } else { + t1 = $[1]; + } + useEffect(t1, [obj]); +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/nonreactive-dep.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/nonreactive-dep.js new file mode 100644 index 0000000000..85d9699750 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/nonreactive-dep.js @@ -0,0 +1,25 @@ +// @inferEffectDependencies +import {useEffect} from 'react'; +import {makeObject_Primitives, print} from 'shared-runtime'; + +/** + * Note that `obj` is currently added to the effect dependency array, even + * though it's non-reactive due to memoization. + * + * This is a TODO in effect dependency inference. Note that we cannot simply + * filter out non-reactive effect dependencies, as some non-reactive (by data + * flow) values become reactive due to scope pruning. See the + * `infer-effect-deps/pruned-nonreactive-obj` fixture for why this matters. + * + * Realizing that this `useEffect` should have an empty dependency array + * requires effect dependency inference to be structured similarly to memo + * dependency inference. + * Pass 1: add all potential dependencies regardless of dataflow reactivity + * Pass 2: (todo) prune non-reactive dependencies + * + * Note that instruction reordering should significantly reduce scope pruning + */ +function NonReactiveDepInEffect() { + const obj = makeObject_Primitives(); + useEffect(() => print(obj)); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/nonreactive-ref.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/nonreactive-ref.expect.md new file mode 100644 index 0000000000..cf1ebd9252 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/nonreactive-ref.expect.md @@ -0,0 +1,51 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useEffect, useRef} from 'react'; +import {print} from 'shared-runtime'; + +/** + * Special case of `infer-effect-deps/nonreactive-dep`. + * + * We know that local `useRef` return values are stable, regardless of + * inferred memoization. + */ +function NonReactiveRefInEffect() { + const ref = useRef('initial value'); + useEffect(() => print(ref.current)); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import { useEffect, useRef } from "react"; +import { print } from "shared-runtime"; + +/** + * Special case of `infer-effect-deps/nonreactive-dep`. + * + * We know that local `useRef` return values are stable, regardless of + * inferred memoization. + */ +function NonReactiveRefInEffect() { + const $ = _c(1); + const ref = useRef("initial value"); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = () => print(ref.current); + $[0] = t0; + } else { + t0 = $[0]; + } + useEffect(t0, [ref]); +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/nonreactive-ref.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/nonreactive-ref.js new file mode 100644 index 0000000000..8a8ab2f636 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/nonreactive-ref.js @@ -0,0 +1,14 @@ +// @inferEffectDependencies +import {useEffect, useRef} from 'react'; +import {print} from 'shared-runtime'; + +/** + * Special case of `infer-effect-deps/nonreactive-dep`. + * + * We know that local `useRef` return values are stable, regardless of + * inferred memoization. + */ +function NonReactiveRefInEffect() { + const ref = useRef('initial value'); + useEffect(() => print(ref.current)); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/outlined-function.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/outlined-function.expect.md new file mode 100644 index 0000000000..8272d4793f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/outlined-function.expect.md @@ -0,0 +1,46 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; +/** + * This compiled output is technically incorrect but this is currently the same + * case as a bailout (an effect that overfires). + * + * To ensure an empty deps array is passed, we need special case + * `InferEffectDependencies` for outlined functions (likely easier) or run it + * before OutlineFunctions + */ +function OutlinedFunctionInEffect() { + useEffect(() => print('hello world!')); +} + +``` + +## Code + +```javascript +// @inferEffectDependencies +import { useEffect } from "react"; +import { print } from "shared-runtime"; +/** + * This compiled output is technically incorrect but this is currently the same + * case as a bailout (an effect that overfires). + * + * To ensure an empty deps array is passed, we need special case + * `InferEffectDependencies` for outlined functions (likely easier) or run it + * before OutlineFunctions + */ +function OutlinedFunctionInEffect() { + useEffect(_temp); +} +function _temp() { + return print("hello world!"); +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/outlined-function.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/outlined-function.js new file mode 100644 index 0000000000..2ee2c94e3c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/outlined-function.js @@ -0,0 +1,14 @@ +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; +/** + * This compiled output is technically incorrect but this is currently the same + * case as a bailout (an effect that overfires). + * + * To ensure an empty deps array is passed, we need special case + * `InferEffectDependencies` for outlined functions (likely easier) or run it + * before OutlineFunctions + */ +function OutlinedFunctionInEffect() { + useEffect(() => print('hello world!')); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/pruned-nonreactive-obj.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/pruned-nonreactive-obj.expect.md new file mode 100644 index 0000000000..fbb88fb328 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/pruned-nonreactive-obj.expect.md @@ -0,0 +1,119 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useIdentity, mutate, makeObject} from 'shared-runtime'; +import {useEffect} from 'react'; + +/** + * When a semantically non-reactive value has a pruned scope (i.e. the object + * identity becomes reactive, but the underlying value it represents should be + * constant), the compiler can choose to either + * - add it as a dependency (and rerun the effect) + * - not add it as a dependency + * + * We keep semantically non-reactive values in both memo block and effect + * dependency arrays to avoid versioning invariants e.g. `x !== y.aliasedX`. + * ```js + * function Component() { + * // obj is semantically non-reactive, but its memo scope is pruned due to + * // the interleaving hook call + * const obj = {}; + * useHook(); + * write(obj); + * + * const ref = useRef(); + * + * // this effect needs to be rerun when obj's referential identity changes, + * // because it might alias obj to a useRef / mutable store. + * useEffect(() => ref.current = obj, ???); + * + * // in a custom hook (or child component), the user might expect versioning + * // invariants to hold + * useHook(ref, obj); + * } + * + * // defined elsewhere + * function useHook(someRef, obj) { + * useEffect( + * () => assert(someRef.current === obj), + * [someRef, obj] + * ); + * } + * ``` + */ +function PrunedNonReactive() { + const obj = makeObject(); + useIdentity(null); + mutate(obj); + + useEffect(() => print(obj.value)); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import { useIdentity, mutate, makeObject } from "shared-runtime"; +import { useEffect } from "react"; + +/** + * When a semantically non-reactive value has a pruned scope (i.e. the object + * identity becomes reactive, but the underlying value it represents should be + * constant), the compiler can choose to either + * - add it as a dependency (and rerun the effect) + * - not add it as a dependency + * + * We keep semantically non-reactive values in both memo block and effect + * dependency arrays to avoid versioning invariants e.g. `x !== y.aliasedX`. + * ```js + * function Component() { + * // obj is semantically non-reactive, but its memo scope is pruned due to + * // the interleaving hook call + * const obj = {}; + * useHook(); + * write(obj); + * + * const ref = useRef(); + * + * // this effect needs to be rerun when obj's referential identity changes, + * // because it might alias obj to a useRef / mutable store. + * useEffect(() => ref.current = obj, ???); + * + * // in a custom hook (or child component), the user might expect versioning + * // invariants to hold + * useHook(ref, obj); + * } + * + * // defined elsewhere + * function useHook(someRef, obj) { + * useEffect( + * () => assert(someRef.current === obj), + * [someRef, obj] + * ); + * } + * ``` + */ +function PrunedNonReactive() { + const $ = _c(2); + const obj = makeObject(); + useIdentity(null); + mutate(obj); + let t0; + if ($[0] !== obj.value) { + t0 = () => print(obj.value); + $[0] = obj.value; + $[1] = t0; + } else { + t0 = $[1]; + } + useEffect(t0, [obj.value]); +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/pruned-nonreactive-obj.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/pruned-nonreactive-obj.js new file mode 100644 index 0000000000..692b97b514 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/pruned-nonreactive-obj.js @@ -0,0 +1,48 @@ +// @inferEffectDependencies +import {useIdentity, mutate, makeObject} from 'shared-runtime'; +import {useEffect} from 'react'; + +/** + * When a semantically non-reactive value has a pruned scope (i.e. the object + * identity becomes reactive, but the underlying value it represents should be + * constant), the compiler can choose to either + * - add it as a dependency (and rerun the effect) + * - not add it as a dependency + * + * We keep semantically non-reactive values in both memo block and effect + * dependency arrays to avoid versioning invariants e.g. `x !== y.aliasedX`. + * ```js + * function Component() { + * // obj is semantically non-reactive, but its memo scope is pruned due to + * // the interleaving hook call + * const obj = {}; + * useHook(); + * write(obj); + * + * const ref = useRef(); + * + * // this effect needs to be rerun when obj's referential identity changes, + * // because it might alias obj to a useRef / mutable store. + * useEffect(() => ref.current = obj, ???); + * + * // in a custom hook (or child component), the user might expect versioning + * // invariants to hold + * useHook(ref, obj); + * } + * + * // defined elsewhere + * function useHook(someRef, obj) { + * useEffect( + * () => assert(someRef.current === obj), + * [someRef, obj] + * ); + * } + * ``` + */ +function PrunedNonReactive() { + const obj = makeObject(); + useIdentity(null); + mutate(obj); + + useEffect(() => print(obj.value)); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-memberexpr-merge.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-memberexpr-merge.expect.md new file mode 100644 index 0000000000..82eb54a908 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-memberexpr-merge.expect.md @@ -0,0 +1,49 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; + +function ReactiveMemberExprMerge({propVal}) { + const obj = {a: {b: propVal}}; + useEffect(() => print(obj.a, obj.a.b)); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import { useEffect } from "react"; +import { print } from "shared-runtime"; + +function ReactiveMemberExprMerge(t0) { + const $ = _c(4); + const { propVal } = t0; + let t1; + if ($[0] !== propVal) { + t1 = { a: { b: propVal } }; + $[0] = propVal; + $[1] = t1; + } else { + t1 = $[1]; + } + const obj = t1; + let t2; + if ($[2] !== obj.a) { + t2 = () => print(obj.a, obj.a.b); + $[2] = obj.a; + $[3] = t2; + } else { + t2 = $[3]; + } + useEffect(t2, [obj.a]); +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-memberexpr-merge.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-memberexpr-merge.js new file mode 100644 index 0000000000..071f5abbf5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-memberexpr-merge.js @@ -0,0 +1,8 @@ +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; + +function ReactiveMemberExprMerge({propVal}) { + const obj = {a: {b: propVal}}; + useEffect(() => print(obj.a, obj.a.b)); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-memberexpr.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-memberexpr.expect.md new file mode 100644 index 0000000000..74c4b9eb1e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-memberexpr.expect.md @@ -0,0 +1,49 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; + +function ReactiveMemberExpr({propVal}) { + const obj = {a: {b: propVal}}; + useEffect(() => print(obj.a.b)); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import { useEffect } from "react"; +import { print } from "shared-runtime"; + +function ReactiveMemberExpr(t0) { + const $ = _c(4); + const { propVal } = t0; + let t1; + if ($[0] !== propVal) { + t1 = { a: { b: propVal } }; + $[0] = propVal; + $[1] = t1; + } else { + t1 = $[1]; + } + const obj = t1; + let t2; + if ($[2] !== obj.a.b) { + t2 = () => print(obj.a.b); + $[2] = obj.a.b; + $[3] = t2; + } else { + t2 = $[3]; + } + useEffect(t2, [obj.a.b]); +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-memberexpr.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-memberexpr.js new file mode 100644 index 0000000000..0eabc54657 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-memberexpr.js @@ -0,0 +1,8 @@ +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; + +function ReactiveMemberExpr({propVal}) { + const obj = {a: {b: propVal}}; + useEffect(() => print(obj.a.b)); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-optional-chain.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-optional-chain.expect.md new file mode 100644 index 0000000000..7c9f21b85c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-optional-chain.expect.md @@ -0,0 +1,60 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; + +// TODO: take optional chains as dependencies +function ReactiveMemberExpr({cond, propVal}) { + const obj = {a: cond ? {b: propVal} : null}; + useEffect(() => print(obj.a?.b)); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import { useEffect } from "react"; +import { print } from "shared-runtime"; + +// TODO: take optional chains as dependencies +function ReactiveMemberExpr(t0) { + const $ = _c(7); + const { cond, propVal } = t0; + let t1; + if ($[0] !== cond || $[1] !== propVal) { + t1 = cond ? { b: propVal } : null; + $[0] = cond; + $[1] = propVal; + $[2] = t1; + } else { + t1 = $[2]; + } + let t2; + if ($[3] !== t1) { + t2 = { a: t1 }; + $[3] = t1; + $[4] = t2; + } else { + t2 = $[4]; + } + const obj = t2; + let t3; + if ($[5] !== obj.a?.b) { + t3 = () => print(obj.a?.b); + $[5] = obj.a?.b; + $[6] = t3; + } else { + t3 = $[6]; + } + useEffect(t3, [obj.a]); +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-optional-chain.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-optional-chain.js new file mode 100644 index 0000000000..8a76784e24 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-optional-chain.js @@ -0,0 +1,9 @@ +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; + +// TODO: take optional chains as dependencies +function ReactiveMemberExpr({cond, propVal}) { + const obj = {a: cond ? {b: propVal} : null}; + useEffect(() => print(obj.a?.b)); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-variable.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-variable.expect.md new file mode 100644 index 0000000000..6443b90e4a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-variable.expect.md @@ -0,0 +1,49 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; + +function ReactiveVariable({propVal}) { + const arr = [propVal]; + useEffect(() => print(arr)); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import { useEffect } from "react"; +import { print } from "shared-runtime"; + +function ReactiveVariable(t0) { + const $ = _c(4); + const { propVal } = t0; + let t1; + if ($[0] !== propVal) { + t1 = [propVal]; + $[0] = propVal; + $[1] = t1; + } else { + t1 = $[1]; + } + const arr = t1; + let t2; + if ($[2] !== arr) { + t2 = () => print(arr); + $[2] = arr; + $[3] = t2; + } else { + t2 = $[3]; + } + useEffect(t2, [arr]); +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-variable.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-variable.js new file mode 100644 index 0000000000..ae3ee2c8e2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/reactive-variable.js @@ -0,0 +1,8 @@ +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; + +function ReactiveVariable({propVal}) { + const arr = [propVal]; + useEffect(() => print(arr)); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/todo-import-namespace-useEffect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/todo-import-namespace-useEffect.expect.md new file mode 100644 index 0000000000..6302067a5a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/todo-import-namespace-useEffect.expect.md @@ -0,0 +1,50 @@ + +## Input + +```javascript +// @inferEffectDependencies +import * as React from 'react'; + +/** + * TODO: recognize import namespace + */ +function NonReactiveDepInEffect() { + const obj = makeObject_Primitives(); + React.useEffect(() => print(obj)); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import * as React from "react"; + +/** + * TODO: recognize import namespace + */ +function NonReactiveDepInEffect() { + const $ = _c(2); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = makeObject_Primitives(); + $[0] = t0; + } else { + t0 = $[0]; + } + const obj = t0; + let t1; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + t1 = () => print(obj); + $[1] = t1; + } else { + t1 = $[1]; + } + React.useEffect(t1); +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/todo-import-namespace-useEffect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/todo-import-namespace-useEffect.js new file mode 100644 index 0000000000..4c9eec8986 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-deps/todo-import-namespace-useEffect.js @@ -0,0 +1,10 @@ +// @inferEffectDependencies +import * as React from 'react'; + +/** + * TODO: recognize import namespace + */ +function NonReactiveDepInEffect() { + const obj = makeObject_Primitives(); + React.useEffect(() => print(obj)); +}