From fa856cd5aade78fec8b6a59ae77d3ffc7a2b41aa Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 10 Jul 2025 19:02:34 -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 | 106 +++++++++++------- ...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, 187 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..15e1ef888f 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,35 @@ 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(''), + suggestions: null, + }).withDetail({ + kind: 'error', + loc: memoLocation, + message: 'Could not preserve existing manual memoization', + }), + ); } class Visitor extends ReactiveFunctionVisitor { @@ -519,14 +531,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 +579,23 @@ 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(''), + }).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..ff9e8c0956 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..5115077f7c 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..c8f429fed5 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..1d29f0bd8d 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