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 e74ec52203b..3bc7b135b6c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -1500,10 +1500,13 @@ export type ReactiveScopeDependency = { export function areEqualPaths(a: DependencyPath, b: DependencyPath): boolean { return ( a.length === b.length && - a.every((item, ix) => item.property === b[ix].property) + a.every( + (item, ix) => + item.property === b[ix].property && item.optional === b[ix].optional, + ) ); } -export type DependencyPath = Array<{property: string}>; +export type DependencyPath = Array<{property: string; optional: boolean}>; /* * Simulated opaque type for BlockIds to prevent using normal numbers as block ids 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 8bcab1d7b25..ecf0b5f0c60 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts @@ -869,7 +869,7 @@ export function printManualMemoDependency( ? val.root.value.identifier.name.value : printIdentifier(val.root.value.identifier); } - return `${rootStr}${val.path.length > 0 ? '.' : ''}${val.path.join('.')}`; + return `${rootStr}${val.path.map(v => `${v.optional ? '?.' : '.'}${v.property}`).join('')}`; } export function printType(type: Type): string { if (type.kind === 'Type') return ''; diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts index e60d8a99145..c580b3e8b75 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts @@ -68,7 +68,8 @@ export function collectMaybeMemoDependencies( if (object != null) { return { root: object.root, - path: [...object.path, {property: value.property}], + // TODO: determine if the access is optional + path: [...object.path, {property: value.property, optional: false}], }; } break; 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 13420ee140e..dcf67a36e5c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/DeriveMinimalDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/DeriveMinimalDependencies.ts @@ -6,7 +6,7 @@ */ import {CompilerError} from '../CompilerError'; -import {Identifier, ReactiveScopeDependency} from '../HIR'; +import {DependencyPath, Identifier, ReactiveScopeDependency} from '../HIR'; import {printIdentifier} from '../HIR/PrintHIR'; import {assertExhaustive} from '../Utils/utils'; @@ -252,7 +252,7 @@ type DependencyNode = { }; type ReduceResultNode = { - relativePath: Array<{property: string}>; + relativePath: DependencyPath; accessType: PropertyAccessType; }; @@ -283,7 +283,10 @@ function deriveMinimalDependenciesInSubtree( const childResult = deriveMinimalDependenciesInSubtree(childNode).map( ({relativePath, accessType}) => { return { - relativePath: [{property: childName}, ...relativePath], + relativePath: [ + {property: childName, optional: false}, + ...relativePath, + ], accessType, }; }, 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 0e47a8dcedf..8fd324ae2c9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PropagateScopeDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PropagateScopeDependencies.ts @@ -485,7 +485,7 @@ class Context { }; } - objectDependency.path.push({property}); + objectDependency.path.push({property, optional: false}); return objectDependency; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts index 43a477f3418..5a9a947d88b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts @@ -116,7 +116,7 @@ function prettyPrintScopeDependency(val: ReactiveScopeDependency): string { } else { rootStr = '[unnamed]'; } - return `${rootStr}${val.path.length > 0 ? '.' : ''}${val.path.join('.')}`; + return `${rootStr}${val.path.map(v => `${v.optional ? '?.' : '.'}${v.property}`).join('')}`; } enum CompareDependencyResult { 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 new file mode 100644 index 00000000000..7e4145a27c1 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-optional-member-expression-as-memo-dep.expect.md @@ -0,0 +1,32 @@ + +## 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/error.todo-optional-member-expression-as-memo-dep.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-optional-member-expression-as-memo-dep.js new file mode 100644 index 00000000000..fd8cf0214c8 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-optional-member-expression-as-memo-dep.js @@ -0,0 +1,7 @@ +// @validatePreserveExistingMemoizationGuarantees +function Component(props) { + const data = useMemo(() => { + return props.items?.edges?.nodes ?? []; + }, [props.items?.edges?.nodes]); + return ; +}