Skip to content

Commit e662b0a

Browse files
committed
[compiler][be] Less ambiguous error messages for validateMemo bailout
ghstack-source-id: 312093e Pull Request resolved: #30601
1 parent e948a5a commit e662b0a

7 files changed

+52
-46
lines changed

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

Lines changed: 43 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -433,64 +433,74 @@ class Visitor extends ReactiveFunctionVisitor<VisitorState> {
433433
* recursively visits ReactiveValues and instructions
434434
*/
435435
this.recordTemporaries(instruction, state);
436-
if (instruction.value.kind === 'StartMemoize') {
436+
const value = instruction.value;
437+
if (value.kind === 'StartMemoize') {
437438
let depsFromSource: Array<ManualMemoDependency> | null = null;
438-
if (instruction.value.deps != null) {
439-
depsFromSource = instruction.value.deps;
439+
if (value.deps != null) {
440+
depsFromSource = value.deps;
440441
}
441442
CompilerError.invariant(state.manualMemoState == null, {
442443
reason: 'Unexpected nested StartMemoize instructions',
443-
description: `Bad manual memoization ids: ${state.manualMemoState?.manualMemoId}, ${instruction.value.manualMemoId}`,
444-
loc: instruction.value.loc,
444+
description: `Bad manual memoization ids: ${state.manualMemoState?.manualMemoId}, ${value.manualMemoId}`,
445+
loc: value.loc,
445446
suggestions: null,
446447
});
447448

448449
state.manualMemoState = {
449450
loc: instruction.loc,
450451
decls: new Set(),
451452
depsFromSource,
452-
manualMemoId: instruction.value.manualMemoId,
453+
manualMemoId: value.manualMemoId,
453454
};
454-
}
455-
if (instruction.value.kind === 'FinishMemoize') {
456-
CompilerError.invariant(
457-
state.manualMemoState != null &&
458-
state.manualMemoState.manualMemoId === instruction.value.manualMemoId,
459-
{
460-
reason: 'Unexpected mismatch between StartMemoize and FinishMemoize',
461-
description: `Encountered StartMemoize id=${state.manualMemoState?.manualMemoId} followed by FinishMemoize id=${instruction.value.manualMemoId}`,
462-
loc: instruction.value.loc,
463-
suggestions: null,
464-
},
465-
);
466-
state.manualMemoState = null;
467-
}
468455

469-
const isDep = instruction.value.kind === 'StartMemoize';
470-
const isDecl =
471-
instruction.value.kind === 'FinishMemoize' && !instruction.value.pruned;
472-
if (isDep || isDecl) {
473-
for (const value of eachInstructionValueOperand(
474-
instruction.value as InstructionValue,
456+
for (const {identifier, loc} of eachInstructionValueOperand(
457+
value as InstructionValue,
475458
)) {
476459
if (
477-
(isDep &&
478-
value.identifier.scope != null &&
479-
!this.scopes.has(value.identifier.scope.id) &&
480-
!this.prunedScopes.has(value.identifier.scope.id)) ||
481-
(isDecl && isUnmemoized(value.identifier, this.scopes))
460+
identifier.scope != null &&
461+
!this.scopes.has(identifier.scope.id) &&
462+
!this.prunedScopes.has(identifier.scope.id)
482463
) {
483464
state.errors.push({
484465
reason:
485-
'React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value may be mutated later, which could cause the value to change unexpectedly',
466+
'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',
486467
description: null,
487468
severity: ErrorSeverity.CannotPreserveMemoization,
488-
loc: typeof instruction.loc !== 'symbol' ? instruction.loc : null,
469+
loc,
489470
suggestions: null,
490471
});
491472
}
492473
}
493474
}
475+
if (value.kind === 'FinishMemoize') {
476+
CompilerError.invariant(
477+
state.manualMemoState != null &&
478+
state.manualMemoState.manualMemoId === value.manualMemoId,
479+
{
480+
reason: 'Unexpected mismatch between StartMemoize and FinishMemoize',
481+
description: `Encountered StartMemoize id=${state.manualMemoState?.manualMemoId} followed by FinishMemoize id=${value.manualMemoId}`,
482+
loc: value.loc,
483+
suggestions: null,
484+
},
485+
);
486+
state.manualMemoState = null;
487+
if (!value.pruned) {
488+
for (const {identifier, loc} of eachInstructionValueOperand(
489+
value as InstructionValue,
490+
)) {
491+
if (isUnmemoized(identifier, this.scopes)) {
492+
state.errors.push({
493+
reason:
494+
'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.',
495+
description: null,
496+
severity: ErrorSeverity.CannotPreserveMemoization,
497+
loc,
498+
suggestions: null,
499+
});
500+
}
501+
}
502+
}
503+
}
494504
}
495505
}
496506

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-repro-unmemoized-callback-captured-in-context-variable.expect.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ export const FIXTURE_ENTRYPOINT = {
5353
9 | const a = useHook();
5454
10 | // Because b is also part of that same mutable range, it can't be memoized either
5555
> 11 | const b = useMemo(() => ({}), []);
56-
| ^^^^^^^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value may be mutated later, which could cause the value to change unexpectedly (11:11)
56+
| ^^^^^^^^^^^^^^^^^^^^^^^ CannotPreserveMemoization: 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. (11:11)
5757
12 |
5858
13 | // Conditional assignment without a subsequent mutation normally doesn't create a mutable
5959
14 | // range, but in this case we're reassigning a context variable

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ export const FIXTURE_ENTRYPOINT = {
4545
> 10 | ref.current.inner = event.target.value;
4646
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4747
> 11 | });
48-
| ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value may be mutated later, which could cause the value to change unexpectedly (7:11)
48+
| ^^^^ CannotPreserveMemoization: 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. (7:11)
4949
12 |
5050
13 | // The ref is modified later, extending its range and preventing memoization of onChange
5151
14 | const reset = () => {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ export const FIXTURE_ENTRYPOINT = {
4242
> 10 | ref.current.inner = event.target.value;
4343
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4444
> 11 | });
45-
| ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value may be mutated later, which could cause the value to change unexpectedly (7:11)
45+
| ^^^^ CannotPreserveMemoization: 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. (7:11)
4646
12 |
4747
13 | // The ref is modified later, extending its range and preventing memoization of onChange
4848
14 | ref.current.inner = null;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useMemo-infer-mutate-deps.expect.md

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,10 @@ export const FIXTURE_ENTRYPOINT = {
2929
## Error
3030

3131
```
32-
10 | const val = [1, 2, 3];
33-
11 |
34-
> 12 | return useMemo(() => {
35-
| ^^^^^^^
36-
> 13 | return identity(val);
37-
| ^^^^^^^^^^^^^^^^^^^^^^^^^
32+
12 | return useMemo(() => {
33+
13 | return identity(val);
3834
> 14 | }, [val]);
39-
| ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value may be mutated later, which could cause the value to change unexpectedly (12:14)
35+
| ^^^ CannotPreserveMemoization: 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 (14:14)
4036
15 | }
4137
16 |
4238
17 | export const FIXTURE_ENTRYPOINT = {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.invalid-useCallback-captures-reassigned-context.expect.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ export const FIXTURE_ENTRYPOINT = {
3333
10 |
3434
11 | // makeArray() is captured, but depsList contains [props]
3535
> 12 | const cb = useCallback(() => [x], [x]);
36-
| ^^^^^^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value may be mutated later, which could cause the value to change unexpectedly (12:12)
36+
| ^ CannotPreserveMemoization: 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 (12:12)
3737
38-
CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value may be mutated later, which could cause the value to change unexpectedly (12:12)
38+
CannotPreserveMemoization: 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. (12:12)
3939
13 |
4040
14 | x = makeArray();
4141
15 |

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-invalidating-value.expect.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ export const FIXTURE_ENTRYPOINT = {
3131
11 | x.push(props);
3232
12 |
3333
> 13 | return useCallback(() => [x], [x]);
34-
| ^^^^^^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value may be mutated later, which could cause the value to change unexpectedly (13:13)
34+
| ^^^^^^^^^ CannotPreserveMemoization: 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. (13:13)
3535
14 | }
3636
15 |
3737
16 | export const FIXTURE_ENTRYPOINT = {

0 commit comments

Comments
 (0)