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 ecf0b5f0c6041..c2db20c5099a1 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts @@ -191,7 +191,7 @@ export function printTerminal(terminal: Terminal): Array | string { case 'branch': { value = `[${terminal.id}] Branch (${printPlace(terminal.test)}) then:bb${ terminal.consequent - } else:bb${terminal.alternate}`; + } else:bb${terminal.alternate} fallthrough:bb${terminal.fallthrough}`; break; } case 'logical': { diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts index 73330b959e018..2df7b5ed1c7fd 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -1446,9 +1446,19 @@ function codegenDependency( dependency: ReactiveScopeDependency, ): t.Expression { let object: t.Expression = convertIdentifier(dependency.identifier); - if (dependency.path !== null) { + if (dependency.path.length !== 0) { + const hasOptional = dependency.path.some(path => path.optional); for (const path of dependency.path) { - object = t.memberExpression(object, t.identifier(path.property)); + if (hasOptional) { + object = t.optionalMemberExpression( + object, + t.identifier(path.property), + false, + path.optional, + ); + } else { + object = t.memberExpression(object, t.identifier(path.property)); + } } } return object; diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/DeriveMinimalDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/DeriveMinimalDependencies.ts index dcf67a36e5c6b..e5fc50eb8abe3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/DeriveMinimalDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/DeriveMinimalDependencies.ts @@ -9,6 +9,7 @@ import {CompilerError} from '../CompilerError'; import {DependencyPath, Identifier, ReactiveScopeDependency} from '../HIR'; import {printIdentifier} from '../HIR/PrintHIR'; import {assertExhaustive} from '../Utils/utils'; +import {printDependency} from './PrintReactiveFunction'; /* * We need to understand optional member expressions only when determining @@ -60,13 +61,14 @@ export class ReactiveScopeDependencyTree { const {path} = dep; let currNode = this.#getOrCreateRoot(dep.identifier); - const accessType = inConditional - ? PropertyAccessType.ConditionalAccess - : PropertyAccessType.UnconditionalAccess; - for (const item of path) { // all properties read 'on the way' to a dependency are marked as 'access' let currChild = getOrMakeProperty(currNode, item.property); + const accessType = inConditional + ? PropertyAccessType.ConditionalAccess + : item.optional + ? PropertyAccessType.OptionalAccess + : PropertyAccessType.UnconditionalAccess; currChild.accessType = merge(currChild.accessType, accessType); currNode = currChild; } @@ -77,7 +79,9 @@ export class ReactiveScopeDependencyTree { */ const depType = inConditional ? PropertyAccessType.ConditionalDependency - : PropertyAccessType.UnconditionalDependency; + : isOptional(currNode.accessType) + ? PropertyAccessType.OptionalDependency + : PropertyAccessType.UnconditionalDependency; currNode.accessType = merge(currNode.accessType, depType); } @@ -85,10 +89,12 @@ export class ReactiveScopeDependencyTree { deriveMinimalDependencies(): Set { const results = new Set(); for (const [rootId, rootNode] of this.#roots.entries()) { - const deps = deriveMinimalDependenciesInSubtree(rootNode); + const deps = deriveMinimalDependenciesInSubtree(rootNode, null); CompilerError.invariant( deps.every( - dep => dep.accessType === PropertyAccessType.UnconditionalDependency, + dep => + dep.accessType === PropertyAccessType.UnconditionalDependency || + dep.accessType == PropertyAccessType.OptionalDependency, ), { reason: @@ -173,6 +179,27 @@ export class ReactiveScopeDependencyTree { } return res.flat().join('\n'); } + + debug(): string { + const buf: Array = [`tree() [`]; + for (const [rootId, rootNode] of this.#roots) { + buf.push(`${printIdentifier(rootId)} (${rootNode.accessType}):`); + this.#debugImpl(buf, rootNode, 1); + } + buf.push(']'); + return buf.length > 2 ? buf.join('\n') : buf.join(''); + } + + #debugImpl( + buf: Array, + node: DependencyNode, + depth: number = 0, + ): void { + for (const [property, childNode] of node.properties) { + buf.push(`${' '.repeat(depth)}.${property} (${childNode.accessType}):`); + this.#debugImpl(buf, childNode, depth + 1); + } + } } /* @@ -196,8 +223,10 @@ export class ReactiveScopeDependencyTree { */ enum PropertyAccessType { ConditionalAccess = 'ConditionalAccess', + OptionalAccess = 'OptionalAccess', UnconditionalAccess = 'UnconditionalAccess', ConditionalDependency = 'ConditionalDependency', + OptionalDependency = 'OptionalDependency', UnconditionalDependency = 'UnconditionalDependency', } @@ -211,9 +240,16 @@ function isUnconditional(access: PropertyAccessType): boolean { function isDependency(access: PropertyAccessType): boolean { return ( access === PropertyAccessType.ConditionalDependency || + access === PropertyAccessType.OptionalDependency || access === PropertyAccessType.UnconditionalDependency ); } +function isOptional(access: PropertyAccessType): boolean { + return ( + access === PropertyAccessType.OptionalAccess || + access === PropertyAccessType.OptionalDependency + ); +} function merge( access1: PropertyAccessType, @@ -222,6 +258,7 @@ function merge( const resultIsUnconditional = isUnconditional(access1) || isUnconditional(access2); const resultIsDependency = isDependency(access1) || isDependency(access2); + const resultIsOptional = isOptional(access1) || isOptional(access2); /* * Straightforward merge. @@ -237,6 +274,12 @@ function merge( } else { return PropertyAccessType.UnconditionalAccess; } + } else if (resultIsOptional) { + if (resultIsDependency) { + return PropertyAccessType.OptionalDependency; + } else { + return PropertyAccessType.OptionalAccess; + } } else { if (resultIsDependency) { return PropertyAccessType.ConditionalDependency; @@ -256,19 +299,34 @@ type ReduceResultNode = { accessType: PropertyAccessType; }; -const promoteUncondResult = [ - { +function promoteResult( + accessType: PropertyAccessType, + path: {property: string; optional: boolean} | null, +): Array { + const result: ReduceResultNode = { relativePath: [], - accessType: PropertyAccessType.UnconditionalDependency, - }, -]; + accessType, + }; + if (path !== null) { + result.relativePath.push(path); + } + return [result]; +} -const promoteCondResult = [ - { - relativePath: [], - accessType: PropertyAccessType.ConditionalDependency, - }, -]; +function prependPath( + results: Array, + path: {property: string; optional: boolean} | null, +): Array { + if (path === null) { + return results; + } + return results.map(result => { + return { + accessType: result.accessType, + relativePath: [path, ...result.relativePath], + }; + }); +} /* * Recursively calculates minimal dependencies in a subtree. @@ -277,42 +335,76 @@ const promoteCondResult = [ */ function deriveMinimalDependenciesInSubtree( dep: DependencyNode, + property: string | null, ): Array { const results: Array = []; for (const [childName, childNode] of dep.properties) { - const childResult = deriveMinimalDependenciesInSubtree(childNode).map( - ({relativePath, accessType}) => { - return { - relativePath: [ - {property: childName, optional: false}, - ...relativePath, - ], - accessType, - }; - }, + const childResult = deriveMinimalDependenciesInSubtree( + childNode, + childName, ); results.push(...childResult); } switch (dep.accessType) { case PropertyAccessType.UnconditionalDependency: { - return promoteUncondResult; + return promoteResult( + PropertyAccessType.UnconditionalDependency, + property !== null ? {property, optional: false} : null, + ); } case PropertyAccessType.UnconditionalAccess: { if ( results.every( ({accessType}) => - accessType === PropertyAccessType.UnconditionalDependency, + accessType === PropertyAccessType.UnconditionalDependency || + accessType === PropertyAccessType.OptionalDependency, ) ) { // all children are unconditional dependencies, return them to preserve granularity - return results; + return prependPath( + results, + property !== null ? {property, optional: false} : null, + ); } else { /* * at least one child is accessed conditionally, so this node needs to be promoted to * unconditional dependency */ - return promoteUncondResult; + return promoteResult( + PropertyAccessType.UnconditionalDependency, + property !== null ? {property, optional: false} : null, + ); + } + } + case PropertyAccessType.OptionalDependency: { + return promoteResult( + PropertyAccessType.OptionalDependency, + property !== null ? {property, optional: true} : null, + ); + } + case PropertyAccessType.OptionalAccess: { + if ( + results.every( + ({accessType}) => + accessType === PropertyAccessType.UnconditionalDependency || + accessType === PropertyAccessType.OptionalDependency, + ) + ) { + // all children are unconditional dependencies, return them to preserve granularity + return prependPath( + results, + property !== null ? {property, optional: true} : null, + ); + } else { + /* + * at least one child is accessed conditionally, so this node needs to be promoted to + * unconditional dependency + */ + return promoteResult( + PropertyAccessType.OptionalDependency, + property !== null ? {property, optional: true} : null, + ); } } case PropertyAccessType.ConditionalAccess: @@ -328,13 +420,19 @@ function deriveMinimalDependenciesInSubtree( * unconditional access. * Truncate results of child nodes here, since we shouldn't access them anyways */ - return promoteCondResult; + return promoteResult( + PropertyAccessType.ConditionalDependency, + property !== null ? {property, optional: true} : null, + ); } else { /* * at least one child is accessed unconditionally, so this node can be promoted to * unconditional dependency */ - return promoteUncondResult; + return promoteResult( + PropertyAccessType.UnconditionalDependency, + property !== null ? {property, optional: true} : null, + ); } } default: { diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PrintReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PrintReactiveFunction.ts index 80395a2e0ea41..b5aa44ead095d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PrintReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PrintReactiveFunction.ts @@ -113,7 +113,7 @@ export function printDependency(dependency: ReactiveScopeDependency): string { const identifier = printIdentifier(dependency.identifier) + printType(dependency.identifier.type); - return `${identifier}${dependency.path.map(token => `.${token.property}`).join('')}`; + return `${identifier}${dependency.path.map(token => `${token.optional ? '?.' : '.'}${token.property}`).join('')}`; } export function printReactiveInstructions( diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PropagateScopeDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PropagateScopeDependencies.ts index 8fd324ae2c9aa..0c204d9f54b8c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PropagateScopeDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PropagateScopeDependencies.ts @@ -5,6 +5,7 @@ * LICENSE file in the root directory of this source tree. */ +import prettyFormat from 'pretty-format'; import {CompilerError} from '../CompilerError'; import { areEqualPaths, @@ -17,11 +18,13 @@ import { isObjectMethodType, isRefValueType, isUseRefType, + LoadLocal, makeInstructionId, Place, PrunedReactiveScopeBlock, ReactiveFunction, ReactiveInstruction, + ReactiveOptionalCallValue, ReactiveScope, ReactiveScopeBlock, ReactiveScopeDependency, @@ -29,6 +32,7 @@ import { ReactiveValue, ScopeId, } from '../HIR/HIR'; +import {printIdentifier, printPlace} from '../HIR/PrintHIR'; import {eachInstructionValueOperand, eachPatternOperand} from '../HIR/visitors'; import {empty, Stack} from '../Utils/Stack'; import {assertExhaustive, Iterable_some} from '../Utils/utils'; @@ -36,6 +40,7 @@ import { ReactiveScopeDependencyTree, ReactiveScopePropertyDependency, } from './DeriveMinimalDependencies'; +import {printDependency, printReactiveFunction} from './PrintReactiveFunction'; import {ReactiveFunctionVisitor, visitReactiveFunction} from './visitors'; /* @@ -302,6 +307,7 @@ class Context { #properties: Map = new Map(); #temporaries: Map = new Map(); #inConditionalWithinScope: boolean = false; + inOptional: boolean = false; /* * Reactive dependencies used unconditionally in the current conditional. * Composed of dependencies: @@ -465,6 +471,7 @@ class Context { #getProperty( object: Place, property: string, + optional: boolean, ): ReactiveScopePropertyDependency { const resolvedObject = this.resolveTemporary(object); const resolvedDependency = this.#properties.get(resolvedObject.identifier); @@ -485,13 +492,18 @@ class Context { }; } - objectDependency.path.push({property, optional: false}); + objectDependency.path.push({property, optional}); return objectDependency; } - declareProperty(lvalue: Place, object: Place, property: string): void { - const nextDependency = this.#getProperty(object, property); + declareProperty( + lvalue: Place, + object: Place, + property: string, + optional: boolean, + ): void { + const nextDependency = this.#getProperty(object, property, optional); this.#properties.set(lvalue.identifier, nextDependency); } @@ -571,8 +583,8 @@ class Context { this.visitDependency(dependency); } - visitProperty(object: Place, property: string): void { - const nextDependency = this.#getProperty(object, property); + visitProperty(object: Place, property: string, optional: boolean): void { + const nextDependency = this.#getProperty(object, property, optional); this.visitDependency(nextDependency); } @@ -744,51 +756,102 @@ class PropagationVisitor extends ReactiveFunctionVisitor { }); } + visitOptionalExpression( + context: Context, + id: InstructionId, + value: ReactiveOptionalCallValue, + lvalue: Place | null, + ): void { + const wasWithinOptional = context.inOptional; + const isOptional = wasWithinOptional || value.optional; + const inner = value.value; + /* + * OptionalExpression value is a SequenceExpression where the instructions + * represent the code prior to the `?` and the final value represents the + * conditional code that follows. + */ + CompilerError.invariant(inner.kind === 'SequenceExpression', { + reason: 'Expected OptionalExpression value to be a SequenceExpression', + description: `Found a \`${value.kind}\``, + loc: value.loc, + }); + // Instructions are the unconditionally executed portion before the `?` + context.inOptional = isOptional; + for (const instr of inner.instructions) { + this.visitInstruction(instr, context); + } + context.inOptional = wasWithinOptional; + const innerValue = inner.value; + if ( + innerValue.kind !== 'SequenceExpression' || + innerValue.value.kind !== 'LoadLocal' + ) { + CompilerError.invariant(false, { + reason: + 'Expected OptionalExpression inner value to be a SequenceExpression', + description: `Found a \`${innerValue.kind}\``, + loc: innerValue.loc, + }); + } + if ( + isOptional && + innerValue.instructions.length === 1 && + innerValue.instructions[0].value.kind === 'PropertyLoad' && + innerValue.instructions[0].lvalue !== null && + innerValue.instructions[0].lvalue.identifier.id === + innerValue.value.place.identifier.id + ) { + const propertyLoad = innerValue.instructions[0].value; + if (lvalue !== null && !context.isUsedOutsideDeclaringScope(lvalue)) { + context.declareProperty( + lvalue, + propertyLoad.object, + propertyLoad.property, + value.optional, + ); + } else { + context.visitProperty( + propertyLoad.object, + propertyLoad.property, + value.optional, + ); + } + if (isOptional && !wasWithinOptional && lvalue !== null) { + context.visitOperand(lvalue); + } + return; + } + context.enterConditional(() => { + this.visitReactiveValue(context, id, innerValue, lvalue); + }); + } + visitReactiveValue( context: Context, id: InstructionId, value: ReactiveValue, + lvalue: Place | null, ): void { switch (value.kind) { case 'OptionalExpression': { - const inner = value.value; - /* - * OptionalExpression value is a SequenceExpression where the instructions - * represent the code prior to the `?` and the final value represents the - * conditional code that follows. - */ - CompilerError.invariant(inner.kind === 'SequenceExpression', { - reason: - 'Expected OptionalExpression value to be a SequenceExpression', - description: `Found a \`${value.kind}\``, - loc: value.loc, - suggestions: null, - }); - // Instructions are the unconditionally executed portion before the `?` - for (const instr of inner.instructions) { - this.visitInstruction(instr, context); - } - // The final value is the conditional portion following the `?` - context.enterConditional(() => { - this.visitReactiveValue(context, id, inner.value); - }); + this.visitOptionalExpression(context, id, value, lvalue); break; } case 'LogicalExpression': { - this.visitReactiveValue(context, id, value.left); + this.visitReactiveValue(context, id, value.left, null); context.enterConditional(() => { - this.visitReactiveValue(context, id, value.right); + this.visitReactiveValue(context, id, value.right, lvalue); }); break; } case 'ConditionalExpression': { - this.visitReactiveValue(context, id, value.test); + this.visitReactiveValue(context, id, value.test, null); const consequentDeps = context.enterConditional(() => { - this.visitReactiveValue(context, id, value.consequent); + this.visitReactiveValue(context, id, value.consequent, null); }); const alternateDeps = context.enterConditional(() => { - this.visitReactiveValue(context, id, value.alternate); + this.visitReactiveValue(context, id, value.alternate, null); }); context.promoteDepsFromExhaustiveConditionals([ consequentDeps, @@ -797,10 +860,34 @@ class PropagationVisitor extends ReactiveFunctionVisitor { break; } case 'SequenceExpression': { + /** + * Sequence + * = Sequence <-- we're here + * = ... + * LoadLocal + */ + if ( + lvalue !== null && + value.instructions.length === 1 && + value.value.kind === 'LoadLocal' && + value.value.place.identifier.id === lvalue.identifier.id && + value.instructions[0].lvalue !== null && + value.instructions[0].lvalue.identifier.id === + value.value.place.identifier.id + ) { + this.visitInstructionValue( + context, + value.instructions[0].id, + value.instructions[0].value, + lvalue, + ); + break; + } + for (const instr of value.instructions) { this.visitInstruction(instr, context); } - this.visitInstructionValue(context, id, value.value, null); + this.visitInstructionValue(context, id, value.value, lvalue); break; } case 'FunctionExpression': { @@ -851,9 +938,9 @@ class PropagationVisitor extends ReactiveFunctionVisitor { } } else if (value.kind === 'PropertyLoad') { if (lvalue !== null && !context.isUsedOutsideDeclaringScope(lvalue)) { - context.declareProperty(lvalue, value.object, value.property); + context.declareProperty(lvalue, value.object, value.property, false); } else { - context.visitProperty(value.object, value.property); + context.visitProperty(value.object, value.property, false); } } else if (value.kind === 'StoreLocal') { context.visitOperand(value.value); @@ -896,7 +983,7 @@ class PropagationVisitor extends ReactiveFunctionVisitor { }); } } else { - this.visitReactiveValue(context, id, value); + this.visitReactiveValue(context, id, value, lvalue); } } @@ -947,25 +1034,30 @@ class PropagationVisitor extends ReactiveFunctionVisitor { break; } case 'for': { - this.visitReactiveValue(context, terminal.id, terminal.init); - this.visitReactiveValue(context, terminal.id, terminal.test); + this.visitReactiveValue(context, terminal.id, terminal.init, null); + this.visitReactiveValue(context, terminal.id, terminal.test, null); context.enterConditional(() => { this.visitBlock(terminal.loop, context); if (terminal.update !== null) { - this.visitReactiveValue(context, terminal.id, terminal.update); + this.visitReactiveValue( + context, + terminal.id, + terminal.update, + null, + ); } }); break; } case 'for-of': { - this.visitReactiveValue(context, terminal.id, terminal.init); + this.visitReactiveValue(context, terminal.id, terminal.init, null); context.enterConditional(() => { this.visitBlock(terminal.loop, context); }); break; } case 'for-in': { - this.visitReactiveValue(context, terminal.id, terminal.init); + this.visitReactiveValue(context, terminal.id, terminal.init, null); context.enterConditional(() => { this.visitBlock(terminal.loop, context); }); @@ -974,12 +1066,12 @@ class PropagationVisitor extends ReactiveFunctionVisitor { case 'do-while': { this.visitBlock(terminal.loop, context); context.enterConditional(() => { - this.visitReactiveValue(context, terminal.id, terminal.test); + this.visitReactiveValue(context, terminal.id, terminal.test, null); }); break; } case 'while': { - this.visitReactiveValue(context, terminal.id, terminal.test); + this.visitReactiveValue(context, terminal.id, terminal.test, null); context.enterConditional(() => { this.visitBlock(terminal.loop, context); }); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-optional-member-expression-as-memo-dep.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-optional-member-expression-as-memo-dep.expect.md deleted file mode 100644 index 7e4145a27c16b..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-optional-member-expression-as-memo-dep.expect.md +++ /dev/null @@ -1,32 +0,0 @@ - -## Input - -```javascript -// @validatePreserveExistingMemoizationGuarantees -function Component(props) { - const data = useMemo(() => { - return props.items?.edges?.nodes ?? []; - }, [props.items?.edges?.nodes]); - return ; -} - -``` - - -## Error - -``` - 1 | // @validatePreserveExistingMemoizationGuarantees - 2 | function Component(props) { -> 3 | const data = useMemo(() => { - | ^^^^^^^ -> 4 | return props.items?.edges?.nodes ?? []; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -> 5 | }, [props.items?.edges?.nodes]); - | ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (3:5) - 6 | return ; - 7 | } - 8 | -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-as-memo-dep.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-as-memo-dep.expect.md new file mode 100644 index 0000000000000..393d73edc5369 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-as-memo-dep.expect.md @@ -0,0 +1,48 @@ + +## Input + +```javascript +// @validatePreserveExistingMemoizationGuarantees +function Component(props) { + const data = useMemo(() => { + return props?.items.edges?.nodes.map(); + }, [props?.items.edges?.nodes]); + return ; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees +function Component(props) { + const $ = _c(4); + + props?.items.edges?.nodes; + let t0; + let t1; + if ($[0] !== props?.items.edges?.nodes) { + t1 = props?.items.edges?.nodes.map(); + $[0] = props?.items.edges?.nodes; + $[1] = t1; + } else { + t1 = $[1]; + } + t0 = t1; + const data = t0; + let t2; + if ($[2] !== data) { + t2 = ; + $[2] = data; + $[3] = t2; + } else { + t2 = $[3]; + } + return t2; +} + +``` + +### 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.todo-optional-member-expression-as-memo-dep.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-as-memo-dep.js similarity index 64% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-optional-member-expression-as-memo-dep.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-as-memo-dep.js index fd8cf0214c87b..286d3be84a21b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-optional-member-expression-as-memo-dep.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-as-memo-dep.js @@ -1,7 +1,7 @@ // @validatePreserveExistingMemoizationGuarantees function Component(props) { const data = useMemo(() => { - return props.items?.edges?.nodes ?? []; - }, [props.items?.edges?.nodes]); + return props?.items.edges?.nodes.map(); + }, [props?.items.edges?.nodes]); return ; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-infer-less-specific-conditional-access.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-infer-less-specific-conditional-access.expect.md index 0cda150692955..25d7bf5f7ccce 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-infer-less-specific-conditional-access.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-infer-less-specific-conditional-access.expect.md @@ -44,8 +44,6 @@ function Component({propA, propB}) { | ^^^^^^^^^^^^^^^^^ > 14 | }, [propA?.a, propB.x.y]); | ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (6:14) - -CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (6:14) 15 | } 16 | ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-dependencies-non-optional-properties-inside-optional-chain.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-dependencies-non-optional-properties-inside-optional-chain.expect.md index 12a84b14f449a..896a547fec25c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-dependencies-non-optional-properties-inside-optional-chain.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-dependencies-non-optional-properties-inside-optional-chain.expect.md @@ -15,9 +15,9 @@ import { c as _c } from "react/compiler-runtime"; function Component(props) { const $ = _c(2); let t0; - if ($[0] !== props.post.feedback.comments) { + if ($[0] !== props.post.feedback.comments?.edges) { t0 = props.post.feedback.comments?.edges?.map(render); - $[0] = props.post.feedback.comments; + $[0] = props.post.feedback.comments?.edges; $[1] = t0; } else { t0 = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/conditional-member-expr.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/conditional-member-expr.expect.md index 59b48898fda16..2dd61732f689f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/conditional-member-expr.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/conditional-member-expr.expect.md @@ -29,10 +29,10 @@ import { c as _c } from "react/compiler-runtime"; // To preserve the nullthrows function Component(props) { const $ = _c(2); let x; - if ($[0] !== props.a) { + if ($[0] !== props.a?.b) { x = []; x.push(props.a?.b); - $[0] = props.a; + $[0] = props.a?.b; $[1] = x; } else { x = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain.expect.md index 7362cd8317091..a66540655ab79 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain.expect.md @@ -44,11 +44,11 @@ import { c as _c } from "react/compiler-runtime"; // To preserve the nullthrows function Component(props) { const $ = _c(2); let x; - if ($[0] !== props.a) { + if ($[0] !== props.a.b) { x = []; x.push(props.a?.b); x.push(props.a.b.c); - $[0] = props.a; + $[0] = props.a.b; $[1] = x; } else { x = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain2.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain2.expect.md index f25ea2a31e552..c45887675fa35 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain2.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain2.expect.md @@ -21,12 +21,20 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; function Component(props) { - const $ = _c(2); + const $ = _c(4); let x; if ($[0] !== props.items) { x = []; x.push(props.items?.length); - x.push(props.items?.edges?.map?.(render)?.filter?.(Boolean) ?? []); + let t0; + if ($[2] !== props) { + t0 = props.items?.edges?.map?.(render)?.filter?.(Boolean) ?? []; + $[2] = props; + $[3] = t0; + } else { + t0 = $[3]; + } + x.push(t0); $[0] = props.items; $[1] = x; } else { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md index 2ce8ffbe4c92e..d8e59c486a55b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md @@ -23,14 +23,14 @@ function HomeDiscoStoreItemTileRating(props) { const $ = _c(4); const item = useFragment(); let count; - if ($[0] !== item) { + if ($[0] !== item?.aggregates) { count = 0; const aggregates = item?.aggregates || []; aggregates.forEach((aggregate) => { count = count + (aggregate.count || 0); count; }); - $[0] = item; + $[0] = item?.aggregates; $[1] = count; } else { count = $[1];