Skip to content

Commit 48bc166

Browse files
authored
[compiler] Update diagnostics for ValidatePreservedManualMemoization (facebook#33759)
Uses the new diagnostic infrastructure for this validation, which lets us provide a more targeted message on the text that we highlight (eg "This dependency may be mutated later") separately from the overall error message. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33759). * facebook#33981 * facebook#33777 * facebook#33767 * facebook#33765 * facebook#33760 * __->__ facebook#33759 * facebook#33758
1 parent 7284802 commit 48bc166

File tree

33 files changed

+191
-209
lines changed

33 files changed

+191
-209
lines changed

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

Lines changed: 70 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@
55
* LICENSE file in the root directory of this source tree.
66
*/
77

8-
import {CompilerError, ErrorSeverity} from '../CompilerError';
8+
import {
9+
CompilerDiagnostic,
10+
CompilerError,
11+
ErrorSeverity,
12+
} from '../CompilerError';
913
import {
1014
DeclarationId,
1115
Effect,
@@ -275,27 +279,37 @@ function validateInferredDep(
275279
errorDiagnostic = merge(errorDiagnostic ?? compareResult, compareResult);
276280
}
277281
}
278-
errorState.push({
279-
severity: ErrorSeverity.CannotPreserveMemoization,
280-
reason:
281-
'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:
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,
296-
loc: memoLocation,
297-
suggestions: null,
298-
});
282+
errorState.pushDiagnostic(
283+
CompilerDiagnostic.create({
284+
severity: ErrorSeverity.CannotPreserveMemoization,
285+
category:
286+
'Compilation skipped because existing memoization could not be preserved',
287+
description: [
288+
'React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. ',
289+
'The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected. ',
290+
DEBUG ||
291+
// If the dependency is a named variable then we can report it. Otherwise only print in debug mode
292+
(dep.identifier.name != null && dep.identifier.name.kind === 'named')
293+
? `The inferred dependency was \`${prettyPrintScopeDependency(
294+
dep,
295+
)}\`, but the source dependencies were [${validDepsInMemoBlock
296+
.map(dep => printManualMemoDependency(dep, true))
297+
.join(', ')}]. ${
298+
errorDiagnostic
299+
? getCompareDependencyResultDescription(errorDiagnostic)
300+
: 'Inferred dependency not present in source'
301+
}.`
302+
: '',
303+
]
304+
.join('')
305+
.trim(),
306+
suggestions: null,
307+
}).withDetail({
308+
kind: 'error',
309+
loc: memoLocation,
310+
message: 'Could not preserve existing manual memoization',
311+
}),
312+
);
299313
}
300314

301315
class Visitor extends ReactiveFunctionVisitor<VisitorState> {
@@ -519,14 +533,21 @@ class Visitor extends ReactiveFunctionVisitor<VisitorState> {
519533
!this.scopes.has(identifier.scope.id) &&
520534
!this.prunedScopes.has(identifier.scope.id)
521535
) {
522-
state.errors.push({
523-
reason:
524-
'React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This dependency may be mutated later, which could cause the value to change unexpectedly',
525-
description: null,
526-
severity: ErrorSeverity.CannotPreserveMemoization,
527-
loc,
528-
suggestions: null,
529-
});
536+
state.errors.pushDiagnostic(
537+
CompilerDiagnostic.create({
538+
severity: ErrorSeverity.CannotPreserveMemoization,
539+
category:
540+
'Compilation skipped because existing memoization could not be preserved',
541+
description: [
542+
'React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. ',
543+
'This dependency may be mutated later, which could cause the value to change unexpectedly.',
544+
].join(''),
545+
}).withDetail({
546+
kind: 'error',
547+
loc,
548+
message: 'This dependency may be modified later',
549+
}),
550+
);
530551
}
531552
}
532553
}
@@ -560,16 +581,25 @@ class Visitor extends ReactiveFunctionVisitor<VisitorState> {
560581

561582
for (const identifier of decls) {
562583
if (isUnmemoized(identifier, this.scopes)) {
563-
state.errors.push({
564-
reason:
565-
'React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value was memoized in source but not in compilation output.',
566-
description: DEBUG
567-
? `${printIdentifier(identifier)} was not memoized`
568-
: null,
569-
severity: ErrorSeverity.CannotPreserveMemoization,
570-
loc,
571-
suggestions: null,
572-
});
584+
state.errors.pushDiagnostic(
585+
CompilerDiagnostic.create({
586+
severity: ErrorSeverity.CannotPreserveMemoization,
587+
category:
588+
'Compilation skipped because existing memoization could not be preserved',
589+
description: [
590+
'React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value was memoized in source but not in compilation output. ',
591+
DEBUG
592+
? `${printIdentifier(identifier)} was not memoized.`
593+
: '',
594+
]
595+
.join('')
596+
.trim(),
597+
}).withDetail({
598+
kind: 'error',
599+
loc,
600+
message: 'Could not preserve existing memoization',
601+
}),
602+
);
573603
}
574604
}
575605
}

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ function Component(props) {
2525
2626
```
2727
Found 1 error:
28-
Memoization: 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
28+
Memoization: Compilation skipped because existing memoization could not be preserved
2929

30-
The inferred dependency was `props.items`, but the source dependencies were [props?.items, props.cond]. Inferred different dependency than source.
30+
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.
3131

3232
error.hoist-optional-member-expression-with-conditional-optional.ts:4:23
3333
2 | import {ValidateMemoization} from 'shared-runtime';
@@ -47,12 +47,10 @@ error.hoist-optional-member-expression-with-conditional-optional.ts:4:23
4747
> 10 | return x;
4848
| ^^^^^^^^^^^^^^^^^
4949
> 11 | }, [props?.items, props.cond]);
50-
| ^^^^ 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
50+
| ^^^^ Could not preserve existing manual memoization
5151
12 | return (
5252
13 | <ValidateMemoization inputs={[props?.items, props.cond]} output={data} />
5353
14 | );
54-
55-
5654
```
5755
5856

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ function Component(props) {
2525
2626
```
2727
Found 1 error:
28-
Memoization: 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
28+
Memoization: Compilation skipped because existing memoization could not be preserved
2929

30-
The inferred dependency was `props.items`, but the source dependencies were [props?.items, props.cond]. Inferred different dependency than source.
30+
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.
3131

3232
error.hoist-optional-member-expression-with-conditional.ts:4:23
3333
2 | import {ValidateMemoization} from 'shared-runtime';
@@ -47,12 +47,10 @@ error.hoist-optional-member-expression-with-conditional.ts:4:23
4747
> 10 | return x;
4848
| ^^^^^^^^^^^^^^^^^
4949
> 11 | }, [props?.items, props.cond]);
50-
| ^^^^ 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
50+
| ^^^^ Could not preserve existing manual memoization
5151
12 | return (
5252
13 | <ValidateMemoization inputs={[props?.items, props.cond]} output={data} />
5353
14 | );
54-
55-
5654
```
5755
5856

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: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@ function Component(props) {
1919
2020
```
2121
Found 1 error:
22-
Memoization: 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
22+
Memoization: Compilation skipped because existing memoization could not be preserved
2323

24-
The inferred dependency was `props.items.edges.nodes`, but the source dependencies were [props.items?.edges?.nodes]. Inferred different dependency than source.
24+
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.
2525

2626
error.invalid-optional-member-expression-as-memo-dep-non-optional-in-body.ts:3:23
2727
1 | // @validatePreserveExistingMemoizationGuarantees
@@ -35,12 +35,10 @@ error.invalid-optional-member-expression-as-memo-dep-non-optional-in-body.ts:3:2
3535
> 6 | // deps are optional
3636
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3737
> 7 | }, [props.items?.edges?.nodes]);
38-
| ^^^^ 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
38+
| ^^^^ Could not preserve existing manual memoization
3939
8 | return <Foo data={data} />;
4040
9 | }
4141
10 |
42-
43-
4442
```
4543
4644

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ export const FIXTURE_ENTRYPOINT = {
3232
3333
```
3434
Found 1 error:
35-
Memoization: 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
35+
Memoization: Compilation skipped because existing memoization could not be preserved
3636

37-
The inferred dependency was `Ref.current`, but the source dependencies were []. Inferred dependency not present in source.
37+
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.
3838

3939
error.ref-like-name-not-Ref.ts:11:30
4040
9 | const Ref = useCustomRef();
@@ -44,12 +44,10 @@ error.ref-like-name-not-Ref.ts:11:30
4444
> 12 | Ref.current?.click();
4545
| ^^^^^^^^^^^^^^^^^^^^^^^^^
4646
> 13 | }, []);
47-
| ^^^^ 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
47+
| ^^^^ Could not preserve existing manual memoization
4848
14 |
4949
15 | return <button onClick={onClick} />;
5050
16 | }
51-
52-
5351
```
5452
5553

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ export const FIXTURE_ENTRYPOINT = {
3232
3333
```
3434
Found 1 error:
35-
Memoization: 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
35+
Memoization: Compilation skipped because existing memoization could not be preserved
3636

37-
The inferred dependency was `notaref.current`, but the source dependencies were []. Inferred dependency not present in source.
37+
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.
3838

3939
error.ref-like-name-not-a-ref.ts:11:30
4040
9 | const notaref = useCustomRef();
@@ -44,12 +44,10 @@ error.ref-like-name-not-a-ref.ts:11:30
4444
> 12 | notaref.current?.click();
4545
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4646
> 13 | }, []);
47-
| ^^^^ 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
47+
| ^^^^ Could not preserve existing manual memoization
4848
14 |
4949
15 | return <button onClick={onClick} />;
5050
16 | }
51-
52-
5351
```
5452
5553

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-repro-missed-memoization-from-capture-in-invoked-function-inferred-as-mutation.expect.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,18 +43,18 @@ component Component() {
4343

4444
```
4545
Found 1 error:
46-
Memoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value was memoized in source but not in compilation output.
46+
Memoization: Compilation skipped because existing memoization could not be preserved
47+
48+
React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value was memoized in source but not in compilation output.
4749
4850
undefined:18:20
4951
16 | // We infer that getIsEnabled returns a mutable value, such that
5052
17 | // isEnabled is mutable
5153
> 18 | const isEnabled = useMemo(() => getIsEnabled(), [getIsEnabled]);
52-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value was memoized in source but not in compilation output.
54+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Could not preserve existing memoization
5355
19 |
5456
20 | // We then infer getLoggingData as capturing that mutable value,
5557
21 | // so any calls to this function are then inferred as extending
56-
57-
5858
```
5959
6060

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-repro-missed-memoization-from-inferred-mutation-in-logger.expect.md

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,9 @@ component Component(id) {
5353

5454
```
5555
Found 3 errors:
56-
Memoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value was memoized in source but not in compilation output.
56+
Memoization: Compilation skipped because existing memoization could not be preserved
57+
58+
React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value was memoized in source but not in compilation output.
5759
5860
undefined:11:18
5961
9 | const [index, setIndex] = useState(0);
@@ -69,25 +71,25 @@ undefined:11:18
6971
> 15 | };
7072
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
7173
> 16 | }, [index, items]);
72-
| ^^^^^^^^^^^^^^^^^^^^^ React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value was memoized in source but not in compilation output.
74+
| ^^^^^^^^^^^^^^^^^^^^^ Could not preserve existing memoization
7375
17 |
7476
18 | const setCurrentIndex = useCallback(
7577
19 | (index: number) => {
78+
Memoization: Compilation skipped because existing memoization could not be preserved
7679
77-
78-
Memoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This dependency may be mutated later, which could cause the value to change unexpectedly
80+
React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This dependency may be mutated later, which could cause the value to change unexpectedly.
7981
8082
undefined:28:12
8183
26 | setIndex(index);
8284
27 | },
8385
> 28 | [index, logData, items]
84-
| ^^^^^^^ React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This dependency may be mutated later, which could cause the value to change unexpectedly
86+
| ^^^^^^^ This dependency may be modified later
8587
29 | );
8688
30 |
8789
31 | if (prevId !== id) {
90+
Memoization: Compilation skipped because existing memoization could not be preserved
8891
89-
90-
Memoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value was memoized in source but not in compilation output.
92+
React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value was memoized in source but not in compilation output.
9193
9294
undefined:19:4
9395
17 |
@@ -109,12 +111,10 @@ undefined:19:4
109111
> 26 | setIndex(index);
110112
| ^^^^^^^^^^^^^^^^^^^^^^
111113
> 27 | },
112-
| ^^^^^^ React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value was memoized in source but not in compilation output.
114+
| ^^^^^^ Could not preserve existing memoization
113115
28 | [index, logData, items]
114116
29 | );
115117
30 |
116-
117-
118118
```
119119
120120

0 commit comments

Comments
 (0)