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 831d1ca38054e..fe97c8d642f60 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -130,6 +130,7 @@ function run( mode, config, contextIdentifiers, + func, logger, filename, code, diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts index b9f82eea18e9f..cfb15fb595ccc 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts @@ -70,12 +70,14 @@ import {BuiltInArrayId} from './ObjectShape'; export function lower( func: NodePath, env: Environment, + // Bindings captured from the outer function, in case lower() is called recursively (for lambdas) bindings: Bindings | null = null, capturedRefs: Array = [], - // the outermost function being compiled, in case lower() is called recursively (for lambdas) - parent: NodePath | null = null, ): Result { - const builder = new HIRBuilder(env, parent ?? func, bindings, capturedRefs); + const builder = new HIRBuilder(env, { + bindings, + context: capturedRefs, + }); const context: HIRFunction['context'] = []; for (const ref of capturedRefs ?? []) { @@ -215,7 +217,7 @@ export function lower( return Ok({ id, params, - fnType: parent == null ? env.fnType : 'Other', + fnType: bindings == null ? env.fnType : 'Other', returnTypeAnnotation: null, // TODO: extract the actual return type node if present returnType: makeType(), body: builder.build(), @@ -3417,7 +3419,7 @@ function lowerFunction( | t.ObjectMethod >, ): LoweredFunction | null { - const componentScope: Scope = builder.parentFunction.scope; + const componentScope: Scope = builder.environment.parentFunction.scope; const capturedContext = gatherCapturedContext(expr, componentScope); /* @@ -3428,13 +3430,10 @@ function lowerFunction( * This isn't a problem in practice because use Babel's scope analysis to * identify the correct references. */ - const lowering = lower( - expr, - builder.environment, - builder.bindings, - [...builder.context, ...capturedContext], - builder.parentFunction, - ); + const lowering = lower(expr, builder.environment, builder.bindings, [ + ...builder.context, + ...capturedContext, + ]); let loweredFunc: HIRFunction; if (lowering.isErr()) { lowering @@ -3456,7 +3455,7 @@ function lowerExpressionToTemporary( return lowerValueToTemporary(builder, value); } -function lowerValueToTemporary( +export function lowerValueToTemporary( builder: HIRBuilder, value: InstructionValue, ): Place { 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 ea7268c573379..a11822538f54f 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 cb787d04d0623..75dad4c1bfe63 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 7f6fb9e88f817..7819ab39b2c69 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/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index 6e6643cd1d68f..27b578b3c7834 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -47,7 +47,7 @@ import { ShapeRegistry, addHook, } from './ObjectShape'; -import {Scope as BabelScope} from '@babel/traverse'; +import {Scope as BabelScope, NodePath} from '@babel/traverse'; import {TypeSchema} from './TypeSchema'; export const ReactElementSymbolSchema = z.object({ @@ -675,6 +675,7 @@ export class Environment { #contextIdentifiers: Set; #hoistedIdentifiers: Set; + parentFunction: NodePath; constructor( scope: BabelScope, @@ -682,6 +683,7 @@ export class Environment { compilerMode: CompilerMode, config: EnvironmentConfig, contextIdentifiers: Set, + parentFunction: NodePath, // the outermost function being compiled logger: Logger | null, filename: string | null, code: string | null, @@ -740,6 +742,7 @@ export class Environment { this.#moduleTypes.set(REANIMATED_MODULE_NAME, reanimatedModuleType); } + this.parentFunction = parentFunction; this.#contextIdentifiers = contextIdentifiers; this.#hoistedIdentifiers = new Set(); } 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 99b8c189ee0fd..1699a0fc3d292 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/HIRBuilder.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts index 44dd34b7d6cae..9ed37bb2fc85f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts @@ -110,7 +110,6 @@ export default class HIRBuilder { #bindings: Bindings; #env: Environment; #exceptionHandlerStack: Array = []; - parentFunction: NodePath; errors: CompilerError = new CompilerError(); /** * Traversal context: counts the number of `fbt` tag parents @@ -136,16 +135,17 @@ export default class HIRBuilder { constructor( env: Environment, - parentFunction: NodePath, // the outermost function being compiled - bindings: Bindings | null = null, - context: Array | null = null, + options?: { + bindings?: Bindings | null; + context?: Array; + entryBlockKind?: BlockKind; + }, ) { this.#env = env; - this.#bindings = bindings ?? new Map(); - this.parentFunction = parentFunction; - this.#context = context ?? []; + this.#bindings = options?.bindings ?? new Map(); + this.#context = options?.context ?? []; this.#entry = makeBlockId(env.nextBlockId); - this.#current = newBlock(this.#entry, 'block'); + this.#current = newBlock(this.#entry, options?.entryBlockKind ?? 'block'); } currentBlockKind(): BlockKind { @@ -239,7 +239,7 @@ export default class HIRBuilder { // Check if the binding is from module scope const outerBinding = - this.parentFunction.scope.parent.getBinding(originalName); + this.#env.parentFunction.scope.parent.getBinding(originalName); if (babelBinding === outerBinding) { const path = babelBinding.path; if (path.isImportDefaultSpecifier()) { @@ -293,7 +293,7 @@ export default class HIRBuilder { const binding = this.#resolveBabelBinding(path); if (binding) { // Check if the binding is from module scope, if so return null - const outerBinding = this.parentFunction.scope.parent.getBinding( + const outerBinding = this.#env.parentFunction.scope.parent.getBinding( path.node.name, ); if (binding === outerBinding) { @@ -376,7 +376,7 @@ export default class HIRBuilder { } // Terminate the current block w the given terminal, and start a new block - terminate(terminal: Terminal, nextBlockKind: BlockKind | null): void { + terminate(terminal: Terminal, nextBlockKind: BlockKind | null): BlockId { const {id: blockId, kind, instructions} = this.#current; this.#completed.set(blockId, { kind, @@ -390,6 +390,7 @@ export default class HIRBuilder { const nextId = this.#env.nextBlockId; this.#current = newBlock(nextId, nextBlockKind); } + return blockId; } /* @@ -746,6 +747,11 @@ function getReversePostorderedBlocks(func: HIR): HIR['blocks'] { * (eg bb2 then bb1), we ensure that they get reversed back to the correct order. */ const block = func.blocks.get(blockId)!; + CompilerError.invariant(block != null, { + reason: '[HIRBuilder] Unexpected null block', + description: `expected block ${blockId} to exist`, + loc: GeneratedSource, + }); const successors = [...eachTerminalSuccessor(block.terminal)].reverse(); const fallthrough = terminalFallthrough(block.terminal); 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 96b9e51710887..91b7712b881fc 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/HIR/ScopeDependencyUtils.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/ScopeDependencyUtils.ts new file mode 100644 index 0000000000000..5d30aeb6444ee --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/ScopeDependencyUtils.ts @@ -0,0 +1,283 @@ +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'; + +type DependencyInstructions = { + place: Place; + value: HIR; + exitBlockId: BlockId; +}; + +export function buildDependencyInstructions( + dep: ReactiveScopeDependency, + env: Environment, +): DependencyInstructions { + const builder = new HIRBuilder(env, { + entryBlockKind: 'value', + }); + let dependencyValue: Identifier; + if (dep.path.every(path => !path.optional)) { + dependencyValue = writeNonOptionalDependency(dep, env, builder); + } else { + dependencyValue = writeOptionalDependency(dep, builder, null); + } + + const exitBlockId = builder.terminate( + { + kind: 'unsupported', + loc: GeneratedSource, + id: makeInstructionId(0), + }, + null, + ); + return { + place: { + kind: 'Identifier', + identifier: dependencyValue, + effect: Effect.Freeze, + reactive: dep.reactive, + loc: GeneratedSource, + }, + value: builder.build(), + exitBlockId, + }; +} + +/** + * Write instructions for a simple dependency (without optional chains) + */ +function writeNonOptionalDependency( + dep: ReactiveScopeDependency, + env: Environment, + builder: HIRBuilder, +): Identifier { + const loc = dep.identifier.loc; + let curr: Identifier = makeTemporaryIdentifier(env.nextIdentifierId, loc); + builder.push({ + lvalue: { + identifier: curr, + 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, + }); + + /** + * Iteratively build up dependency instructions by reading from the last written + * instruction. + */ + 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: curr, + kind: 'Identifier', + effect: Effect.Freeze, + reactive: dep.reactive, + loc, + }, + property: path.property, + loc, + }, + id: makeInstructionId(1), + loc: loc, + }); + curr = next; + } + return curr; +} + +/** + * Write a dependency into optional blocks. + * + * e.g. `a.b?.c.d` is written to an optional block that tests `a.b` and + * conditionally evaluates `c.d`. + */ +function writeOptionalDependency( + dep: ReactiveScopeDependency, + builder: HIRBuilder, + parentAlternate: BlockId | null, +): Identifier { + const env = builder.environment; + /** + * Reserve an identifier which will be used to store the result of this + * dependency. + */ + const dependencyValue: Place = { + kind: 'Identifier', + identifier: makeTemporaryIdentifier(env.nextIdentifierId, GeneratedSource), + effect: Effect.Mutate, + reactive: dep.reactive, + loc: GeneratedSource, + }; + + /** + * Reserve a block which is the fallthrough (and transitive successor) of this + * optional chain. + */ + const continuationBlock = builder.reserve(builder.currentBlockKind()); + let alternate; + if (parentAlternate != null) { + alternate = parentAlternate; + } else { + /** + * If an outermost alternate block has not been reserved, write one + * + * $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: {...dependencyValue}}, + value: {...temp}, + type: null, + loc: GeneratedSource, + }); + return { + kind: 'goto', + variant: GotoVariant.Break, + block: continuationBlock.id, + id: makeInstructionId(0), + loc: GeneratedSource, + }; + }); + } + + // Reserve the consequent block, which is the successor of the test block + const consequent = builder.reserve('value'); + + let testIdentifier: Identifier | null = null; + const testBlock = builder.enter('value', () => { + const testDependency = { + ...dep, + path: dep.path.slice(0, dep.path.length - 1), + }; + const firstOptional = dep.path.findIndex(path => path.optional); + CompilerError.invariant(firstOptional !== -1, { + reason: + '[ScopeDependencyUtils] Internal invariant broken: expected optional path', + loc: dep.identifier.loc, + description: null, + suggestions: null, + }); + if (firstOptional === dep.path.length - 1) { + // Base case: the test block is simple + testIdentifier = writeNonOptionalDependency(testDependency, env, builder); + } else { + // Otherwise, the test block is a nested optional chain + testIdentifier = writeOptionalDependency( + testDependency, + 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, + }); + + lowerValueToTemporary(builder, { + kind: 'StoreLocal', + lvalue: {kind: InstructionKind.Const, place: {...dependencyValue}}, + value: lowerValueToTemporary(builder, { + kind: 'PropertyLoad', + object: { + identifier: testIdentifier, + kind: 'Identifier', + effect: Effect.Freeze, + reactive: dep.reactive, + loc: GeneratedSource, + }, + property: dep.path.at(-1)!.property, + loc: GeneratedSource, + }), + 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.at(-1)!.optional, + test: testBlock, + fallthrough: continuationBlock.id, + id: makeInstructionId(0), + loc: GeneratedSource, + }, + continuationBlock, + ); + + return dependencyValue.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 f1a584341912b..4daa2f9fbaee7 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'; @@ -38,13 +40,20 @@ import { createTemporaryPlace, fixScopeAndIdentifierRanges, markInstructionIds, + markPredecessors, + reversePostorderBlocks, } from '../HIR/HIRBuilder'; import { collectTemporariesSidemap, 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 +62,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 +94,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 +110,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 +174,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 +204,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 +246,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,85 +302,164 @@ 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); + } + + /** + * Fixup the HIR to restore RPO, ensure correct predecessors, and renumber + * instructions. + */ + reversePostorderBlocks(fn.body); + markPredecessors(fn.body); // Renumber instructions and fix scope ranges markInstructionIds(fn.body); fixScopeAndIdentifierRanges(fn.body); + fn.env.hasInferredEffect = true; } } -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; - } - 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; +): 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++; } - 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, - }, + CompilerError.invariant(originalInstrs[cursor].id === rewrite.location, { + reason: + '[InferEffectDependencies] Internal invariant broken: splice location not found', + loc: originalInstrs[cursor].loc, }); - currValue = nextValue; + + 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); + } + } + } } - currValue.effect = Effect.Freeze; - return {place: currValue, instructions}; + currBlock.instructions.push(...originalInstrs.slice(cursor)); } function inferReactiveIdentifiers(fn: HIRFunction): Set { 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 b05b292124c72..88faccd8cf3b6 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 08d2212d86b95..6f2d97ff8e528 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: [], })), ), 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 0000000000000..e4560848dd5f2 --- /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 0000000000000..c435b72d1a9ef --- /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 05ef40c150833..5e6f19dd83e65 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 f497d7e595e3b..bd3f6d1de54bd 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 fa1df3ef88722..3b61fbf834246 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 2e2eb7bc08d3d..fbcbf004a308c 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 0000000000000..a840ab81a5eee --- /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,99 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print, shallowCopy} from 'shared-runtime'; + +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"; + +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 0000000000000..93e10250cf389 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain-complex.js @@ -0,0 +1,17 @@ +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print, shallowCopy} from 'shared-runtime'; + +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 7c9f21b85cdab..ffd81732c6e96 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 @@ -6,12 +6,17 @@ 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}; + 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 @@ -21,9 +26,8 @@ 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 $ = _c(9); const { cond, propVal } = t0; let t1; if ($[0] !== cond || $[1] !== propVal) { @@ -36,7 +40,7 @@ function ReactiveMemberExpr(t0) { } let t2; if ($[3] !== t1) { - t2 = { a: t1 }; + t2 = { a: t1, c: null }; $[3] = t1; $[4] = t2; } else { @@ -51,10 +55,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 8a76784e241b5..d81f9029a4e3c 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 @@ -2,8 +2,13 @@ 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}; + 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}], +};