[compiler] Add DependencyPath optional property
Adds an `optional: boolean` property to each token in a DependencyPath, currently always set to false. Also updates the equality and printing logic for paths to account for this field. Subsequent PRs will update our logic to determine which manual dependencies were optional, then we can start inferring optional deps as well. ghstack-source-id: 66c2da2cfab5e5ba6c2ac5e20adae5e4f615ad29 Pull Request resolved: https://github.com/facebook/react/pull/30813
Joe Savona committed
Aug 28, 2024 at 10:52 UTC
a718da0b23c3f72ba6fb8e1bd087aca85f2b0b4a
8 files changed
+55
-9
compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
+5
-2
@@ -1500,10 +1500,13 @@ export type ReactiveScopeDependency = {
1500
export function areEqualPaths(a: DependencyPath, b: DependencyPath): boolean {
1501
return (
1502
a.length === b.length &&
1503
- a.every((item, ix) => item.property === b[ix].property)
1503
+ a.every(
1504
+ (item, ix) =>
1505
+ item.property === b[ix].property && item.optional === b[ix].optional,
1506
+ )
1507
);
1508
}
1506
-export type DependencyPath = Array<{property: string}>;
1509
+export type DependencyPath = Array<{property: string; optional: boolean}>;
1510
1511
/*
1512
* Simulated opaque type for BlockIds to prevent using normal numbers as block ids
compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts
+1
-1
@@ -869,7 +869,7 @@ export function printManualMemoDependency(
869
? val.root.value.identifier.name.value
870
: printIdentifier(val.root.value.identifier);
871
}
872
- return `${rootStr}${val.path.length > 0 ? '.' : ''}${val.path.join('.')}`;
872
+ return `${rootStr}${val.path.map(v => `${v.optional ? '?.' : '.'}${v.property}`).join('')}`;
873
}
874
export function printType(type: Type): string {
875
if (type.kind === 'Type') return '';
compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts
+2
-1
@@ -68,7 +68,8 @@ export function collectMaybeMemoDependencies(
68
if (object != null) {
69
return {
70
root: object.root,
71
- path: [...object.path, {property: value.property}],
71
+ // TODO: determine if the access is optional
72
+ path: [...object.path, {property: value.property, optional: false}],
73
};
74
}
75
break;
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/DeriveMinimalDependencies.ts
+6
-3
@@ -6,7 +6,7 @@
6
*/
7
8
import {CompilerError} from '../CompilerError';
9
-import {Identifier, ReactiveScopeDependency} from '../HIR';
9
+import {DependencyPath, Identifier, ReactiveScopeDependency} from '../HIR';
10
import {printIdentifier} from '../HIR/PrintHIR';
11
import {assertExhaustive} from '../Utils/utils';
12
@@ -252,7 +252,7 @@ type DependencyNode = {
252
};
253
254
type ReduceResultNode = {
255
- relativePath: Array<{property: string}>;
255
+ relativePath: DependencyPath;
256
accessType: PropertyAccessType;
257
};
258
@@ -283,7 +283,10 @@ function deriveMinimalDependenciesInSubtree(
283
const childResult = deriveMinimalDependenciesInSubtree(childNode).map(
284
({relativePath, accessType}) => {
285
return {
286
- relativePath: [{property: childName}, ...relativePath],
286
+ relativePath: [
287
+ {property: childName, optional: false},
288
+ ...relativePath,
289
+ ],
290
accessType,
291
};
292
},
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PropagateScopeDependencies.ts
+1
-1
@@ -485,7 +485,7 @@ class Context {
485
};
486
}
487
488
- objectDependency.path.push({property});
488
+ objectDependency.path.push({property, optional: false});
489
490
return objectDependency;
491
}
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts
+1
-1
@@ -116,7 +116,7 @@ function prettyPrintScopeDependency(val: ReactiveScopeDependency): string {
116
} else {
117
rootStr = '[unnamed]';
118
}
119
- return `${rootStr}${val.path.length > 0 ? '.' : ''}${val.path.join('.')}`;
119
+ return `${rootStr}${val.path.map(v => `${v.optional ? '?.' : '.'}${v.property}`).join('')}`;
120
}
121
122
enum CompareDependencyResult {
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-optional-member-expression-as-memo-dep.expect.md
new
+32
@@ -0,0 +1,32 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @validatePreserveExistingMemoizationGuarantees
6
+function Component(props) {
7
+ const data = useMemo(() => {
8
+ return props.items?.edges?.nodes ?? [];
9
+ }, [props.items?.edges?.nodes]);
10
+ return <Foo data={data} />;
11
+}
12
+
13
+```
14
+
15
+
16
+## Error
17
+
18
+```
19
+ 1 | // @validatePreserveExistingMemoizationGuarantees
20
+ 2 | function Component(props) {
21
+> 3 | const data = useMemo(() => {
22
+ | ^^^^^^^
23
+> 4 | return props.items?.edges?.nodes ?? [];
24
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
25
+> 5 | }, [props.items?.edges?.nodes]);
26
+ | ^^^^ 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)
27
+ 6 | return <Foo data={data} />;
28
+ 7 | }
29
+ 8 |
30
+```
31
+
32
+
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-optional-member-expression-as-memo-dep.js
new
+7
@@ -0,0 +1,7 @@
1
+// @validatePreserveExistingMemoizationGuarantees
2
+function Component(props) {
3
+ const data = useMemo(() => {
4
+ return props.items?.edges?.nodes ?? [];
5
+ }, [props.items?.edges?.nodes]);
6
+ return <Foo data={data} />;
7
+}