From 43ae8710ca773d9d56e86ce6fc139937063261f0 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 24 Jul 2025 15:38:22 -0700 Subject: [PATCH] [compiler] Update diagnostics for ValidatePreservedManualMemoization 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. --- .../ValidatePreservedManualMemoization.ts | 110 +++++++++++------- ...ession-with-conditional-optional.expect.md | 8 +- ...mber-expression-with-conditional.expect.md | 8 +- ...as-memo-dep-non-optional-in-body.expect.md | 8 +- .../error.ref-like-name-not-Ref.expect.md | 8 +- .../error.ref-like-name-not-a-ref.expect.md | 8 +- ...ed-function-inferred-as-mutation.expect.md | 8 +- ...from-inferred-mutation-in-logger.expect.md | 20 ++-- ...ack-captured-in-context-variable.expect.md | 8 +- .../dynamic-gating-bailout-nopanic.expect.md | 2 +- ...back-captures-reassigned-context.expect.md | 14 +-- ...ropped-infer-always-invalidating.expect.md | 8 +- ...sitive-useMemo-infer-mutate-deps.expect.md | 8 +- ...-positive-useMemo-overlap-scopes.expect.md | 8 +- ...ack-conditional-access-own-scope.expect.md | 8 +- ...ck-infer-conditional-value-block.expect.md | 16 +-- ...back-captures-reassigned-context.expect.md | 14 +-- ...nvalid-useCallback-read-maybeRef.expect.md | 8 +- ...be-invalid-useMemo-read-maybeRef.expect.md | 8 +- ...ve-use-memo-ref-missing-reactive.expect.md | 8 +- ...back-captures-invalidating-value.expect.md | 8 +- .../error.useCallback-aliased-var.expect.md | 8 +- ...lback-conditional-access-noAlloc.expect.md | 8 +- ...less-specific-conditional-access.expect.md | 8 +- ...or.useCallback-property-call-dep.expect.md | 8 +- .../error.useMemo-aliased-var.expect.md | 8 +- ...less-specific-conditional-access.expect.md | 8 +- ...specific-conditional-value-block.expect.md | 16 +-- ...emo-property-call-chained-object.expect.md | 8 +- .../error.useMemo-property-call-dep.expect.md | 8 +- ...o-unrelated-mutation-in-depslist.expect.md | 8 +- ...ession-with-conditional-optional.expect.md | 8 +- ...mber-expression-with-conditional.expect.md | 8 +- 33 files changed, 191 insertions(+), 209 deletions(-) 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 c150c1fbd7..6bb59247da 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts @@ -5,7 +5,11 @@ * LICENSE file in the root directory of this source tree. */ -import {CompilerError, ErrorSeverity} from '../CompilerError'; +import { + CompilerDiagnostic, + CompilerError, + ErrorSeverity, +} from '../CompilerError'; import { DeclarationId, Effect, @@ -275,27 +279,37 @@ function validateInferredDep( errorDiagnostic = merge(errorDiagnostic ?? compareResult, compareResult); } } - errorState.push({ - severity: ErrorSeverity.CannotPreserveMemoization, - reason: - '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', - description: - DEBUG || - // If the dependency is a named variable then we can report it. Otherwise only print in debug mode - (dep.identifier.name != null && dep.identifier.name.kind === 'named') - ? `The inferred dependency was \`${prettyPrintScopeDependency( - dep, - )}\`, but the source dependencies were [${validDepsInMemoBlock - .map(dep => printManualMemoDependency(dep, true)) - .join(', ')}]. ${ - errorDiagnostic - ? getCompareDependencyResultDescription(errorDiagnostic) - : 'Inferred dependency not present in source' - }` - : null, - loc: memoLocation, - suggestions: null, - }); + errorState.pushDiagnostic( + CompilerDiagnostic.create({ + severity: ErrorSeverity.CannotPreserveMemoization, + category: + 'Compilation skipped because existing memoization could not be preserved', + description: [ + '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. ', + DEBUG || + // If the dependency is a named variable then we can report it. Otherwise only print in debug mode + (dep.identifier.name != null && dep.identifier.name.kind === 'named') + ? `The inferred dependency was \`${prettyPrintScopeDependency( + dep, + )}\`, but the source dependencies were [${validDepsInMemoBlock + .map(dep => printManualMemoDependency(dep, true)) + .join(', ')}]. ${ + errorDiagnostic + ? getCompareDependencyResultDescription(errorDiagnostic) + : 'Inferred dependency not present in source' + }.` + : '', + ] + .join('') + .trim(), + suggestions: null, + }).withDetail({ + kind: 'error', + loc: memoLocation, + message: 'Could not preserve existing manual memoization', + }), + ); } class Visitor extends ReactiveFunctionVisitor { @@ -519,14 +533,21 @@ class Visitor extends ReactiveFunctionVisitor { !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 dependency may be mutated later, which could cause the value to change unexpectedly', - description: null, - severity: ErrorSeverity.CannotPreserveMemoization, - loc, - suggestions: null, - }); + state.errors.pushDiagnostic( + CompilerDiagnostic.create({ + severity: ErrorSeverity.CannotPreserveMemoization, + category: + 'Compilation skipped because existing memoization could not be preserved', + description: [ + '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.', + ].join(''), + }).withDetail({ + kind: 'error', + loc, + message: 'This dependency may be modified later', + }), + ); } } } @@ -560,16 +581,25 @@ class Visitor extends ReactiveFunctionVisitor { for (const identifier of decls) { 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: DEBUG - ? `${printIdentifier(identifier)} was not memoized` - : null, - severity: ErrorSeverity.CannotPreserveMemoization, - loc, - suggestions: null, - }); + state.errors.pushDiagnostic( + CompilerDiagnostic.create({ + severity: ErrorSeverity.CannotPreserveMemoization, + category: + 'Compilation skipped because existing memoization could not be preserved', + description: [ + '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. ', + DEBUG + ? `${printIdentifier(identifier)} was not memoized.` + : '', + ] + .join('') + .trim(), + }).withDetail({ + kind: 'error', + loc, + message: 'Could not preserve existing memoization', + }), + ); } } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional-optional.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional-optional.expect.md index dcde3a9f83..815a8fb208 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional-optional.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional-optional.expect.md @@ -25,9 +25,9 @@ function Component(props) { ``` Found 1 error: -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 +Memoization: Compilation skipped because existing memoization could not be preserved -The inferred dependency was `props.items`, but the source dependencies were [props?.items, props.cond]. Inferred different dependency than source. +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. error.hoist-optional-member-expression-with-conditional-optional.ts:4:23 2 | import {ValidateMemoization} from 'shared-runtime'; @@ -47,12 +47,10 @@ error.hoist-optional-member-expression-with-conditional-optional.ts:4:23 > 10 | return x; | ^^^^^^^^^^^^^^^^^ > 11 | }, [props?.items, props.cond]); - | ^^^^ 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 + | ^^^^ Could not preserve existing manual memoization 12 | return ( 13 | 14 | ); - - ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional.expect.md index ea6683fd0a..64065a7819 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional.expect.md @@ -25,9 +25,9 @@ function Component(props) { ``` Found 1 error: -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 +Memoization: Compilation skipped because existing memoization could not be preserved -The inferred dependency was `props.items`, but the source dependencies were [props?.items, props.cond]. Inferred different dependency than source. +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. error.hoist-optional-member-expression-with-conditional.ts:4:23 2 | import {ValidateMemoization} from 'shared-runtime'; @@ -47,12 +47,10 @@ error.hoist-optional-member-expression-with-conditional.ts:4:23 > 10 | return x; | ^^^^^^^^^^^^^^^^^ > 11 | }, [props?.items, props.cond]); - | ^^^^ 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 + | ^^^^ Could not preserve existing manual memoization 12 | return ( 13 | 14 | ); - - ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-optional-member-expression-as-memo-dep-non-optional-in-body.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-optional-member-expression-as-memo-dep-non-optional-in-body.expect.md index 196ce696b2..4a6cded7b8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-optional-member-expression-as-memo-dep-non-optional-in-body.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-optional-member-expression-as-memo-dep-non-optional-in-body.expect.md @@ -19,9 +19,9 @@ function Component(props) { ``` Found 1 error: -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 +Memoization: Compilation skipped because existing memoization could not be preserved -The inferred dependency was `props.items.edges.nodes`, but the source dependencies were [props.items?.edges?.nodes]. Inferred different dependency than source. +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. error.invalid-optional-member-expression-as-memo-dep-non-optional-in-body.ts:3:23 1 | // @validatePreserveExistingMemoizationGuarantees @@ -35,12 +35,10 @@ error.invalid-optional-member-expression-as-memo-dep-non-optional-in-body.ts:3:2 > 6 | // deps are optional | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > 7 | }, [props.items?.edges?.nodes]); - | ^^^^ 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 + | ^^^^ Could not preserve existing manual memoization 8 | return ; 9 | } 10 | - - ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-Ref.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-Ref.expect.md index a86193b84f..838e7a5b6d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-Ref.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-Ref.expect.md @@ -32,9 +32,9 @@ export const FIXTURE_ENTRYPOINT = { ``` Found 1 error: -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 +Memoization: Compilation skipped because existing memoization could not be preserved -The inferred dependency was `Ref.current`, but the source dependencies were []. Inferred dependency not present in source. +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. error.ref-like-name-not-Ref.ts:11:30 9 | const Ref = useCustomRef(); @@ -44,12 +44,10 @@ error.ref-like-name-not-Ref.ts:11:30 > 12 | Ref.current?.click(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ > 13 | }, []); - | ^^^^ 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 + | ^^^^ Could not preserve existing manual memoization 14 | 15 | return