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..991ac9e72d16c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -300,6 +300,10 @@ export type FunctionEffect = places: ReadonlySet; effect: Effect; loc: SourceLocation; + } + | { + kind: 'CaptureEffect'; + places: ReadonlySet; }; /* diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts index c8182c9e72a7c..93acb4c9447ff 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts @@ -546,12 +546,23 @@ export function printInstructionValue(instrValue: ReactiveValue): string { const effects = instrValue.loweredFunc.func.effects ?.map(effect => { - if (effect.kind === 'ContextMutation') { - return `ContextMutation places=[${[...effect.places] - .map(place => printPlace(place)) - .join(', ')}] effect=${effect.effect}`; - } else { - return `GlobalMutation`; + switch (effect.kind) { + case 'ContextMutation': { + return `ContextMutation places=[${[...effect.places] + .map(place => printPlace(place)) + .join(', ')}] effect=${effect.effect}`; + } + case 'GlobalMutation': { + return 'GlobalMutation'; + } + case 'ReactMutation': { + return 'ReactMutation'; + } + case 'CaptureEffect': { + return `CaptureEffect places=[${[...effect.places] + .map(place => printPlace(place)) + .join(', ')}]`; + } } }) .join(', ') ?? ''; diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts index a439b4cd01232..b7322d901a2dc 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts @@ -11,6 +11,7 @@ import { HIRFunction, Identifier, LoweredFunction, + Place, isRefOrRefValue, makeInstructionId, } from '../HIR'; @@ -19,6 +20,14 @@ import {inferReactiveScopeVariables} from '../ReactiveScopes'; import {rewriteInstructionKindsBasedOnReassignment} from '../SSA'; import {inferMutableRanges} from './InferMutableRanges'; import inferReferenceEffects from './InferReferenceEffects'; +import DisjointSet from '../Utils/DisjointSet'; +import { + eachInstructionLValue, + eachInstructionValueOperand, +} from '../HIR/visitors'; +import prettyFormat from 'pretty-format'; +import {printIdentifier} from '../HIR/PrintHIR'; +import {Iterable_some} from '../Utils/utils'; export default function analyseFunctions(func: HIRFunction): void { for (const [_, block] of func.body.blocks) { @@ -26,8 +35,8 @@ export default function analyseFunctions(func: HIRFunction): void { switch (instr.value.kind) { case 'ObjectMethod': case 'FunctionExpression': { - lower(instr.value.loweredFunc.func); - infer(instr.value.loweredFunc); + const aliases = lower(instr.value.loweredFunc.func); + infer(instr.value.loweredFunc, aliases); /** * Reset mutable range for outer inferReferenceEffects @@ -44,11 +53,11 @@ export default function analyseFunctions(func: HIRFunction): void { } } -function lower(func: HIRFunction): void { +function lower(func: HIRFunction): DisjointSet { analyseFunctions(func); inferReferenceEffects(func, {isFunctionExpression: true}); deadCodeElimination(func); - inferMutableRanges(func); + const aliases = inferMutableRanges(func); rewriteInstructionKindsBasedOnReassignment(func); inferReactiveScopeVariables(func); func.env.logger?.debugLogIRs?.({ @@ -56,9 +65,70 @@ function lower(func: HIRFunction): void { name: 'AnalyseFunction (inner)', value: func, }); + inferAliasesForCapturing(func, aliases); + return aliases; +} + +export function debugAliases(aliases: DisjointSet): void { + console.log( + prettyFormat( + aliases + .buildSets() + .map(set => [...set].map(ident => printIdentifier(ident))), + ), + ); +} + +/** + * The alias sets returned by InferMutableRanges() accounts only for aliases that + * are known to mutate together. Notably this skips cases where a value is captured + * into some other value, but neither is subsequently mutated. An example is pushing + * a mutable value onto an array, where neither the array or value are subsequently + * mutated. + * + * This function extends the aliases sets to account for such capturing, so that we + * can detect cases where one of the values in a set is mutated later (in an outer function) + * we can correctly infer them as mutating together. + */ +function inferAliasesForCapturing( + fn: HIRFunction, + aliases: DisjointSet, +): void { + for (const block of fn.body.blocks.values()) { + for (const instr of block.instructions) { + const {lvalue, value} = instr; + const hasStore = + lvalue.effect === Effect.Store || + Iterable_some( + eachInstructionValueOperand(value), + operand => operand.effect === Effect.Store, + ); + if (!hasStore) { + continue; + } + const operands: Array = []; + for (const lvalue of eachInstructionLValue(instr)) { + operands.push(lvalue.identifier); + } + for (const operand of eachInstructionValueOperand(instr.value)) { + if ( + operand.effect === Effect.Store || + operand.effect === Effect.Capture + ) { + operands.push(operand.identifier); + } + } + if (operands.length > 1) { + aliases.union(operands); + } + } + } } -function infer(loweredFunc: LoweredFunction): void { +function infer( + loweredFunc: LoweredFunction, + aliases: DisjointSet, +): void { for (const operand of loweredFunc.func.context) { const identifier = operand.identifier; CompilerError.invariant(operand.effect === Effect.Unknown, { @@ -85,6 +155,23 @@ function infer(loweredFunc: LoweredFunction): void { operand.effect = Effect.Read; } } + const contextIdentifiers = new Map( + loweredFunc.func.context.map(place => [place.identifier, place]), + ); + for (const set of aliases.buildSets()) { + const contextOperands: Set = new Set( + [...set] + .map(identifier => contextIdentifiers.get(identifier)) + .filter(place => place != null) as Array, + ); + if (contextOperands.size !== 0) { + loweredFunc.func.effects ??= []; + loweredFunc.func.effects?.push({ + kind: 'CaptureEffect', + places: contextOperands, + }); + } + } } function isMutatedOrReassigned(id: Identifier): boolean { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAlias.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAlias.ts index 80422c8391f46..fd7ec6becd701 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAlias.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAlias.ts @@ -60,6 +60,10 @@ function inferInstr( alias = instrValue.value; break; } + case 'IteratorNext': { + alias = instrValue.collection; + break; + } default: return; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAliasesForFunctionCaptureEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAliasesForFunctionCaptureEffects.ts new file mode 100644 index 0000000000000..8bc7cee4f9a79 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAliasesForFunctionCaptureEffects.ts @@ -0,0 +1,33 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import {HIRFunction, Identifier} from '../HIR/HIR'; +import DisjointSet from '../Utils/DisjointSet'; + +export function inferAliasForFunctionCaptureEffects( + func: HIRFunction, + aliases: DisjointSet, +): void { + for (const [_, block] of func.body.blocks) { + for (const instr of block.instructions) { + const {value} = instr; + if ( + value.kind !== 'FunctionExpression' && + value.kind !== 'ObjectMethod' + ) { + continue; + } + const loweredFunction = value.loweredFunc.func; + for (const effect of loweredFunction.effects ?? []) { + if (effect.kind !== 'CaptureEffect') { + continue; + } + aliases.union([...effect.places].map(place => place.identifier)); + } + } + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts index a58ae440219b9..ba09bcba1e9a4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts @@ -95,45 +95,58 @@ function inheritFunctionEffects( return effects .flatMap(effect => { - if (effect.kind === 'GlobalMutation' || effect.kind === 'ReactMutation') { - return [effect]; - } else { - const effects: Array = []; - CompilerError.invariant(effect.kind === 'ContextMutation', { - reason: 'Expected ContextMutation', - loc: null, - }); - /** - * Contextual effects need to be replayed against the current inference - * state, which may know more about the value to which the effect applied. - * The main cases are: - * 1. The mutated context value is _still_ a context value in the current scope, - * so we have to continue propagating the original context mutation. - * 2. The mutated context value is a mutable value in the current scope, - * so the context mutation was fine and we can skip propagating the effect. - * 3. The mutated context value is an immutable value in the current scope, - * resulting in a non-ContextMutation FunctionEffect. We propagate that new, - * more detailed effect to the current function context. - */ - for (const place of effect.places) { - if (state.isDefined(place)) { - const replayedEffect = inferOperandEffect(state, { - ...place, - loc: effect.loc, - effect: effect.effect, - }); - if (replayedEffect != null) { - if (replayedEffect.kind === 'ContextMutation') { - // Case 1, still a context value so propagate the original effect - effects.push(effect); - } else { - // Case 3, immutable value so propagate the more precise effect - effects.push(replayedEffect); - } - } // else case 2, local mutable value so this effect was fine + switch (effect.kind) { + case 'GlobalMutation': + case 'ReactMutation': { + return [effect]; + } + case 'ContextMutation': { + const effects: Array = []; + CompilerError.invariant(effect.kind === 'ContextMutation', { + reason: 'Expected ContextMutation', + loc: null, + }); + /** + * Contextual effects need to be replayed against the current inference + * state, which may know more about the value to which the effect applied. + * The main cases are: + * 1. The mutated context value is _still_ a context value in the current scope, + * so we have to continue propagating the original context mutation. + * 2. The mutated context value is a mutable value in the current scope, + * so the context mutation was fine and we can skip propagating the effect. + * 3. The mutated context value is an immutable value in the current scope, + * resulting in a non-ContextMutation FunctionEffect. We propagate that new, + * more detailed effect to the current function context. + */ + for (const place of effect.places) { + if (state.isDefined(place)) { + const replayedEffect = inferOperandEffect(state, { + ...place, + loc: effect.loc, + effect: effect.effect, + }); + if (replayedEffect != null) { + if (replayedEffect.kind === 'ContextMutation') { + // Case 1, still a context value so propagate the original effect + effects.push(effect); + } else { + // Case 3, immutable value so propagate the more precise effect + effects.push(replayedEffect); + } + } // else case 2, local mutable value so this effect was fine + } } + return effects; + } + case 'CaptureEffect': { + return []; + } + default: { + assertExhaustive( + effect, + `Unexpected effect kind '${(effect as any).kind}'`, + ); } - return effects; } }) .filter((effect): effect is FunctionEffect => effect != null); @@ -298,26 +311,31 @@ export function inferTerminalFunctionEffects( export function transformFunctionEffectErrors( functionEffects: Array, ): Array { - return functionEffects.map(eff => { - switch (eff.kind) { - case 'ReactMutation': - case 'GlobalMutation': { - return eff.error; - } - case 'ContextMutation': { - return { - severity: ErrorSeverity.Invariant, - reason: `Unexpected ContextMutation in top-level function effects`, - loc: eff.loc, - }; + return functionEffects + .map(eff => { + switch (eff.kind) { + case 'ReactMutation': + case 'GlobalMutation': { + return eff.error; + } + case 'ContextMutation': { + return { + severity: ErrorSeverity.Invariant, + reason: `Unexpected ContextMutation in top-level function effects`, + loc: eff.loc, + }; + } + case 'CaptureEffect': { + return null; + } + default: + assertExhaustive( + eff, + `Unexpected function effect kind \`${(eff as any).kind}\``, + ); } - default: - assertExhaustive( - eff, - `Unexpected function effect kind \`${(eff as any).kind}\``, - ); - } - }); + }) + .filter(eff => eff != null) as Array; } function isEffectSafeOutsideRender(effect: FunctionEffect): boolean { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts index 624c302fbf7ee..27230b9e7f453 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts @@ -6,15 +6,17 @@ */ import {HIRFunction, Identifier} from '../HIR/HIR'; +import DisjointSet from '../Utils/DisjointSet'; import {inferAliasForUncalledFunctions} from './InerAliasForUncalledFunctions'; import {inferAliases} from './InferAlias'; +import {inferAliasForFunctionCaptureEffects} from './InferAliasesForFunctionCaptureEffects'; import {inferAliasForPhis} from './InferAliasForPhis'; import {inferAliasForStores} from './InferAliasForStores'; import {inferMutableLifetimes} from './InferMutableLifetimes'; import {inferMutableRangesForAlias} from './InferMutableRangesForAlias'; import {inferTryCatchAliases} from './InferTryCatchAliases'; -export function inferMutableRanges(ir: HIRFunction): void { +export function inferMutableRanges(ir: HIRFunction): DisjointSet { // Infer mutable ranges for non fields inferMutableLifetimes(ir, false); @@ -38,6 +40,8 @@ export function inferMutableRanges(ir: HIRFunction): void { // Update aliasing information of fields inferAliasForStores(ir, aliases); + inferAliasForFunctionCaptureEffects(ir, aliases); + // Update aliasing information of phis inferAliasForPhis(ir, aliases); @@ -84,6 +88,8 @@ export function inferMutableRanges(ir: HIRFunction): void { } prevAliases = nextAliases; } + + return aliases; } function areEqualMaps(a: Map, b: Map): boolean { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts index 3dc532c87d6ea..01d78663d0a2b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts @@ -31,7 +31,6 @@ import { isArrayType, isMapType, isMutableEffect, - isObjectType, isSetType, } from '../HIR/HIR'; import {FunctionSignature} from '../HIR/ObjectShape'; @@ -48,7 +47,7 @@ import { eachTerminalOperand, eachTerminalSuccessor, } from '../HIR/visitors'; -import {assertExhaustive} from '../Utils/utils'; +import {assertExhaustive, retainWhere} from '../Utils/utils'; import { inferTerminalFunctionEffects, inferInstructionFunctionEffects, @@ -521,7 +520,7 @@ class InferenceState { * `expected valueKind to be 'Mutable' but found to be \`${valueKind}\`` * ); */ - effect = isObjectType(place.identifier) ? Effect.Store : Effect.Mutate; + effect = Effect.Store; break; } case Effect.Capture: { @@ -670,11 +669,7 @@ class InferenceState { for (const [value, kind] of this.#values) { const id = identify(value); result.values[id] = { - abstract: { - kind: kind.kind, - context: [...kind.context].map(printPlace), - reason: [...kind.reason], - }, + abstract: this.debugAbstractValue(kind), value: printInstructionValue(value), }; } @@ -684,6 +679,14 @@ class InferenceState { return result; } + debugAbstractValue(value: AbstractValue): any { + return { + kind: value.kind, + context: [...value.context].map(printPlace), + reason: [...value.reason], + }; + } + inferPhi(phi: Phi): void { const values: Set = new Set(); for (const [_, operand] of phi.operands) { @@ -909,19 +912,11 @@ function inferBlock( break; } case 'ArrayExpression': { - const contextRefOperands = getContextRefOperand(state, instrValue); - const valueKind: AbstractValue = - contextRefOperands.length > 0 - ? { - kind: ValueKind.Context, - reason: new Set([ValueReason.Other]), - context: new Set(contextRefOperands), - } - : { - kind: ValueKind.Mutable, - reason: new Set([ValueReason.Other]), - context: new Set(), - }; + const valueKind: AbstractValue = { + kind: ValueKind.Mutable, + reason: new Set([ValueReason.Other]), + context: new Set(), + }; for (const element of instrValue.elements) { if (element.kind === 'Spread') { @@ -942,6 +937,7 @@ function inferBlock( let _: 'Hole' = element.kind; } } + state.initialize(instrValue, valueKind); state.define(instr.lvalue, instrValue); instr.lvalue.effect = Effect.Store; @@ -961,19 +957,11 @@ function inferBlock( break; } case 'ObjectExpression': { - const contextRefOperands = getContextRefOperand(state, instrValue); - const valueKind: AbstractValue = - contextRefOperands.length > 0 - ? { - kind: ValueKind.Context, - reason: new Set([ValueReason.Other]), - context: new Set(contextRefOperands), - } - : { - kind: ValueKind.Mutable, - reason: new Set([ValueReason.Other]), - context: new Set(), - }; + const valueKind: AbstractValue = { + kind: ValueKind.Mutable, + reason: new Set([ValueReason.Other]), + context: new Set(), + }; for (const property of instrValue.properties) { switch (property.kind) { @@ -1197,6 +1185,35 @@ function inferBlock( ); hasMutableOperand ||= isMutableEffect(operand.effect, operand.loc); } + + /* + * Filter CaptureEffects to remove values that are immutable and don't + * need to be tracked for aliasing + */ + const effects = instrValue.loweredFunc.func.effects; + if (effects != null && effects.length !== 0) { + retainWhere(effects, effect => { + if (effect.kind !== 'CaptureEffect') { + return true; + } + const places: Set = new Set(); + for (const place of effect.places) { + const kind = state.kind(place); + if ( + kind.kind === ValueKind.Context || + kind.kind === ValueKind.MaybeFrozen || + kind.kind === ValueKind.Mutable + ) { + places.add(place); + } + } + if (places.size === 0) { + return false; + } + effect.places = places; + return true; + }); + } /* * If a closure did not capture any mutable values, then we can consider it to be * frozen, which allows it to be independently memoized. @@ -1287,20 +1304,18 @@ function inferBlock( break; } case 'PropertyStore': { - const effect = - state.kind(instrValue.object).kind === ValueKind.Context - ? Effect.ConditionallyMutate - : Effect.Capture; state.referenceAndRecordEffects( freezeActions, instrValue.value, - effect, + Effect.Capture, ValueReason.Other, ); state.referenceAndRecordEffects( freezeActions, instrValue.object, - Effect.Store, + typeof instrValue.property === 'string' + ? Effect.Store + : Effect.Mutate, ValueReason.Other, ); @@ -1818,7 +1833,9 @@ function inferBlock( state.isDefined(operand) && ((operand.identifier.type.kind === 'Function' && state.isFunctionExpression) || - state.kind(operand).kind === ValueKind.Context) + state.kind(operand).kind === ValueKind.Context || + (state.kind(operand).kind === ValueKind.Mutable && + state.isFunctionExpression)) ) { /** * Returned values should only be typed as 'frozen' if they are both (1) @@ -1845,22 +1862,6 @@ function inferBlock( ); } -function getContextRefOperand( - state: InferenceState, - instrValue: InstructionValue, -): Array { - const result = []; - for (const place of eachInstructionValueOperand(instrValue)) { - if (state.isDefined(place)) { - const kind = state.kind(place); - if (kind.kind === ValueKind.Context) { - result.push(...kind.context); - } - } - } - return result; -} - export function getFunctionCallSignature( env: Environment, type: Type, diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-map-captures-receiver-noAlias.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-map-captures-receiver-noAlias.expect.md index 1680386c741a3..efd094c1a5360 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-map-captures-receiver-noAlias.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-map-captures-receiver-noAlias.expect.md @@ -23,34 +23,18 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; function Component(props) { - const $ = _c(6); + const $ = _c(2); let t0; if ($[0] !== props.a) { - t0 = { a: props.a }; + const item = { a: props.a }; + const items = [item]; + t0 = items.map(_temp); $[0] = props.a; $[1] = t0; } else { t0 = $[1]; } - const item = t0; - let t1; - if ($[2] !== item) { - t1 = [item]; - $[2] = item; - $[3] = t1; - } else { - t1 = $[3]; - } - const items = t1; - let t2; - if ($[4] !== items) { - t2 = items.map(_temp); - $[4] = items; - $[5] = t2; - } else { - t2 = $[5]; - } - const mapped = t2; + const mapped = t0; return mapped; } function _temp(item_0) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-effect-function-mutates-ref.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-effect-function-mutates-ref.expect.md deleted file mode 100644 index b3f1104239938..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-effect-function-mutates-ref.expect.md +++ /dev/null @@ -1,48 +0,0 @@ - -## Input - -```javascript -// @validateNoFreezingKnownMutableFunctions - -import {useCallback, useEffect, useRef} from 'react'; -import {useHook} from 'shared-runtime'; - -function Component() { - const params = useHook(); - const update = useCallback( - partialParams => { - const nextParams = { - ...params, - ...partialParams, - }; - nextParams.param = 'value'; - console.log(nextParams); - }, - [params] - ); - const ref = useRef(null); - useEffect(() => { - if (ref.current === null) { - update(); - } - }, [update]); - - return 'ok'; -} - -``` - - -## Error - -``` - 12 | ...partialParams, - 13 | }; -> 14 | nextParams.param = 'value'; - | ^^^^^^^^^^ InvalidReact: Mutating a value returned from a function whose return value should not be mutated. Found mutation of `params` (14:14) - 15 | console.log(nextParams); - 16 | }, - 17 | [params] -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-local-mutation-incorrectly-flagged-as-context-mutation.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-local-mutation-incorrectly-flagged-as-context-mutation.expect.md new file mode 100644 index 0000000000000..cc3e8107994a5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-local-mutation-incorrectly-flagged-as-context-mutation.expect.md @@ -0,0 +1,89 @@ + +## Input + +```javascript +// @validateNoFreezingKnownMutableFunctions + +import {useCallback, useEffect, useRef} from 'react'; +import {useHook} from 'shared-runtime'; + +function Component() { + const params = useHook(); + const update = useCallback( + partialParams => { + const nextParams = { + ...params, + ...partialParams, + }; + // Due to how we previously represented ObjectExpressions in InferReferenceEffects, + // this was recorded as a mutation of a context value (`params`) which then made + // the function appear ineligible for freezing when passing to useEffect below. + nextParams.param = 'value'; + console.log(nextParams); + }, + [params] + ); + const ref = useRef(null); + useEffect(() => { + if (ref.current === null) { + update(); + } + }, [update]); + + return 'ok'; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoFreezingKnownMutableFunctions + +import { useCallback, useEffect, useRef } from "react"; +import { useHook } from "shared-runtime"; + +function Component() { + const $ = _c(5); + const params = useHook(); + let t0; + if ($[0] !== params) { + t0 = (partialParams) => { + const nextParams = { ...params, ...partialParams }; + + nextParams.param = "value"; + console.log(nextParams); + }; + $[0] = params; + $[1] = t0; + } else { + t0 = $[1]; + } + const update = t0; + + const ref = useRef(null); + let t1; + let t2; + if ($[2] !== update) { + t1 = () => { + if (ref.current === null) { + update(); + } + }; + + t2 = [update]; + $[2] = update; + $[3] = t1; + $[4] = t2; + } else { + t1 = $[3]; + t2 = $[4]; + } + useEffect(t1, t2); + return "ok"; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-effect-function-mutates-ref.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-local-mutation-incorrectly-flagged-as-context-mutation.js similarity index 67% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-effect-function-mutates-ref.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-local-mutation-incorrectly-flagged-as-context-mutation.js index 2e8b7827d09d9..5ba1eb1f648d1 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-effect-function-mutates-ref.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-local-mutation-incorrectly-flagged-as-context-mutation.js @@ -11,6 +11,9 @@ function Component() { ...params, ...partialParams, }; + // Due to how we previously represented ObjectExpressions in InferReferenceEffects, + // this was recorded as a mutation of a context value (`params`) which then made + // the function appear ineligible for freezing when passing to useEffect below. nextParams.param = 'value'; console.log(nextParams); },