From 2d0a5e399f195bfc98fc5e1efa37aab9fa53e097 Mon Sep 17 00:00:00 2001 From: mofeiZ <34200447+mofeiZ@users.noreply.github.com> Date: Fri, 25 Apr 2025 15:42:40 -0400 Subject: [PATCH 1/3] [compiler] Patch for reactive refs in inferred effect dependencies (#32991) Inferred effect dependencies and inlined jsx (both experimental features) rely on `InferReactivePlaces` to determine their dependencies. Since adding type inference for phi nodes (https://github.com/facebook/react/pull/30796), we have been incorrectly inferring stable-typed value blocks (e.g. `props.cond ? setState1 : setState2`) as non-reactive. This fix patches InferReactivePlaces instead of adding a new pass since we want non-reactivity propagated correctly --- .../src/HIR/HIR.ts | 34 ++++++ .../src/Inference/InferReactivePlaces.ts | 112 +++++++++++++++++- .../reactive-ref-ternary.expect.md | 69 +++++++++++ .../reactive-ref-ternary.js | 15 +++ .../compiler/inline-jsx-transform.expect.md | 9 +- 5 files changed, 234 insertions(+), 5 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-ref-ternary.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-ref-ternary.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts index 0dfa937f37f03..ea58b3c49425e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -1738,6 +1738,40 @@ export function isStableType(id: Identifier): boolean { ); } +export function isStableTypeContainer(id: Identifier): boolean { + const type_ = id.type; + if (type_.kind !== 'Object') { + return false; + } + return ( + isUseStateType(id) || // setState + type_.shapeId === 'BuiltInUseActionState' || // setActionState + isUseReducerType(id) || // dispatcher + type_.shapeId === 'BuiltInUseTransition' // startTransition + ); +} + +export function evaluatesToStableTypeOrContainer( + env: Environment, + {value}: Instruction, +): boolean { + if (value.kind === 'CallExpression' || value.kind === 'MethodCall') { + const callee = + value.kind === 'CallExpression' ? value.callee : value.property; + + const calleeHookKind = getHookKind(env, callee.identifier); + switch (calleeHookKind) { + case 'useState': + case 'useReducer': + case 'useActionState': + case 'useRef': + case 'useTransition': + return true; + } + } + return false; +} + export function isUseEffectHookType(id: Identifier): boolean { return ( id.type.kind === 'Function' && id.type.shapeId === 'BuiltInUseEffectHook' diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts index e2deab15dbf0d..b05b292124c72 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts @@ -9,14 +9,19 @@ import {CompilerError} from '..'; import { BlockId, Effect, + Environment, HIRFunction, Identifier, IdentifierId, + Instruction, Place, computePostDominatorTree, + evaluatesToStableTypeOrContainer, getHookKind, isStableType, + isStableTypeContainer, isUseOperator, + isUseRefType, } from '../HIR'; import {PostDominator} from '../HIR/Dominator'; import { @@ -31,6 +36,103 @@ import { import DisjointSet from '../Utils/DisjointSet'; import {assertExhaustive} from '../Utils/utils'; +/** + * Side map to track and propagate sources of stability (i.e. hook calls such as + * `useRef()` and property reads such as `useState()[1]). Note that this + * requires forward data flow analysis since stability is not part of React + * Compiler's type system. + */ +class StableSidemap { + map: Map = new Map(); + env: Environment; + + constructor(env: Environment) { + this.env = env; + } + + handleInstruction(instr: Instruction): void { + const {value, lvalue} = instr; + + switch (value.kind) { + case 'CallExpression': + case 'MethodCall': { + /** + * Sources of stability are known hook calls + */ + if (evaluatesToStableTypeOrContainer(this.env, instr)) { + if (isStableType(lvalue.identifier)) { + this.map.set(lvalue.identifier.id, { + isStable: true, + }); + } else { + this.map.set(lvalue.identifier.id, { + isStable: false, + }); + } + } else if ( + this.env.config.enableTreatRefLikeIdentifiersAsRefs && + isUseRefType(lvalue.identifier) + ) { + this.map.set(lvalue.identifier.id, { + isStable: true, + }); + } + break; + } + + case 'Destructure': + case 'PropertyLoad': { + /** + * PropertyLoads may from stable containers may also produce stable + * values. ComputedLoads are technically safe for now (as all stable + * containers have differently-typed elements), but are not handled as + * they should be rare anyways. + */ + const source = + value.kind === 'Destructure' + ? value.value.identifier.id + : value.object.identifier.id; + const entry = this.map.get(source); + if (entry) { + for (const lvalue of eachInstructionLValue(instr)) { + if (isStableTypeContainer(lvalue.identifier)) { + this.map.set(lvalue.identifier.id, { + isStable: false, + }); + } else if (isStableType(lvalue.identifier)) { + this.map.set(lvalue.identifier.id, { + isStable: true, + }); + } + } + } + break; + } + + case 'StoreLocal': { + const entry = this.map.get(value.value.identifier.id); + if (entry) { + this.map.set(lvalue.identifier.id, entry); + this.map.set(value.lvalue.place.identifier.id, entry); + } + break; + } + + case 'LoadLocal': { + const entry = this.map.get(value.place.identifier.id); + if (entry) { + this.map.set(lvalue.identifier.id, entry); + } + break; + } + } + } + + isStable(id: IdentifierId): boolean { + const entry = this.map.get(id); + return entry != null ? entry.isStable : false; + } +} /* * Infers which `Place`s are reactive, ie may *semantically* change * over the course of the component/hook's lifetime. Places are reactive @@ -111,6 +213,7 @@ import {assertExhaustive} from '../Utils/utils'; */ export function inferReactivePlaces(fn: HIRFunction): void { const reactiveIdentifiers = new ReactivityMap(findDisjointMutableValues(fn)); + const stableIdentifierSources = new StableSidemap(fn.env); for (const param of fn.params) { const place = param.kind === 'Identifier' ? param : param.place; reactiveIdentifiers.markReactive(place); @@ -184,6 +287,7 @@ export function inferReactivePlaces(fn: HIRFunction): void { } } for (const instruction of block.instructions) { + stableIdentifierSources.handleInstruction(instruction); const {value} = instruction; let hasReactiveInput = false; /* @@ -218,7 +322,13 @@ export function inferReactivePlaces(fn: HIRFunction): void { if (hasReactiveInput) { for (const lvalue of eachInstructionLValue(instruction)) { - if (isStableType(lvalue.identifier)) { + /** + * Note that it's not correct to mark all stable-typed identifiers + * as non-reactive, since ternaries and other value blocks can + * produce reactive identifiers typed as these. + * (e.g. `props.cond ? setState1 : setState2`) + */ + if (stableIdentifierSources.isStable(lvalue.identifier.id)) { continue; } reactiveIdentifiers.markReactive(lvalue); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-ref-ternary.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-ref-ternary.expect.md new file mode 100644 index 0000000000000..def392cd8427b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-ref-ternary.expect.md @@ -0,0 +1,69 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useRef, useEffect} from 'react'; +import {print, mutate} from 'shared-runtime'; + +function Component({cond}) { + const arr = useRef([]); + const other = useRef([]); + // Although arr and other are both stable, derived is not + const derived = cond ? arr : other; + useEffect(() => { + mutate(derived.current); + print(derived.current); + }); + return arr; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import { useRef, useEffect } from "react"; +import { print, mutate } from "shared-runtime"; + +function Component(t0) { + const $ = _c(4); + const { cond } = t0; + let t1; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t1 = []; + $[0] = t1; + } else { + t1 = $[0]; + } + const arr = useRef(t1); + let t2; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + t2 = []; + $[1] = t2; + } else { + t2 = $[1]; + } + const other = useRef(t2); + + const derived = cond ? arr : other; + let t3; + if ($[2] !== derived) { + t3 = () => { + mutate(derived.current); + print(derived.current); + }; + $[2] = derived; + $[3] = t3; + } else { + t3 = $[3]; + } + useEffect(t3, [derived]); + return 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-dependencies/reactive-ref-ternary.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-ref-ternary.js new file mode 100644 index 0000000000000..93e5968a591ab --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-ref-ternary.js @@ -0,0 +1,15 @@ +// @inferEffectDependencies +import {useRef, useEffect} from 'react'; +import {print, mutate} from 'shared-runtime'; + +function Component({cond}) { + const arr = useRef([]); + const other = useRef([]); + // Although arr and other are both stable, derived is not + const derived = cond ? arr : other; + useEffect(() => { + mutate(derived.current); + print(derived.current); + }); + return arr; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-jsx-transform.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-jsx-transform.expect.md index 01b1470f93c8b..8101ddb072378 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-jsx-transform.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-jsx-transform.expect.md @@ -83,10 +83,10 @@ export const FIXTURE_ENTRYPOINT = { import { c as _c2 } from "react/compiler-runtime"; // @inlineJsxTransform function Parent(t0) { - const $ = _c2(2); + const $ = _c2(3); const { children, ref } = t0; let t1; - if ($[0] !== children) { + if ($[0] !== children || $[1] !== ref) { if (DEV) { t1 =
{children}
; } else { @@ -99,9 +99,10 @@ function Parent(t0) { }; } $[0] = children; - $[1] = t1; + $[1] = ref; + $[2] = t1; } else { - t1 = $[1]; + t1 = $[2]; } return t1; } From 89e8875ec48c86b366bf62398112923cdf76016a Mon Sep 17 00:00:00 2001 From: mofeiZ <34200447+mofeiZ@users.noreply.github.com> Date: Fri, 25 Apr 2025 15:44:39 -0400 Subject: [PATCH 2/3] [compiler] Fallback for inferred effect dependencies (#32984) When effect dependencies cannot be inferred due to memoization-related bailouts or unexpected mutable ranges (which currently often have to do with writes to refs), fall back to traversing the effect lambda itself. This fallback uses the same logic as PropagateScopeDependencies: 1. Collect a sidemap of loads and property loads 2. Find hoistable accesses from the control flow graph. Note that here, we currently take into account the mutable ranges of instructions (see `mutate-after-useeffect-granular-access` fixture) 3. Collect the set of property paths accessed by the effect 4. Merge to get the set of minimal dependencies --- .../src/HIR/CollectHoistablePropertyLoads.ts | 29 +++ .../src/HIR/PropagateScopeDependenciesHIR.ts | 11 +- .../src/Inference/InferEffectDependencies.ts | 192 +++++++++++++++--- ...-after-useeffect-granular-access.expect.md | 39 ++++ .../mutate-after-useeffect-granular-access.js | 12 ++ ...utate-after-useeffect-ref-access.expect.md | 38 ++++ .../mutate-after-useeffect-ref-access.js | 11 + .../mutate-after-useeffect.expect.md | 34 ++++ .../bailout-retry/mutate-after-useeffect.js | 9 + .../error.todo-infer-deps-on-retry.expect.md | 42 ---- .../infer-deps-on-retry.expect.md | 54 +++++ ...eps-on-retry.js => infer-deps-on-retry.js} | 0 12 files changed, 393 insertions(+), 78 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-granular-access.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-granular-access.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-ref-access.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-ref-access.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect.js delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-infer-deps-on-retry.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/infer-deps-on-retry.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/{error.todo-infer-deps-on-retry.js => infer-deps-on-retry.js} (100%) diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts index e29ef51ce0806..ea7268c573379 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -19,6 +19,7 @@ import { BasicBlock, BlockId, DependencyPathEntry, + FunctionExpression, GeneratedSource, getHookKind, HIRFunction, @@ -30,6 +31,7 @@ import { PropertyLiteral, ReactiveScopeDependency, ScopeId, + TInstruction, } from './HIR'; const DEBUG_PRINT = false; @@ -127,6 +129,33 @@ export function collectHoistablePropertyLoads( }); } +export function collectHoistablePropertyLoadsInInnerFn( + fnInstr: TInstruction, + temporaries: ReadonlyMap, + hoistableFromOptionals: ReadonlyMap, +): ReadonlyMap { + const fn = fnInstr.value.loweredFunc.func; + const initialContext: CollectHoistablePropertyLoadsContext = { + temporaries, + knownImmutableIdentifiers: new Set(), + hoistableFromOptionals, + registry: new PropertyPathRegistry(), + nestedFnImmutableContext: null, + assumedInvokedFns: fn.env.config.enableTreatFunctionDepsAsConditional + ? new Set() + : getAssumedInvokedFunctions(fn), + }; + const nestedFnImmutableContext = new Set( + fn.context + .filter(place => + isImmutableAtInstr(place.identifier, fnInstr.id, initialContext), + ) + .map(place => place.identifier.id), + ); + initialContext.nestedFnImmutableContext = nestedFnImmutableContext; + return collectHoistablePropertyLoadsImpl(fn, initialContext); +} + type CollectHoistablePropertyLoadsContext = { temporaries: ReadonlyMap; knownImmutableIdentifiers: ReadonlySet; diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts index 934fd98f73daf..3d183e8e72c68 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts @@ -116,7 +116,7 @@ export function propagateScopeDependenciesHIR(fn: HIRFunction): void { } } -function findTemporariesUsedOutsideDeclaringScope( +export function findTemporariesUsedOutsideDeclaringScope( fn: HIRFunction, ): ReadonlySet { /* @@ -378,7 +378,7 @@ type Decl = { scope: Stack; }; -class Context { +export class DependencyCollectionContext { #declarations: Map = new Map(); #reassignments: Map = new Map(); @@ -645,7 +645,10 @@ enum HIRValue { Terminal, } -function handleInstruction(instr: Instruction, context: Context): void { +export function handleInstruction( + instr: Instruction, + context: DependencyCollectionContext, +): void { const {id, value, lvalue} = instr; context.declare(lvalue.identifier, { id, @@ -708,7 +711,7 @@ function collectDependencies( temporaries: ReadonlyMap, processedInstrsInOptional: ReadonlySet, ): Map> { - const context = new Context( + const context = new DependencyCollectionContext( usedOutsideDeclaringScope, temporaries, processedInstrsInOptional, 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 a70f49dacd13a..f1a584341912b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts @@ -22,18 +22,30 @@ import { ScopeId, ReactiveScopeDependency, Place, + ReactiveScope, ReactiveScopeDependencies, + Terminal, isUseRefType, isSetStateType, isFireFunctionType, + makeScopeId, } from '../HIR'; +import {collectHoistablePropertyLoadsInInnerFn} from '../HIR/CollectHoistablePropertyLoads'; +import {collectOptionalChainSidemap} from '../HIR/CollectOptionalChainDependencies'; +import {ReactiveScopeDependencyTreeHIR} from '../HIR/DeriveMinimalDependenciesHIR'; import {DEFAULT_EXPORT} from '../HIR/Environment'; import { createTemporaryPlace, fixScopeAndIdentifierRanges, markInstructionIds, } from '../HIR/HIRBuilder'; +import { + collectTemporariesSidemap, + DependencyCollectionContext, + handleInstruction, +} from '../HIR/PropagateScopeDependenciesHIR'; import {eachInstructionOperand, eachTerminalOperand} from '../HIR/visitors'; +import {empty} from '../Utils/Stack'; import {getOrInsertWith} from '../Utils/utils'; /** @@ -62,10 +74,7 @@ export function inferEffectDependencies(fn: HIRFunction): void { const autodepFnLoads = new Map(); const autodepModuleLoads = new Map>(); - const scopeInfos = new Map< - ScopeId, - {pruned: boolean; deps: ReactiveScopeDependencies; hasSingleInstr: boolean} - >(); + const scopeInfos = new Map(); const loadGlobals = new Set(); @@ -79,19 +88,18 @@ export function inferEffectDependencies(fn: HIRFunction): void { const reactiveIds = inferReactiveIdentifiers(fn); for (const [, block] of fn.body.blocks) { - if ( - block.terminal.kind === 'scope' || - block.terminal.kind === 'pruned-scope' - ) { + if (block.terminal.kind === '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, - }); + if ( + scopeBlock.instructions.length === 1 && + scopeBlock.terminal.kind === 'goto' && + scopeBlock.terminal.block === block.terminal.fallthrough + ) { + scopeInfos.set( + block.terminal.scope.id, + block.terminal.scope.dependencies, + ); + } } const rewriteInstrs = new Map>(); for (const instr of block.instructions) { @@ -173,22 +181,12 @@ export function inferEffectDependencies(fn: HIRFunction): void { 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, - }); + let minimalDeps: Set; + if (scopeInfo != null) { + minimalDeps = new Set(scopeInfo); + } else { + minimalDeps = inferMinimalDependencies(fnExpr); } - /** * Step 1: push dependencies to the effect deps array * @@ -196,8 +194,9 @@ export function inferEffectDependencies(fn: HIRFunction): void { * the `infer-effect-deps/pruned-nonreactive-obj` fixture for an * explanation. */ + const usedDeps = []; - for (const dep of scopeInfo.deps) { + for (const dep of minimalDeps) { if ( ((isUseRefType(dep.identifier) || isSetStateType(dep.identifier)) && @@ -422,3 +421,132 @@ function collectDepUsages( return sourceLocations; } + +function inferMinimalDependencies( + fnInstr: TInstruction, +): Set { + const fn = fnInstr.value.loweredFunc.func; + + const temporaries = collectTemporariesSidemap(fn, new Set()); + const { + hoistableObjects, + processedInstrsInOptional, + temporariesReadInOptional, + } = collectOptionalChainSidemap(fn); + + const hoistablePropertyLoads = collectHoistablePropertyLoadsInInnerFn( + fnInstr, + temporaries, + hoistableObjects, + ); + const hoistableToFnEntry = hoistablePropertyLoads.get(fn.body.entry); + CompilerError.invariant(hoistableToFnEntry != null, { + reason: + '[InferEffectDependencies] Internal invariant broken: missing entry block', + loc: fnInstr.loc, + }); + + const dependencies = inferDependencies( + fnInstr, + new Map([...temporaries, ...temporariesReadInOptional]), + processedInstrsInOptional, + ); + + const tree = new ReactiveScopeDependencyTreeHIR( + [...hoistableToFnEntry.assumedNonNullObjects].map(o => o.fullPath), + ); + for (const dep of dependencies) { + tree.addDependency({...dep}); + } + + return tree.deriveMinimalDependencies(); +} + +function inferDependencies( + fnInstr: TInstruction, + temporaries: ReadonlyMap, + processedInstrsInOptional: ReadonlySet, +): Set { + const fn = fnInstr.value.loweredFunc.func; + const context = new DependencyCollectionContext( + new Set(), + temporaries, + processedInstrsInOptional, + ); + for (const dep of fn.context) { + context.declare(dep.identifier, { + id: makeInstructionId(0), + scope: empty(), + }); + } + const placeholderScope: ReactiveScope = { + id: makeScopeId(0), + range: { + start: fnInstr.id, + end: makeInstructionId(fnInstr.id + 1), + }, + dependencies: new Set(), + reassignments: new Set(), + declarations: new Map(), + earlyReturnValue: null, + merged: new Set(), + loc: GeneratedSource, + }; + context.enterScope(placeholderScope); + inferDependenciesInFn(fn, context, temporaries); + context.exitScope(placeholderScope, false); + const resultUnfiltered = context.deps.get(placeholderScope); + CompilerError.invariant(resultUnfiltered != null, { + reason: + '[InferEffectDependencies] Internal invariant broken: missing scope dependencies', + loc: fn.loc, + }); + + const fnContext = new Set(fn.context.map(dep => dep.identifier.id)); + const result = new Set(); + for (const dep of resultUnfiltered) { + if (fnContext.has(dep.identifier.id)) { + result.add(dep); + } + } + + return result; +} + +function inferDependenciesInFn( + fn: HIRFunction, + context: DependencyCollectionContext, + temporaries: ReadonlyMap, +): void { + for (const [, block] of fn.body.blocks) { + // Record referenced optional chains in phis + for (const phi of block.phis) { + for (const operand of phi.operands) { + const maybeOptionalChain = temporaries.get(operand[1].identifier.id); + if (maybeOptionalChain) { + context.visitDependency(maybeOptionalChain); + } + } + } + for (const instr of block.instructions) { + if ( + instr.value.kind === 'FunctionExpression' || + instr.value.kind === 'ObjectMethod' + ) { + context.declare(instr.lvalue.identifier, { + id: instr.id, + scope: context.currentScope, + }); + /** + * Recursively visit the inner function to extract dependencies + */ + const innerFn = instr.value.loweredFunc.func; + context.enterInnerFn(instr as TInstruction, () => { + inferDependenciesInFn(innerFn, context, temporaries); + }); + } else { + handleInstruction(instr, context); + } + } + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-granular-access.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-granular-access.expect.md new file mode 100644 index 0000000000000..7099388378b32 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-granular-access.expect.md @@ -0,0 +1,39 @@ + +## Input + +```javascript +// @inferEffectDependencies @panicThreshold(none) +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; + +function Component({foo}) { + const arr = []; + // Taking either arr[0].value or arr as a dependency is reasonable + // as long as developers know what to expect. + useEffect(() => print(arr[0].value)); + arr.push({value: foo}); + return arr; +} + +``` + +## Code + +```javascript +// @inferEffectDependencies @panicThreshold(none) +import { useEffect } from "react"; +import { print } from "shared-runtime"; + +function Component(t0) { + const { foo } = t0; + const arr = []; + + useEffect(() => print(arr[0].value), [arr[0].value]); + arr.push({ value: foo }); + return 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-dependencies/bailout-retry/mutate-after-useeffect-granular-access.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-granular-access.js new file mode 100644 index 0000000000000..fe00af39227e9 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-granular-access.js @@ -0,0 +1,12 @@ +// @inferEffectDependencies @panicThreshold(none) +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; + +function Component({foo}) { + const arr = []; + // Taking either arr[0].value or arr as a dependency is reasonable + // as long as developers know what to expect. + useEffect(() => print(arr[0].value)); + arr.push({value: foo}); + return arr; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-ref-access.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-ref-access.expect.md new file mode 100644 index 0000000000000..d0532a495a941 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-ref-access.expect.md @@ -0,0 +1,38 @@ + +## Input + +```javascript +// @inferEffectDependencies @panicThreshold(none) + +import {useEffect, useRef} from 'react'; +import {print} from 'shared-runtime'; + +function Component({arrRef}) { + // Avoid taking arr.current as a dependency + useEffect(() => print(arrRef.current)); + arrRef.current.val = 2; + return arrRef; +} + +``` + +## Code + +```javascript +// @inferEffectDependencies @panicThreshold(none) + +import { useEffect, useRef } from "react"; +import { print } from "shared-runtime"; + +function Component(t0) { + const { arrRef } = t0; + + useEffect(() => print(arrRef.current), [arrRef]); + arrRef.current.val = 2; + return arrRef; +} + +``` + +### 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-dependencies/bailout-retry/mutate-after-useeffect-ref-access.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-ref-access.js new file mode 100644 index 0000000000000..ff2cda6b898de --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-ref-access.js @@ -0,0 +1,11 @@ +// @inferEffectDependencies @panicThreshold(none) + +import {useEffect, useRef} from 'react'; +import {print} from 'shared-runtime'; + +function Component({arrRef}) { + // Avoid taking arr.current as a dependency + useEffect(() => print(arrRef.current)); + arrRef.current.val = 2; + return arrRef; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect.expect.md new file mode 100644 index 0000000000000..5da3ceca2225b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect.expect.md @@ -0,0 +1,34 @@ + +## Input + +```javascript +// @inferEffectDependencies @panicThreshold(none) +import {useEffect} from 'react'; + +function Component({foo}) { + const arr = []; + useEffect(() => arr.push(foo)); + arr.push(2); + return arr; +} + +``` + +## Code + +```javascript +// @inferEffectDependencies @panicThreshold(none) +import { useEffect } from "react"; + +function Component(t0) { + const { foo } = t0; + const arr = []; + useEffect(() => arr.push(foo), [arr, foo]); + arr.push(2); + return 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-dependencies/bailout-retry/mutate-after-useeffect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect.js new file mode 100644 index 0000000000000..806ca5322f071 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect.js @@ -0,0 +1,9 @@ +// @inferEffectDependencies @panicThreshold(none) +import {useEffect} from 'react'; + +function Component({foo}) { + const arr = []; + useEffect(() => arr.push(foo)); + arr.push(2); + return arr; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-infer-deps-on-retry.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-infer-deps-on-retry.expect.md deleted file mode 100644 index 0384d3335ca57..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-infer-deps-on-retry.expect.md +++ /dev/null @@ -1,42 +0,0 @@ - -## Input - -```javascript -// @inferEffectDependencies @panicThreshold(none) -import {useRef} from 'react'; -import {useSpecialEffect} from 'shared-runtime'; - -/** - * The retry pipeline disables memoization features, which means we need to - * provide an alternate implementation of effect dependencies which does not - * rely on memoization. - */ -function useFoo({cond}) { - const ref = useRef(); - const derived = cond ? ref.current : makeObject(); - useSpecialEffect(() => { - log(derived); - }, [derived]); - return ref; -} - -``` - - -## Error - -``` - 11 | const ref = useRef(); - 12 | const derived = cond ? ref.current : makeObject(); -> 13 | useSpecialEffect(() => { - | ^^^^^^^^^^^^^^^^^^^^^^^^ -> 14 | log(derived); - | ^^^^^^^^^^^^^^^^^ -> 15 | }, [derived]); - | ^^^^^^^^^^^^^^^^ InvalidReact: [InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. This will break your build! To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics.. (Bailout reason: Invariant: Expected function expression scope to exist (13:15)) (13:15) - 16 | return ref; - 17 | } - 18 | -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/infer-deps-on-retry.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/infer-deps-on-retry.expect.md new file mode 100644 index 0000000000000..42c800f8e55c0 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/infer-deps-on-retry.expect.md @@ -0,0 +1,54 @@ + +## Input + +```javascript +// @inferEffectDependencies @panicThreshold(none) +import {useRef} from 'react'; +import {useSpecialEffect} from 'shared-runtime'; + +/** + * The retry pipeline disables memoization features, which means we need to + * provide an alternate implementation of effect dependencies which does not + * rely on memoization. + */ +function useFoo({cond}) { + const ref = useRef(); + const derived = cond ? ref.current : makeObject(); + useSpecialEffect(() => { + log(derived); + }, [derived]); + return ref; +} + +``` + +## Code + +```javascript +// @inferEffectDependencies @panicThreshold(none) +import { useRef } from "react"; +import { useSpecialEffect } from "shared-runtime"; + +/** + * The retry pipeline disables memoization features, which means we need to + * provide an alternate implementation of effect dependencies which does not + * rely on memoization. + */ +function useFoo(t0) { + const { cond } = t0; + const ref = useRef(); + const derived = cond ? ref.current : makeObject(); + useSpecialEffect( + () => { + log(derived); + }, + [derived], + [derived], + ); + return 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/transform-fire/bailout-retry/error.todo-infer-deps-on-retry.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/infer-deps-on-retry.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/error.todo-infer-deps-on-retry.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/bailout-retry/infer-deps-on-retry.js From 8e9a5fc6c1a6252ca1727ab8fe0d4ee13f2568ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Fri, 25 Apr 2025 16:10:53 -0400 Subject: [PATCH 3/3] [Fizz] Enable the progressiveChunkSize option (#33027) Since the very beginning we have had the `progressiveChunkSize` option but we never actually took advantage of it because we didn't count the bytes that we emitted. This starts counting the bytes by taking a pass over the added chunks each time a segment completes. That allows us to outline a Suspense boundary to stream in late even if it is already loaded by the time that back-pressure flow and in a `prerender`. Meaning it gets inserted with script. The effect can be seen in the fixture where if you have large HTML content that can block initial paint (thanks to [`rel="expect"`](https://github.com/facebook/react/pull/33016) but also nested Suspense boundaries). Before this fix, the paint would be blocked until the large content loaded. This lets us paint the fallback first in the case that the raw bytes of the content takes a while to download. You can set it to `Infinity` to opt-out. E.g. if you want to ensure there's never any scripts. It's always set to `Infinity` in `renderToHTML` and the legacy `renderToString`. One downside is that if we might choose to outline a boundary, we need to let its fallback complete. We don't currently discount the size of the fallback but really just consider them additive even though in theory the fallback itself could also add significant size or even more than the content. It should maybe really be considered the delta but that would require us to track the size of the fallback separately which is tricky. One problem with the current heuristic is that we just consider the size of the boundary content itself down to the next boundary. If you have a lot of small boundaries adding up, it'll never kick in. I intend to address that in a follow up. --- fixtures/ssr/src/components/Chrome.js | 8 +- fixtures/ssr/src/components/LargeContent.js | 243 ++++++++++++++++++ .../src/ReactNoopServer.js | 2 + packages/react-server/src/ReactFizzServer.js | 66 ++++- .../src/ReactServerStreamConfigFB.js | 6 +- 5 files changed, 309 insertions(+), 16 deletions(-) create mode 100644 fixtures/ssr/src/components/LargeContent.js diff --git a/fixtures/ssr/src/components/Chrome.js b/fixtures/ssr/src/components/Chrome.js index 984c726a02652..b2a4a96c43fb5 100644 --- a/fixtures/ssr/src/components/Chrome.js +++ b/fixtures/ssr/src/components/Chrome.js @@ -4,6 +4,8 @@ import Theme, {ThemeToggleButton} from './Theme'; import './Chrome.css'; +import LargeContent from './LargeContent'; + export default class Chrome extends Component { state = {theme: 'light'}; render() { @@ -25,7 +27,6 @@ export default class Chrome extends Component { /> - {this.props.children}
{ @@ -35,9 +36,14 @@ export default class Chrome extends Component { }} />
+ {this.props.children}

This should appear in the first paint.

+ +

This content should not block paint.

+ +