From 50a37693f1c463ec1234414ffffe747690323321 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Wed, 21 May 2025 14:10:04 -0400 Subject: [PATCH 1/2] [compiler] Add reactive flag on scope dependencies When collecting scope dependencies, mark each dependency with `reactive: true | false`. This prepares for later PRs https://github.com/facebook/react/pull/33326 and https://github.com/facebook/react/pull/32099 which rewrite scope dependencies into instructions. Note that some reactive objects may have non-reactive properties, but we do not currently track this. Technically, state[0] is reactive and state[1] is not. Currently, both would be marked as reactive. ```js const state = useState(); ``` --- .../src/HIR/CollectHoistablePropertyLoads.ts | 28 ++++++++++---- .../HIR/CollectOptionalChainDependencies.ts | 2 + .../src/HIR/DeriveMinimalDependenciesHIR.ts | 36 +++++++++++++----- .../src/HIR/HIR.ts | 12 ++++++ .../src/HIR/PropagateScopeDependenciesHIR.ts | 11 +++++- .../src/Inference/InferReactivePlaces.ts | 38 ++++++++++++++++++- ...rgeReactiveScopesThatInvalidateTogether.ts | 1 + 7 files changed, 109 insertions(+), 19 deletions(-) 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 ea7268c573..a11822538f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -241,7 +241,10 @@ type PropertyPathNode = class PropertyPathRegistry { roots: Map = new Map(); - getOrCreateIdentifier(identifier: Identifier): PropertyPathNode { + getOrCreateIdentifier( + identifier: Identifier, + reactive: boolean, + ): PropertyPathNode { /** * Reads from a statically scoped variable are always safe in JS, * with the exception of TDZ (not addressed by this pass). @@ -255,12 +258,19 @@ class PropertyPathRegistry { optionalProperties: new Map(), fullPath: { identifier, + reactive, path: [], }, hasOptional: false, parent: null, }; this.roots.set(identifier.id, rootNode); + } else { + CompilerError.invariant(reactive === rootNode.fullPath.reactive, { + reason: + '[HoistablePropertyLoads] Found inconsistencies in `reactive` flag when deduping identifier reads within the same scope', + loc: identifier.loc, + }); } return rootNode; } @@ -278,6 +288,7 @@ class PropertyPathRegistry { parent: parent, fullPath: { identifier: parent.fullPath.identifier, + reactive: parent.fullPath.reactive, path: parent.fullPath.path.concat(entry), }, hasOptional: parent.hasOptional || entry.optional, @@ -293,7 +304,7 @@ class PropertyPathRegistry { * so all subpaths of a PropertyLoad should already exist * (e.g. a.b is added before a.b.c), */ - let currNode = this.getOrCreateIdentifier(n.identifier); + let currNode = this.getOrCreateIdentifier(n.identifier, n.reactive); if (n.path.length === 0) { return currNode; } @@ -315,10 +326,11 @@ function getMaybeNonNullInInstruction( instr: InstructionValue, context: CollectHoistablePropertyLoadsContext, ): PropertyPathNode | null { - let path = null; + let path: ReactiveScopeDependency | null = null; if (instr.kind === 'PropertyLoad') { path = context.temporaries.get(instr.object.identifier.id) ?? { identifier: instr.object.identifier, + reactive: instr.object.reactive, path: [], }; } else if (instr.kind === 'Destructure') { @@ -381,7 +393,7 @@ function collectNonNullsInBlocks( ) { const identifier = fn.params[0].identifier; knownNonNullIdentifiers.add( - context.registry.getOrCreateIdentifier(identifier), + context.registry.getOrCreateIdentifier(identifier, true), ); } const nodes = new Map< @@ -616,9 +628,11 @@ function reduceMaybeOptionalChains( changed = false; for (const original of optionalChainNodes) { - let {identifier, path: origPath} = original.fullPath; - let currNode: PropertyPathNode = - registry.getOrCreateIdentifier(identifier); + let {identifier, path: origPath, reactive} = original.fullPath; + let currNode: PropertyPathNode = registry.getOrCreateIdentifier( + identifier, + reactive, + ); for (let i = 0; i < origPath.length; i++) { const entry = origPath[i]; // If the base is known to be non-null, replace with a non-optional load diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectOptionalChainDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectOptionalChainDependencies.ts index cb787d04d0..75dad4c1bf 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectOptionalChainDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectOptionalChainDependencies.ts @@ -290,6 +290,7 @@ function traverseOptionalBlock( ); baseObject = { identifier: maybeTest.instructions[0].value.place.identifier, + reactive: maybeTest.instructions[0].value.place.reactive, path, }; test = maybeTest.terminal; @@ -391,6 +392,7 @@ function traverseOptionalBlock( ); const load = { identifier: baseObject.identifier, + reactive: baseObject.reactive, path: [ ...baseObject.path, { diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts index 7f6fb9e88f..7819ab39b2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts @@ -25,8 +25,9 @@ export class ReactiveScopeDependencyTreeHIR { * `identifier.path`, or `identifier?.path` is in this map, it is safe to * evaluate (non-optional) PropertyLoads from. */ - #hoistableObjects: Map = new Map(); - #deps: Map = new Map(); + #hoistableObjects: Map = + new Map(); + #deps: Map = new Map(); /** * @param hoistableObjects a set of paths from which we can safely evaluate @@ -35,9 +36,10 @@ export class ReactiveScopeDependencyTreeHIR { * duplicates when traversing the CFG. */ constructor(hoistableObjects: Iterable) { - for (const {path, identifier} of hoistableObjects) { + for (const {path, identifier, reactive} of hoistableObjects) { let currNode = ReactiveScopeDependencyTreeHIR.#getOrCreateRoot( identifier, + reactive, this.#hoistableObjects, path.length > 0 && path[0].optional ? 'Optional' : 'NonNull', ); @@ -70,7 +72,8 @@ export class ReactiveScopeDependencyTreeHIR { static #getOrCreateRoot( identifier: Identifier, - roots: Map>, + reactive: boolean, + roots: Map & {reactive: boolean}>, defaultAccessType: T, ): TreeNode { // roots can always be accessed unconditionally in JS @@ -79,9 +82,16 @@ export class ReactiveScopeDependencyTreeHIR { if (rootNode === undefined) { rootNode = { properties: new Map(), + reactive, accessType: defaultAccessType, }; roots.set(identifier, rootNode); + } else { + CompilerError.invariant(reactive === rootNode.reactive, { + reason: '[DeriveMinimalDependenciesHIR] Conflicting reactive root flag', + description: `Identifier ${printIdentifier(identifier)}`, + loc: GeneratedSource, + }); } return rootNode; } @@ -92,9 +102,10 @@ export class ReactiveScopeDependencyTreeHIR { * safe-to-evaluate subpath */ addDependency(dep: ReactiveScopeDependency): void { - const {identifier, path} = dep; + const {identifier, reactive, path} = dep; let depCursor = ReactiveScopeDependencyTreeHIR.#getOrCreateRoot( identifier, + reactive, this.#deps, PropertyAccessType.UnconditionalAccess, ); @@ -172,7 +183,13 @@ export class ReactiveScopeDependencyTreeHIR { deriveMinimalDependencies(): Set { const results = new Set(); for (const [rootId, rootNode] of this.#deps.entries()) { - collectMinimalDependenciesInSubtree(rootNode, rootId, [], results); + collectMinimalDependenciesInSubtree( + rootNode, + rootNode.reactive, + rootId, + [], + results, + ); } return results; @@ -294,25 +311,24 @@ type HoistableNode = TreeNode<'Optional' | 'NonNull'>; type DependencyNode = TreeNode; /** - * TODO: this is directly pasted from DeriveMinimalDependencies. Since we no - * longer have conditionally accessed nodes, we can simplify - * * Recursively calculates minimal dependencies in a subtree. * @param node DependencyNode representing a dependency subtree. * @returns a minimal list of dependencies in this subtree. */ function collectMinimalDependenciesInSubtree( node: DependencyNode, + reactive: boolean, rootIdentifier: Identifier, path: Array, results: Set, ): void { if (isDependency(node.accessType)) { - results.add({identifier: rootIdentifier, path}); + results.add({identifier: rootIdentifier, reactive, path}); } else { for (const [childName, childNode] of node.properties) { collectMinimalDependenciesInSubtree( childNode, + reactive, rootIdentifier, [ ...path, 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 99b8c189ee..1699a0fc3d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -1568,6 +1568,18 @@ export type DependencyPathEntry = { export type DependencyPath = Array; export type ReactiveScopeDependency = { identifier: Identifier; + /** + * Reflects whether the base identifier is reactive. Note that some reactive + * objects may have non-reactive properties, but we do not currently track + * this. + * + * ```js + * // Technically, result[0] is reactive and result[1] is not. + * // Currently, both dependencies would be marked as reactive. + * const result = useState(); + * ``` + */ + reactive: boolean; path: DependencyPath; }; 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 96b9e51710..91b7712b88 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts @@ -316,6 +316,7 @@ function collectTemporariesSidemapImpl( ) { temporaries.set(lvalue.identifier.id, { identifier: value.place.identifier, + reactive: value.place.reactive, path: [], }); } @@ -369,11 +370,13 @@ function getProperty( if (resolvedDependency == null) { property = { identifier: object.identifier, + reactive: object.reactive, path: [{property: propertyName, optional}], }; } else { property = { identifier: resolvedDependency.identifier, + reactive: resolvedDependency.reactive, path: [...resolvedDependency.path, {property: propertyName, optional}], }; } @@ -532,6 +535,7 @@ export class DependencyCollectionContext { this.visitDependency( this.#temporaries.get(place.identifier.id) ?? { identifier: place.identifier, + reactive: place.reactive, path: [], }, ); @@ -596,6 +600,7 @@ export class DependencyCollectionContext { ) { maybeDependency = { identifier: maybeDependency.identifier, + reactive: maybeDependency.reactive, path: [], }; } @@ -617,7 +622,11 @@ export class DependencyCollectionContext { identifier => identifier.declarationId === place.identifier.declarationId, ) && - this.#checkValidDependency({identifier: place.identifier, path: []}) + this.#checkValidDependency({ + identifier: place.identifier, + reactive: place.reactive, + path: [], + }) ) { currentScope.reassignments.add(place.identifier); } 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 b05b292124..88faccd8cf 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts @@ -26,6 +26,7 @@ import { import {PostDominator} from '../HIR/Dominator'; import { eachInstructionLValue, + eachInstructionOperand, eachInstructionValueOperand, eachTerminalOperand, } from '../HIR/visitors'; @@ -292,7 +293,7 @@ export function inferReactivePlaces(fn: HIRFunction): void { let hasReactiveInput = false; /* * NOTE: we want to mark all operands as reactive or not, so we - * avoid short-circuting here + * avoid short-circuiting here */ for (const operand of eachInstructionValueOperand(value)) { const reactive = reactiveIdentifiers.isReactive(operand); @@ -375,6 +376,41 @@ export function inferReactivePlaces(fn: HIRFunction): void { } } } while (reactiveIdentifiers.snapshot()); + + function propagateReactivityToInnerFunctions( + fn: HIRFunction, + isOutermost: boolean, + ): void { + for (const [, block] of fn.body.blocks) { + for (const instr of block.instructions) { + if (!isOutermost) { + for (const operand of eachInstructionOperand(instr)) { + reactiveIdentifiers.isReactive(operand); + } + } + if ( + instr.value.kind === 'ObjectMethod' || + instr.value.kind === 'FunctionExpression' + ) { + propagateReactivityToInnerFunctions( + instr.value.loweredFunc.func, + false, + ); + } + } + if (!isOutermost) { + for (const operand of eachTerminalOperand(block.terminal)) { + reactiveIdentifiers.isReactive(operand); + } + } + } + } + + /** + * Propagate reactivity for inner functions, as we eventually hoist and dedupe + * dependency instructions for scopes. + */ + propagateReactivityToInnerFunctions(fn, true); } /* diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts index 08d2212d86..6f2d97ff8e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts @@ -456,6 +456,7 @@ function canMergeScopes( new Set( [...current.scope.declarations.values()].map(declaration => ({ identifier: declaration.identifier, + reactive: true, path: [], })), ), From 53cc92d2d21bebc261784306c04110a7a9734385 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Wed, 21 May 2025 14:46:06 -0400 Subject: [PATCH 2/2] [compiler] Inferred effect dependencies now include optional chains Inferred effect dependencies now include optional chains. This is a temporary solution while https://github.com/facebook/react/pull/32099 and its followups are worked on. Ideally, we should model reactive scope dependencies in the IR similarly to `ComputeIR` -- dependencies should be hoisted and all references rewritten to use the hoisted dependencies. ` --- .../src/HIR/ScopeDependencyUtils.ts | 280 ++++++++++++++++++ .../src/Inference/InferEffectDependencies.ts | 278 +++++++++++------ ...e-after-useeffect-optional-chain.expect.md | 58 ++++ .../mutate-after-useeffect-optional-chain.js | 17 ++ ...utate-after-useeffect-ref-access.expect.md | 25 +- .../mutate-after-useeffect-ref-access.js | 7 +- .../mutate-after-useeffect.expect.md | 32 +- .../bailout-retry/mutate-after-useeffect.js | 11 +- .../reactive-optional-chain-complex.expect.md | 101 +++++++ .../reactive-optional-chain-complex.js | 18 ++ .../reactive-optional-chain.expect.md | 31 +- .../reactive-optional-chain.js | 8 +- 12 files changed, 754 insertions(+), 112 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/HIR/ScopeDependencyUtils.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-optional-chain.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-optional-chain.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain-complex.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain-complex.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/ScopeDependencyUtils.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/ScopeDependencyUtils.ts new file mode 100644 index 0000000000..b986cd211b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/ScopeDependencyUtils.ts @@ -0,0 +1,280 @@ +import { + Place, + ReactiveScopeDependency, + Identifier, + makeInstructionId, + InstructionKind, + GeneratedSource, + BlockId, + makeTemporaryIdentifier, + Effect, + GotoVariant, + HIR, +} from './HIR'; +import {CompilerError} from '../CompilerError'; +import {Environment} from './Environment'; +import HIRBuilder from './HIRBuilder'; +import {lowerValueToTemporary} from './BuildHIR'; +import {printDependency} from '../ReactiveScopes/PrintReactiveFunction'; + +type DependencyInstructions = { + place: Place; + value: HIR; + exitBlockId: BlockId; +}; + +export function buildDependencyInstructions( + dep: ReactiveScopeDependency, + env: Environment, +): DependencyInstructions { + const builder = new HIRBuilder(env, { + entryBlockKind: 'value', + }); + let result: Place; + if (dep.path.every(path => !path.optional)) { + const last = writeNonOptionalDependency(dep, env, builder); + result = { + kind: 'Identifier', + identifier: last, + effect: Effect.Freeze, + reactive: dep.reactive, + loc: GeneratedSource, + }; + } else { + const last = writeOptionalDependency( + dep.path.length - 1, + dep, + builder, + null, + ); + result = { + kind: 'Identifier', + identifier: last, + effect: Effect.Freeze, + reactive: dep.reactive, + loc: GeneratedSource, + }; + } + + const exitBlockId = builder.terminate( + { + kind: 'unsupported', + loc: GeneratedSource, + id: makeInstructionId(0), + }, + null, + ); + return { + place: result, + value: builder.build(), + exitBlockId, + }; +} +function writeNonOptionalDependency( + dep: ReactiveScopeDependency, + env: Environment, + builder: HIRBuilder, +): Identifier { + const loc = dep.identifier.loc; + let last: Identifier = makeTemporaryIdentifier(env.nextIdentifierId, loc); + builder.push({ + lvalue: { + identifier: last, + kind: 'Identifier', + effect: Effect.Mutate, + reactive: dep.reactive, + loc, + }, + value: { + kind: 'LoadLocal', + place: { + identifier: dep.identifier, + kind: 'Identifier', + effect: Effect.Freeze, + reactive: dep.reactive, + loc, + }, + loc, + }, + id: makeInstructionId(1), + loc: loc, + }); + + for (const path of dep.path) { + const next = makeTemporaryIdentifier(env.nextIdentifierId, loc); + builder.push({ + lvalue: { + identifier: next, + kind: 'Identifier', + effect: Effect.Mutate, + reactive: dep.reactive, + loc, + }, + value: { + kind: 'PropertyLoad', + object: { + identifier: last, + kind: 'Identifier', + effect: Effect.Freeze, + reactive: dep.reactive, + loc, + }, + property: path.property, + loc, + }, + id: makeInstructionId(1), + loc: loc, + }); + last = next; + } + return last; +} + +function writeOptionalDependency( + idx: number, + dep: ReactiveScopeDependency, + builder: HIRBuilder, + parentAlternate: BlockId | null, +): Identifier { + const env = builder.environment; + CompilerError.invariant( + idx >= 0 && !dep.path.slice(0, idx + 1).every(path => !path.optional), + { + reason: '[WriteOptional] Expected optional path', + description: `${idx} ${printDependency(dep)}`, + loc: GeneratedSource, + }, + ); + const continuationBlock = builder.reserve(builder.currentBlockKind()); + const consequent = builder.reserve('value'); + + const returnPlace: Place = { + kind: 'Identifier', + identifier: makeTemporaryIdentifier(env.nextIdentifierId, GeneratedSource), + effect: Effect.Mutate, + reactive: dep.reactive, + loc: GeneratedSource, + }; + + let alternate; + if (parentAlternate != null) { + alternate = parentAlternate; + } else { + /** + * Make outermost alternate block + * $N = Primitive undefined + * $M = StoreLocal $OptionalResult = $N + * goto fallthrough + */ + alternate = builder.enter('value', () => { + const temp = lowerValueToTemporary(builder, { + kind: 'Primitive', + value: undefined, + loc: GeneratedSource, + }); + lowerValueToTemporary(builder, { + kind: 'StoreLocal', + lvalue: {kind: InstructionKind.Const, place: {...returnPlace}}, + value: {...temp}, + type: null, + loc: GeneratedSource, + }); + return { + kind: 'goto', + variant: GotoVariant.Break, + block: continuationBlock.id, + id: makeInstructionId(0), + loc: GeneratedSource, + }; + }); + } + + let testIdentifier: Identifier | null = null; + const testBlock = builder.enter('value', () => { + const firstOptional = dep.path.findIndex(path => path.optional); + if (idx === firstOptional) { + // Lower test block + testIdentifier = writeNonOptionalDependency( + { + identifier: dep.identifier, + reactive: dep.reactive, + path: dep.path.slice(0, idx), + }, + env, + builder, + ); + } else { + testIdentifier = writeOptionalDependency( + idx - 1, + dep, + builder, + alternate, + ); + } + + return { + kind: 'branch', + test: { + identifier: testIdentifier, + effect: Effect.Freeze, + kind: 'Identifier', + loc: GeneratedSource, + reactive: dep.reactive, + }, + consequent: consequent.id, + alternate, + id: makeInstructionId(0), + loc: GeneratedSource, + fallthrough: continuationBlock.id, + }; + }); + + builder.enterReserved(consequent, () => { + CompilerError.invariant(testIdentifier !== null, { + reason: 'Satisfy type checker', + description: null, + loc: null, + suggestions: null, + }); + + const tmpConsequent = lowerValueToTemporary(builder, { + kind: 'PropertyLoad', + object: { + identifier: testIdentifier, + kind: 'Identifier', + effect: Effect.Freeze, + reactive: dep.reactive, + loc: GeneratedSource, + }, + property: dep.path[idx].property, + loc: GeneratedSource, + }); + lowerValueToTemporary(builder, { + kind: 'StoreLocal', + lvalue: {kind: InstructionKind.Const, place: {...returnPlace}}, + value: {...tmpConsequent}, + type: null, + loc: GeneratedSource, + }); + return { + kind: 'goto', + variant: GotoVariant.Break, + block: continuationBlock.id, + id: makeInstructionId(0), + loc: GeneratedSource, + }; + }); + builder.terminateWithContinuation( + { + kind: 'optional', + optional: dep.path[idx].optional, + test: testBlock, + fallthrough: continuationBlock.id, + id: makeInstructionId(0), + loc: GeneratedSource, + }, + continuationBlock, + ); + + return returnPlace.identifier; +} 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 f1a5843419..722f9f7587 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts @@ -10,7 +10,6 @@ import {CompilerError, SourceLocation} from '..'; import { ArrayExpression, Effect, - Environment, FunctionExpression, GeneratedSource, HIRFunction, @@ -29,6 +28,9 @@ import { isSetStateType, isFireFunctionType, makeScopeId, + HIR, + BasicBlock, + BlockId, } from '../HIR'; import {collectHoistablePropertyLoadsInInnerFn} from '../HIR/CollectHoistablePropertyLoads'; import {collectOptionalChainSidemap} from '../HIR/CollectOptionalChainDependencies'; @@ -44,7 +46,12 @@ import { DependencyCollectionContext, handleInstruction, } from '../HIR/PropagateScopeDependenciesHIR'; -import {eachInstructionOperand, eachTerminalOperand} from '../HIR/visitors'; +import {buildDependencyInstructions} from '../HIR/ScopeDependencyUtils'; +import { + eachInstructionOperand, + eachTerminalOperand, + terminalFallthrough, +} from '../HIR/visitors'; import {empty} from '../Utils/Stack'; import {getOrInsertWith} from '../Utils/utils'; @@ -53,7 +60,6 @@ import {getOrInsertWith} from '../Utils/utils'; * a second argument to the useEffect call if no dependency array is provided. */ export function inferEffectDependencies(fn: HIRFunction): void { - let hasRewrite = false; const fnExpressions = new Map< IdentifierId, TInstruction @@ -86,6 +92,7 @@ export function inferEffectDependencies(fn: HIRFunction): void { * reactive(Identifier i) = Union_{reference of i}(reactive(reference)) */ const reactiveIds = inferReactiveIdentifiers(fn); + const rewriteBlocks: Array = []; for (const [, block] of fn.body.blocks) { if (block.terminal.kind === 'scope') { @@ -101,7 +108,7 @@ export function inferEffectDependencies(fn: HIRFunction): void { ); } } - const rewriteInstrs = new Map>(); + const rewriteInstrs: Array = []; for (const instr of block.instructions) { const {value, lvalue} = instr; if (value.kind === 'FunctionExpression') { @@ -165,7 +172,6 @@ export function inferEffectDependencies(fn: HIRFunction): void { ) { // We have a useEffect call with no deps array, so we need to infer the deps const effectDeps: Array = []; - const newInstructions: Array = []; const deps: ArrayExpression = { kind: 'ArrayExpression', elements: effectDeps, @@ -196,24 +202,28 @@ export function inferEffectDependencies(fn: HIRFunction): void { */ const usedDeps = []; - for (const dep of minimalDeps) { + for (const maybeDep of minimalDeps) { if ( - ((isUseRefType(dep.identifier) || - isSetStateType(dep.identifier)) && - !reactiveIds.has(dep.identifier.id)) || - isFireFunctionType(dep.identifier) + ((isUseRefType(maybeDep.identifier) || + isSetStateType(maybeDep.identifier)) && + !reactiveIds.has(maybeDep.identifier.id)) || + isFireFunctionType(maybeDep.identifier) ) { // exclude non-reactive hook results, which will never be in a memo block continue; } - const {place, instructions} = writeDependencyToInstructions( + const dep = truncateDepAtCurrent(maybeDep); + const {place, value, exitBlockId} = buildDependencyInstructions( dep, - reactiveIds.has(dep.identifier.id), fn.env, - fnExpr.loc, ); - newInstructions.push(...instructions); + rewriteInstrs.push({ + kind: 'block', + location: instr.id, + value, + exitBlockId: exitBlockId, + }); effectDeps.push(place); usedDeps.push(dep); } @@ -234,27 +244,32 @@ export function inferEffectDependencies(fn: HIRFunction): void { }); } - newInstructions.push({ - id: makeInstructionId(0), - loc: GeneratedSource, - lvalue: {...depsPlace, effect: Effect.Mutate}, - value: deps, - }); - // Step 2: push the inferred deps array as an argument of the useEffect + rewriteInstrs.push({ + kind: 'instr', + location: instr.id, + value: { + id: makeInstructionId(0), + loc: GeneratedSource, + lvalue: {...depsPlace, effect: Effect.Mutate}, + value: deps, + }, + }); value.args.push({...depsPlace, effect: Effect.Freeze}); - rewriteInstrs.set(instr.id, newInstructions); fn.env.inferredEffectLocations.add(callee.loc); } else if (loadGlobals.has(value.args[0].identifier.id)) { // Global functions have no reactive dependencies, so we can insert an empty array - newInstructions.push({ - id: makeInstructionId(0), - loc: GeneratedSource, - lvalue: {...depsPlace, effect: Effect.Mutate}, - value: deps, + rewriteInstrs.push({ + kind: 'instr', + location: instr.id, + value: { + id: makeInstructionId(0), + loc: GeneratedSource, + lvalue: {...depsPlace, effect: Effect.Mutate}, + value: deps, + }, }); value.args.push({...depsPlace, effect: Effect.Freeze}); - rewriteInstrs.set(instr.id, newInstructions); fn.env.inferredEffectLocations.add(callee.loc); } } else if ( @@ -285,21 +300,13 @@ export function inferEffectDependencies(fn: HIRFunction): void { } } } - 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); - } else { - newInstrs.push(instr); - } - } - block.instructions = newInstrs; - } + rewriteSplices(block, rewriteInstrs, rewriteBlocks); } - if (hasRewrite) { + + if (rewriteBlocks.length > 0) { + for (const block of rewriteBlocks) { + fn.body.blocks.set(block.id, block); + } // Renumber instructions and fix scope ranges markInstructionIds(fn.body); fixScopeAndIdentifierRanges(fn.body); @@ -307,63 +314,144 @@ export function inferEffectDependencies(fn: HIRFunction): void { } } -function writeDependencyToInstructions( +function truncateDepAtCurrent( 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(s) - */ - break; +): ReactiveScopeDependency { + const idx = dep.path.findIndex(path => path.property === 'current'); + if (idx === -1) { + return dep; + } else { + return {...dep, path: dep.path.slice(0, idx)}; + } +} + +type SpliceInfo = + | {kind: 'instr'; location: InstructionId; value: Instruction} + | { + kind: 'block'; + location: InstructionId; + value: HIR; + exitBlockId: BlockId; + }; + +function rewriteSplices( + originalBlock: BasicBlock, + splices: Array, + rewriteBlocks: Array, +): void { + if (splices.length === 0) { + return; + } + /** + * Splice instructions or value blocks into the original block. + * --- original block --- + * bb_original + * instr1 + * ... + * instr2 <-- splice location + * instr3 + * ... + * + * + * If there is more than one block in the splice, this means that we're + * splicing in a set of value-blocks of the following structure: + * --- blocks we're splicing in --- + * bb_entry: + * instrEntry + * ... + * fallthrough=bb_exit + * + * bb1(value): + * ... + * + * bb_exit: + * instrExit + * ... + * + * + * + * --- rewritten blocks --- + * bb_original + * instr1 + * ... (original instructions) + * instr2 + * instrEntry + * ... (spliced instructions) + * fallthrough=bb_exit + * + * bb1(value): + * ... + * + * bb_exit: + * instrExit + * ... (spliced instructions) + * instr3 + * ... (original instructions) + * + */ + const originalInstrs = originalBlock.instructions; + let currBlock: BasicBlock = {...originalBlock, instructions: []}; + rewriteBlocks.push(currBlock); + + let cursor = 0; + for (const rewrite of splices) { + while (originalInstrs[cursor].id < rewrite.location) { + CompilerError.invariant( + originalInstrs[cursor].id < originalInstrs[cursor + 1].id, + { + reason: + '[InferEffectDependencies] Internal invariant broken: expected block instructions to be sorted', + loc: originalInstrs[cursor].loc, + }, + ); + currBlock.instructions.push(originalInstrs[cursor]); + cursor++; } - if (path.property === 'current') { - /* - * Prune ref.current accesses. This may over-capture for non-ref values with - * a current property, but that's fine. - */ - break; + CompilerError.invariant(originalInstrs[cursor].id === rewrite.location, { + reason: + '[InferEffectDependencies] Internal invariant broken: splice location not found', + loc: originalInstrs[cursor].loc, + }); + + if (rewrite.kind === 'instr') { + currBlock.instructions.push(rewrite.value); + } else { + const {entry, blocks} = rewrite.value; + const entryBlock = blocks.get(entry)!; + // splice in all instructions from the entry block + currBlock.instructions.push(...entryBlock.instructions); + if (blocks.size > 1) { + /** + * We're splicing in a set of value-blocks, which means we need + * to push new blocks and update terminals. + */ + CompilerError.invariant( + terminalFallthrough(entryBlock.terminal) === rewrite.exitBlockId, + { + reason: + '[InferEffectDependencies] Internal invariant broken: expected entry block to have a fallthrough', + loc: entryBlock.terminal.loc, + }, + ); + const originalTerminal = currBlock.terminal; + currBlock.terminal = entryBlock.terminal; + + for (const [id, block] of blocks) { + if (id === entry) { + continue; + } + if (id === rewrite.exitBlockId) { + block.terminal = originalTerminal; + currBlock = block; + } + rewriteBlocks.push(block); + } + } } - 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; + } + for (let i = cursor; i < originalInstrs.length; i++) { + currBlock.instructions.push(originalInstrs[i]); } - currValue.effect = Effect.Freeze; - return {place: currValue, instructions}; } function inferReactiveIdentifiers(fn: HIRFunction): Set { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-optional-chain.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-optional-chain.expect.md new file mode 100644 index 0000000000..e4560848dd --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-optional-chain.expect.md @@ -0,0 +1,58 @@ + +## Input + +```javascript +// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly +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; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{foo: 1}], +}; + +``` + +## Code + +```javascript +// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly +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; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ foo: 1 }], +}; + +``` + +## Logs + +``` +{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":139},"end":{"line":12,"column":1,"index":384},"filename":"mutate-after-useeffect-optional-chain.ts"},"detail":{"reason":"This mutates a variable that React considers immutable","description":null,"loc":{"start":{"line":10,"column":2,"index":345},"end":{"line":10,"column":5,"index":348},"filename":"mutate-after-useeffect-optional-chain.ts","identifierName":"arr"},"suggestions":null,"severity":"InvalidReact"}} +{"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":9,"column":2,"index":304},"end":{"line":9,"column":39,"index":341},"filename":"mutate-after-useeffect-optional-chain.ts"},"decorations":[{"start":{"line":9,"column":24,"index":326},"end":{"line":9,"column":27,"index":329},"filename":"mutate-after-useeffect-optional-chain.ts","identifierName":"arr"}]} +{"kind":"CompileSuccess","fnLoc":{"start":{"line":5,"column":0,"index":139},"end":{"line":12,"column":1,"index":384},"filename":"mutate-after-useeffect-optional-chain.ts"},"fnName":"Component","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0} +``` + +### Eval output +(kind: ok) [{"value":1}] +logs: [1] \ 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-optional-chain.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-optional-chain.js new file mode 100644 index 0000000000..c435b72d1a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-optional-chain.js @@ -0,0 +1,17 @@ +// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly +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; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{foo: 1}], +}; 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 index 05ef40c150..5e6f19dd83 100644 --- 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 @@ -2,7 +2,7 @@ ## Input ```javascript -// @inferEffectDependencies @panicThreshold:"none" +// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly import {useEffect, useRef} from 'react'; import {print} from 'shared-runtime'; @@ -14,12 +14,17 @@ function Component({arrRef}) { return arrRef; } +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{arrRef: {current: {val: 'initial ref value'}}}], +}; + ``` ## Code ```javascript -// @inferEffectDependencies @panicThreshold:"none" +// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly import { useEffect, useRef } from "react"; import { print } from "shared-runtime"; @@ -32,7 +37,21 @@ function Component(t0) { return arrRef; } +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ arrRef: { current: { val: "initial ref value" } } }], +}; + +``` + +## Logs + +``` +{"kind":"CompileError","fnLoc":{"start":{"line":6,"column":0,"index":148},"end":{"line":11,"column":1,"index":311},"filename":"mutate-after-useeffect-ref-access.ts"},"detail":{"reason":"Mutating component props or hook arguments is not allowed. Consider using a local variable instead","description":null,"loc":{"start":{"line":9,"column":2,"index":269},"end":{"line":9,"column":16,"index":283},"filename":"mutate-after-useeffect-ref-access.ts"},"suggestions":null,"severity":"InvalidReact"}} +{"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":8,"column":2,"index":227},"end":{"line":8,"column":40,"index":265},"filename":"mutate-after-useeffect-ref-access.ts"},"decorations":[{"start":{"line":8,"column":24,"index":249},"end":{"line":8,"column":30,"index":255},"filename":"mutate-after-useeffect-ref-access.ts","identifierName":"arrRef"}]} +{"kind":"CompileSuccess","fnLoc":{"start":{"line":6,"column":0,"index":148},"end":{"line":11,"column":1,"index":311},"filename":"mutate-after-useeffect-ref-access.ts"},"fnName":"Component","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0} ``` ### Eval output -(kind: exception) Fixture not implemented \ No newline at end of file +(kind: ok) {"current":{"val":2}} +logs: [{ val: 2 }] \ 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 index f497d7e595..bd3f6d1de5 100644 --- 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 @@ -1,4 +1,4 @@ -// @inferEffectDependencies @panicThreshold:"none" +// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly import {useEffect, useRef} from 'react'; import {print} from 'shared-runtime'; @@ -9,3 +9,8 @@ function Component({arrRef}) { arrRef.current.val = 2; return arrRef; } + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{arrRef: {current: {val: 'initial ref value'}}}], +}; 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 index fa1df3ef88..3b61fbf834 100644 --- 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 @@ -2,33 +2,55 @@ ## Input ```javascript -// @inferEffectDependencies @panicThreshold:"none" +// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly import {useEffect} from 'react'; function Component({foo}) { const arr = []; - useEffect(() => arr.push(foo)); + useEffect(() => { + arr.push(foo); + }); arr.push(2); return arr; } +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{foo: 1}], +}; + ``` ## Code ```javascript -// @inferEffectDependencies @panicThreshold:"none" +// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly import { useEffect } from "react"; function Component(t0) { const { foo } = t0; const arr = []; - useEffect(() => arr.push(foo), [arr, foo]); + useEffect(() => { + arr.push(foo); + }, [arr, foo]); arr.push(2); return arr; } +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ foo: 1 }], +}; + +``` + +## Logs + +``` +{"kind":"CompileError","fnLoc":{"start":{"line":4,"column":0,"index":101},"end":{"line":11,"column":1,"index":222},"filename":"mutate-after-useeffect.ts"},"detail":{"reason":"This mutates a variable that React considers immutable","description":null,"loc":{"start":{"line":9,"column":2,"index":194},"end":{"line":9,"column":5,"index":197},"filename":"mutate-after-useeffect.ts","identifierName":"arr"},"suggestions":null,"severity":"InvalidReact"}} +{"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":6,"column":2,"index":149},"end":{"line":8,"column":4,"index":190},"filename":"mutate-after-useeffect.ts"},"decorations":[{"start":{"line":7,"column":4,"index":171},"end":{"line":7,"column":7,"index":174},"filename":"mutate-after-useeffect.ts","identifierName":"arr"},{"start":{"line":7,"column":4,"index":171},"end":{"line":7,"column":7,"index":174},"filename":"mutate-after-useeffect.ts","identifierName":"arr"},{"start":{"line":7,"column":13,"index":180},"end":{"line":7,"column":16,"index":183},"filename":"mutate-after-useeffect.ts","identifierName":"foo"}]} +{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":101},"end":{"line":11,"column":1,"index":222},"filename":"mutate-after-useeffect.ts"},"fnName":"Component","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0} ``` ### Eval output -(kind: exception) Fixture not implemented \ No newline at end of file +(kind: ok) [2] \ 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 index 2e2eb7bc08..fbcbf004a3 100644 --- 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 @@ -1,9 +1,16 @@ -// @inferEffectDependencies @panicThreshold:"none" +// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly import {useEffect} from 'react'; function Component({foo}) { const arr = []; - useEffect(() => arr.push(foo)); + useEffect(() => { + arr.push(foo); + }); arr.push(2); return arr; } + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{foo: 1}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain-complex.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain-complex.expect.md new file mode 100644 index 0000000000..be107a2afa --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain-complex.expect.md @@ -0,0 +1,101 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print, shallowCopy} from 'shared-runtime'; + +// TODO: take optional chains as dependencies +function ReactiveMemberExpr({cond, propVal}) { + const obj = {a: cond ? {b: propVal} : null, c: null}; + const other = shallowCopy({a: {b: {c: {d: {e: {f: propVal + 1}}}}}}); + const primitive = shallowCopy(propVal); + useEffect(() => + print(obj.a?.b, other?.a?.b?.c?.d?.e.f, primitive.a?.b.c?.d?.e.f) + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: ReactiveMemberExpr, + params: [{cond: true, propVal: 1}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import { useEffect } from "react"; +import { print, shallowCopy } from "shared-runtime"; + +// TODO: take optional chains as dependencies +function ReactiveMemberExpr(t0) { + const $ = _c(13); + 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, c: null }; + $[3] = t1; + $[4] = t2; + } else { + t2 = $[4]; + } + const obj = t2; + const t3 = propVal + 1; + let t4; + if ($[5] !== t3) { + t4 = shallowCopy({ a: { b: { c: { d: { e: { f: t3 } } } } } }); + $[5] = t3; + $[6] = t4; + } else { + t4 = $[6]; + } + const other = t4; + let t5; + if ($[7] !== propVal) { + t5 = shallowCopy(propVal); + $[7] = propVal; + $[8] = t5; + } else { + t5 = $[8]; + } + const primitive = t5; + let t6; + if ( + $[9] !== obj.a?.b || + $[10] !== other?.a?.b?.c?.d?.e.f || + $[11] !== primitive.a?.b.c?.d?.e.f + ) { + t6 = () => + print(obj.a?.b, other?.a?.b?.c?.d?.e.f, primitive.a?.b.c?.d?.e.f); + $[9] = obj.a?.b; + $[10] = other?.a?.b?.c?.d?.e.f; + $[11] = primitive.a?.b.c?.d?.e.f; + $[12] = t6; + } else { + t6 = $[12]; + } + useEffect(t6, [obj.a?.b, other?.a?.b?.c?.d?.e.f, primitive.a?.b.c?.d?.e.f]); +} + +export const FIXTURE_ENTRYPOINT = { + fn: ReactiveMemberExpr, + params: [{ cond: true, propVal: 1 }], +}; + +``` + +### Eval output +(kind: ok) +logs: [1,2,undefined] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain-complex.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain-complex.js new file mode 100644 index 0000000000..f6e5a18d47 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain-complex.js @@ -0,0 +1,18 @@ +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print, shallowCopy} from 'shared-runtime'; + +// TODO: take optional chains as dependencies +function ReactiveMemberExpr({cond, propVal}) { + const obj = {a: cond ? {b: propVal} : null, c: null}; + const other = shallowCopy({a: {b: {c: {d: {e: {f: propVal + 1}}}}}}); + const primitive = shallowCopy(propVal); + useEffect(() => + print(obj.a?.b, other?.a?.b?.c?.d?.e.f, primitive.a?.b.c?.d?.e.f) + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: ReactiveMemberExpr, + params: [{cond: true, propVal: 1}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain.expect.md index 7c9f21b85c..d2414f3f9b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain.expect.md @@ -8,10 +8,16 @@ import {print} from 'shared-runtime'; // TODO: take optional chains as dependencies function ReactiveMemberExpr({cond, propVal}) { - const obj = {a: cond ? {b: propVal} : null}; + const obj = {a: cond ? {b: propVal} : null, c: null}; useEffect(() => print(obj.a?.b)); + useEffect(() => print(obj.c?.d)); } +export const FIXTURE_ENTRYPOINT = { + fn: ReactiveMemberExpr, + params: [{cond: true, propVal: 1}], +}; + ``` ## Code @@ -23,7 +29,7 @@ import { print } from "shared-runtime"; // TODO: take optional chains as dependencies function ReactiveMemberExpr(t0) { - const $ = _c(7); + const $ = _c(9); const { cond, propVal } = t0; let t1; if ($[0] !== cond || $[1] !== propVal) { @@ -36,7 +42,7 @@ function ReactiveMemberExpr(t0) { } let t2; if ($[3] !== t1) { - t2 = { a: t1 }; + t2 = { a: t1, c: null }; $[3] = t1; $[4] = t2; } else { @@ -51,10 +57,25 @@ function ReactiveMemberExpr(t0) { } else { t3 = $[6]; } - useEffect(t3, [obj.a]); + useEffect(t3, [obj.a?.b]); + let t4; + if ($[7] !== obj.c?.d) { + t4 = () => print(obj.c?.d); + $[7] = obj.c?.d; + $[8] = t4; + } else { + t4 = $[8]; + } + useEffect(t4, [obj.c?.d]); } +export const FIXTURE_ENTRYPOINT = { + fn: ReactiveMemberExpr, + params: [{ cond: true, propVal: 1 }], +}; + ``` ### Eval output -(kind: exception) Fixture not implemented \ No newline at end of file +(kind: ok) +logs: [1,undefined] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain.js index 8a76784e24..4f567fa152 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain.js @@ -4,6 +4,12 @@ import {print} from 'shared-runtime'; // TODO: take optional chains as dependencies function ReactiveMemberExpr({cond, propVal}) { - const obj = {a: cond ? {b: propVal} : null}; + const obj = {a: cond ? {b: propVal} : null, c: null}; useEffect(() => print(obj.a?.b)); + useEffect(() => print(obj.c?.d)); } + +export const FIXTURE_ENTRYPOINT = { + fn: ReactiveMemberExpr, + params: [{cond: true, propVal: 1}], +};