Skip to content

Commit 2e648a5

Browse files
committed
[compiler] ValidatePreservedManualMemoization reports detailed errors
This pass didn't previously report the precise difference btw inferred/manual dependencies unless a debug flag was set. But the error message is really good (nice job mofeiz): the only catch is that in theory the inferred dep could be a temporary that can't trivially be reported to the user. But the messages are really useful for quickly verifying why the compiler couldn't preserve memoization. So here we switch to outputting a detailed message about the discrepancy btw inferred/manual deps so long as the inferred dep root is a named variable. I also slightly adjusted the message to handle the case where there is no diagnostic, which can occur if there were no manual deps but the compiler inferred a dependency. ghstack-source-id: 534f6f1 Pull Request resolved: #33095
1 parent e9db3cc commit 2e648a5

File tree

23 files changed

+42
-39
lines changed

23 files changed

+42
-39
lines changed

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -141,14 +141,14 @@ function getCompareDependencyResultDescription(
141141
): string {
142142
switch (result) {
143143
case CompareDependencyResult.Ok:
144-
return 'dependencies equal';
144+
return 'Dependencies equal';
145145
case CompareDependencyResult.RootDifference:
146146
case CompareDependencyResult.PathDifference:
147-
return 'inferred different dependency than source';
147+
return 'Inferred different dependency than source';
148148
case CompareDependencyResult.RefAccessDifference:
149-
return 'differences in ref.current access';
149+
return 'Differences in ref.current access';
150150
case CompareDependencyResult.Subpath:
151-
return 'inferred less specific property than source';
151+
return 'Inferred less specific property than source';
152152
}
153153
}
154154

@@ -279,17 +279,20 @@ function validateInferredDep(
279279
severity: ErrorSeverity.CannotPreserveMemoization,
280280
reason:
281281
'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',
282-
description: DEBUG
283-
? `The inferred dependency was \`${prettyPrintScopeDependency(
284-
dep,
285-
)}\`, but the source dependencies were [${validDepsInMemoBlock
286-
.map(dep => printManualMemoDependency(dep, true))
287-
.join(', ')}]. Detail: ${
288-
errorDiagnostic
289-
? getCompareDependencyResultDescription(errorDiagnostic)
290-
: 'none'
291-
}`
292-
: null,
282+
description:
283+
DEBUG ||
284+
// If the dependency is a named variable then we can report it. Otherwise only print in debug mode
285+
(dep.identifier.name != null && dep.identifier.name.kind === 'named')
286+
? `The inferred dependency was \`${prettyPrintScopeDependency(
287+
dep,
288+
)}\`, but the source dependencies were [${validDepsInMemoBlock
289+
.map(dep => printManualMemoDependency(dep, true))
290+
.join(', ')}]. ${
291+
errorDiagnostic
292+
? getCompareDependencyResultDescription(errorDiagnostic)
293+
: 'Inferred dependency not present in source'
294+
}`
295+
: null,
293296
loc: memoLocation,
294297
suggestions: null,
295298
});

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional-optional.expect.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ function Component(props) {
4141
> 10 | return x;
4242
| ^^^^^^^^^^^^^^^^^
4343
> 11 | }, [props?.items, props.cond]);
44-
| ^^^^ 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 (4:11)
44+
| ^^^^ 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. The inferred dependency was `props.items`, but the source dependencies were [props?.items, props.cond]. Inferred different dependency than source (4:11)
4545
12 | return (
4646
13 | <ValidateMemoization inputs={[props?.items, props.cond]} output={data} />
4747
14 | );

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional.expect.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ function Component(props) {
4141
> 10 | return x;
4242
| ^^^^^^^^^^^^^^^^^
4343
> 11 | }, [props?.items, props.cond]);
44-
| ^^^^ 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 (4:11)
44+
| ^^^^ 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. The inferred dependency was `props.items`, but the source dependencies were [props?.items, props.cond]. Inferred different dependency than source (4:11)
4545
12 | return (
4646
13 | <ValidateMemoization inputs={[props?.items, props.cond]} output={data} />
4747
14 | );

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-optional-member-expression-as-memo-dep-non-optional-in-body.expect.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ function Component(props) {
2929
> 6 | // deps are optional
3030
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3131
> 7 | }, [props.items?.edges?.nodes]);
32-
| ^^^^ 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:7)
32+
| ^^^^ 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. The inferred dependency was `props.items.edges.nodes`, but the source dependencies were [props.items?.edges?.nodes]. Inferred different dependency than source (3:7)
3333
8 | return <Foo data={data} />;
3434
9 | }
3535
10 |

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-Ref.expect.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export const FIXTURE_ENTRYPOINT = {
3838
> 12 | Ref.current?.click();
3939
| ^^^^^^^^^^^^^^^^^^^^^^^^^
4040
> 13 | }, []);
41-
| ^^^^ 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 (11:13)
41+
| ^^^^ 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. The inferred dependency was `Ref.current`, but the source dependencies were []. Inferred dependency not present in source (11:13)
4242
14 |
4343
15 | return <button onClick={onClick} />;
4444
16 | }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-a-ref.expect.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export const FIXTURE_ENTRYPOINT = {
3838
> 12 | notaref.current?.click();
3939
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4040
> 13 | }, []);
41-
| ^^^^ 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 (11:13)
41+
| ^^^^ 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. The inferred dependency was `notaref.current`, but the source dependencies were []. Inferred dependency not present in source (11:13)
4242
14 |
4343
15 | return <button onClick={onClick} />;
4444
16 | }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.hoist-useCallback-conditional-access-own-scope.expect.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export const FIXTURE_ENTRYPOINT = {
4141
> 10 | }
4242
| ^^^^^^^^^^^^^^^^
4343
> 11 | }, [propA, propB.x.y]);
44-
| ^^^^ 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 (5:11)
44+
| ^^^^ 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. The inferred dependency was `propB`, but the source dependencies were [propA, propB.x.y]. Inferred less specific property than source (5:11)
4545
12 | }
4646
13 |
4747
14 | export const FIXTURE_ENTRYPOINT = {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.hoist-useCallback-infer-conditional-value-block.expect.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,9 @@ export const FIXTURE_ENTRYPOINT = {
4848
> 13 | }
4949
| ^^^^^^^^^^^^^^^^^
5050
> 14 | }, [propA.a, propB.x.y]);
51-
| ^^^^ 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)
51+
| ^^^^ 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. The inferred dependency was `propA`, but the source dependencies were [propA.a, propB.x.y]. Inferred less specific property than source (6:14)
5252

53-
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)
53+
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. The inferred dependency was `propB`, but the source dependencies were [propA.a, propB.x.y]. Inferred less specific property than source (6:14)
5454
15 | }
5555
16 |
5656
17 | export const FIXTURE_ENTRYPOINT = {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.maybe-invalid-useCallback-read-maybeRef.expect.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ function useHook(maybeRef) {
2424
> 6 | return [maybeRef.current];
2525
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2626
> 7 | }, [maybeRef]);
27-
| ^^^^ 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 (5:7)
27+
| ^^^^ 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. The inferred dependency was `maybeRef.current`, but the source dependencies were [maybeRef]. Differences in ref.current access (5:7)
2828
8 | }
2929
9 |
3030
```

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.maybe-invalid-useMemo-read-maybeRef.expect.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ function useHook(maybeRef, shouldRead) {
2424
> 6 | return () => [maybeRef.current];
2525
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2626
> 7 | }, [shouldRead, maybeRef]);
27-
| ^^^^ 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 (5:7)
27+
| ^^^^ 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. The inferred dependency was `maybeRef.current`, but the source dependencies were [shouldRead, maybeRef]. Differences in ref.current access (5:7)
2828
8 | }
2929
9 |
3030
```

0 commit comments

Comments
 (0)