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 f0009601f1d..20448607090 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts @@ -433,15 +433,16 @@ class Visitor extends ReactiveFunctionVisitor { * recursively visits ReactiveValues and instructions */ this.recordTemporaries(instruction, state); - if (instruction.value.kind === 'StartMemoize') { + const value = instruction.value; + if (value.kind === 'StartMemoize') { let depsFromSource: Array | null = null; - if (instruction.value.deps != null) { - depsFromSource = instruction.value.deps; + if (value.deps != null) { + depsFromSource = value.deps; } CompilerError.invariant(state.manualMemoState == null, { reason: 'Unexpected nested StartMemoize instructions', - description: `Bad manual memoization ids: ${state.manualMemoState?.manualMemoId}, ${instruction.value.manualMemoId}`, - loc: instruction.value.loc, + description: `Bad manual memoization ids: ${state.manualMemoState?.manualMemoId}, ${value.manualMemoId}`, + loc: value.loc, suggestions: null, }); @@ -449,48 +450,57 @@ class Visitor extends ReactiveFunctionVisitor { loc: instruction.loc, decls: new Set(), depsFromSource, - manualMemoId: instruction.value.manualMemoId, + manualMemoId: value.manualMemoId, }; - } - if (instruction.value.kind === 'FinishMemoize') { - CompilerError.invariant( - state.manualMemoState != null && - state.manualMemoState.manualMemoId === instruction.value.manualMemoId, - { - reason: 'Unexpected mismatch between StartMemoize and FinishMemoize', - description: `Encountered StartMemoize id=${state.manualMemoState?.manualMemoId} followed by FinishMemoize id=${instruction.value.manualMemoId}`, - loc: instruction.value.loc, - suggestions: null, - }, - ); - state.manualMemoState = null; - } - const isDep = instruction.value.kind === 'StartMemoize'; - const isDecl = - instruction.value.kind === 'FinishMemoize' && !instruction.value.pruned; - if (isDep || isDecl) { - for (const value of eachInstructionValueOperand( - instruction.value as InstructionValue, + for (const {identifier, loc} of eachInstructionValueOperand( + value as InstructionValue, )) { if ( - (isDep && - value.identifier.scope != null && - !this.scopes.has(value.identifier.scope.id) && - !this.prunedScopes.has(value.identifier.scope.id)) || - (isDecl && isUnmemoized(value.identifier, this.scopes)) + identifier.scope != null && + !this.scopes.has(identifier.scope.id) && + !this.prunedScopes.has(identifier.scope.id) ) { state.errors.push({ reason: - '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', + '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', description: null, severity: ErrorSeverity.CannotPreserveMemoization, - loc: typeof instruction.loc !== 'symbol' ? instruction.loc : null, + loc, suggestions: null, }); } } } + if (value.kind === 'FinishMemoize') { + CompilerError.invariant( + state.manualMemoState != null && + state.manualMemoState.manualMemoId === value.manualMemoId, + { + reason: 'Unexpected mismatch between StartMemoize and FinishMemoize', + description: `Encountered StartMemoize id=${state.manualMemoState?.manualMemoId} followed by FinishMemoize id=${value.manualMemoId}`, + loc: value.loc, + suggestions: null, + }, + ); + state.manualMemoState = null; + if (!value.pruned) { + for (const {identifier, loc} of eachInstructionValueOperand( + value as InstructionValue, + )) { + if (isUnmemoized(identifier, this.scopes)) { + state.errors.push({ + reason: + '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.', + description: null, + severity: ErrorSeverity.CannotPreserveMemoization, + loc, + suggestions: null, + }); + } + } + } + } } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-repro-unmemoized-callback-captured-in-context-variable.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-repro-unmemoized-callback-captured-in-context-variable.expect.md index 8979a7642ff..30eb3ee27e6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-repro-unmemoized-callback-captured-in-context-variable.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-repro-unmemoized-callback-captured-in-context-variable.expect.md @@ -53,7 +53,7 @@ export const FIXTURE_ENTRYPOINT = { 9 | const a = useHook(); 10 | // Because b is also part of that same mutable range, it can't be memoized either > 11 | const b = useMemo(() => ({}), []); - | ^^^^^^^^^^ 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) + | ^^^^^^^^^^^^^^^^^^^^^^^ 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) 12 | 13 | // Conditional assignment without a subsequent mutation normally doesn't create a mutable 14 | // range, but in this case we're reassigning a context variable diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md index 813c5257667..ad21b4ba8c5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md @@ -45,7 +45,7 @@ export const FIXTURE_ENTRYPOINT = { > 10 | ref.current.inner = event.target.value; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > 11 | }); - | ^^^^ 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) + | ^^^^ 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) 12 | 13 | // The ref is modified later, extending its range and preventing memoization of onChange 14 | const reset = () => { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md index c92fd6c0f0d..5eeb8c6b0ab 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md @@ -42,7 +42,7 @@ export const FIXTURE_ENTRYPOINT = { > 10 | ref.current.inner = event.target.value; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > 11 | }); - | ^^^^ 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) + | ^^^^ 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) 12 | 13 | // The ref is modified later, extending its range and preventing memoization of onChange 14 | ref.current.inner = null; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useMemo-infer-mutate-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useMemo-infer-mutate-deps.expect.md index 1ac98d9c309..564796f18d4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useMemo-infer-mutate-deps.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useMemo-infer-mutate-deps.expect.md @@ -29,14 +29,10 @@ export const FIXTURE_ENTRYPOINT = { ## Error ``` - 10 | const val = [1, 2, 3]; - 11 | -> 12 | return useMemo(() => { - | ^^^^^^^ -> 13 | return identity(val); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ + 12 | return useMemo(() => { + 13 | return identity(val); > 14 | }, [val]); - | ^^^^ 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) + | ^^^ 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) 15 | } 16 | 17 | export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.invalid-useCallback-captures-reassigned-context.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.invalid-useCallback-captures-reassigned-context.expect.md index 073a265e7e0..cfa2999835f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.invalid-useCallback-captures-reassigned-context.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.invalid-useCallback-captures-reassigned-context.expect.md @@ -33,9 +33,9 @@ export const FIXTURE_ENTRYPOINT = { 10 | 11 | // makeArray() is captured, but depsList contains [props] > 12 | const cb = useCallback(() => [x], [x]); - | ^^^^^^^^^ 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) + | ^ 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) -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) +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) 13 | 14 | x = makeArray(); 15 | diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-invalidating-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-invalidating-value.expect.md index fef849d3413..1f26e03b836 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-invalidating-value.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-invalidating-value.expect.md @@ -31,7 +31,7 @@ export const FIXTURE_ENTRYPOINT = { 11 | x.push(props); 12 | > 13 | return useCallback(() => [x], [x]); - | ^^^^^^^^^ 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) + | ^^^^^^^^^ 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) 14 | } 15 | 16 | export const FIXTURE_ENTRYPOINT = {