[compiler] Deduplicate errors between ValidateExhaustiveDependencies and ValidatePreservedManualMemoization (facebook#35917)

With the recent changes to make the compiler fault tolerant and always
continue through all passes, we can now sometimes report duplicative
errors. Specifically, when `ValidateExhaustiveDependencies` finds
incorrect deps for a useMemo/useCallback call,
`ValidatePreservedManualMemoization` will generally also error for the
same block, producing duplicate errors. The exhaustive deps error is
strictly more informative, so if we've already reported the earlier
error we don't need the later one.

This adds a `hasInvalidDeps` flag to StartMemoize that is set when
ValidateExhaustiveDependencies produces a diagnostic.
ValidatePreservedManualMemoization then skips validation for memo blocks
with this flag set.

Author: josephsavona

compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts

@@ -826,6 +826,7 @@ export type StartMemoize = {
    * emitting diagnostics with a suggested replacement
    */
   depsLoc: SourceLocation | null;
+  hasInvalidDeps?: true;
   loc: SourceLocation;
 };
 export type FinishMemoize = {

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

@@ -143,6 +143,7 @@ export function validateExhaustiveDependencies(fn: HIRFunction): void {
       );
       if (diagnostic != null) {
         fn.env.recordError(diagnostic);
+        startMemo.hasInvalidDeps = true;
       }
     }
 

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

@@ -486,16 +486,25 @@ class Visitor extends ReactiveFunctionVisitor<VisitorState> {
       ids.add(value.place.identifier);
     }
     if (value.kind === 'StartMemoize') {
-      let depsFromSource: Array<ManualMemoDependency> | null = null;
-      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}, ${value.manualMemoId}`,
         loc: value.loc,
       });
 
+      if (value.hasInvalidDeps === true) {
+        /*
+         * ValidateExhaustiveDependencies already reported an error for this
+         * memo block, skip validation to avoid duplicate errors
+         */
+        return;
+      }
+
+      let depsFromSource: Array<ManualMemoDependency> | null = null;
+      if (value.deps != null) {
+        depsFromSource = value.deps;
+      }
+
       state.manualMemoState = {
         loc: instruction.loc,
         decls: new Set(),
@@ -547,12 +556,15 @@ class Visitor extends ReactiveFunctionVisitor<VisitorState> {
       }
     }
     if (value.kind === 'FinishMemoize') {
+      if (state.manualMemoState == null) {
+        // StartMemoize had invalid deps, skip validation
+        return;
+      }
       CompilerError.invariant(
-        state.manualMemoState != null &&
-          state.manualMemoState.manualMemoId === value.manualMemoId,
+        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}`,
+          description: `Encountered StartMemoize id=${state.manualMemoState.manualMemoId} followed by FinishMemoize id=${value.manualMemoId}`,
           loc: value.loc,
         },
       );

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-ReactUseMemo-async-callback.expect.md

@@ -15,7 +15,7 @@ function component(a, b) {
 ## Error
 
 ```
-Found 3 errors:
+Found 2 errors:
 
 Error: useMemo() callbacks may not be async or generator functions
 
@@ -47,22 +47,6 @@ error.invalid-ReactUseMemo-async-callback.ts:3:10
   6 | }
 
 Inferred dependencies: `[a]`
-
-Compilation Skipped: Existing memoization could not be preserved
-
-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 `a`, but the source dependencies were []. Inferred dependency not present in source.
-
-error.invalid-ReactUseMemo-async-callback.ts:2:24
-  1 | function component(a, b) {
-> 2 |   let x = React.useMemo(async () => {
-    |                         ^^^^^^^^^^^^^
-> 3 |     await a;
-    | ^^^^^^^^^^^^
-> 4 |   }, []);
-    | ^^^^ Could not preserve existing manual memoization
-  5 |   return x;
-  6 | }
-  7 |
 ```
           
       

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-useMemo-async-callback.expect.md

@@ -15,7 +15,7 @@ function component(a, b) {
 ## Error
 
 ```
-Found 3 errors:
+Found 2 errors:
 
 Error: useMemo() callbacks may not be async or generator functions
 
@@ -47,22 +47,6 @@ error.invalid-useMemo-async-callback.ts:3:10
   6 | }
 
 Inferred dependencies: `[a]`
-
-Compilation Skipped: Existing memoization could not be preserved
-
-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 `a`, but the source dependencies were []. Inferred dependency not present in source.
-
-error.invalid-useMemo-async-callback.ts:2:18
-  1 | function component(a, b) {
-> 2 |   let x = useMemo(async () => {
-    |                   ^^^^^^^^^^^^^
-> 3 |     await a;
-    | ^^^^^^^^^^^^
-> 4 |   }, []);
-    | ^^^^ Could not preserve existing manual memoization
-  5 |   return x;
-  6 | }
-  7 |
 ```
           
       

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-useMemo-callback-args.expect.md

@@ -13,7 +13,7 @@ function component(a, b) {
 ## Error
 
 ```
-Found 3 errors:
+Found 2 errors:
 
 Error: useMemo() callbacks may not accept parameters
 
@@ -40,18 +40,6 @@ error.invalid-useMemo-callback-args.ts:2:23
   5 |
 
 Inferred dependencies: `[a]`
-
-Compilation Skipped: Existing memoization could not be preserved
-
-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 `a`, but the source dependencies were []. Inferred dependency not present in source.
-
-error.invalid-useMemo-callback-args.ts:2:18
-  1 | function component(a, b) {
-> 2 |   let x = useMemo(c => a, []);
-    |                   ^^^^^^ Could not preserve existing manual memoization
-  3 |   return x;
-  4 | }
-  5 |
 ```
           
       

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps.expect.md

@@ -51,7 +51,7 @@ function Component({x, y, z}) {
 ## Error
 
 ```
-Found 6 errors:
+Found 4 errors:
 
 Error: Found missing/extra memoization dependencies
 
@@ -157,48 +157,6 @@ error.invalid-exhaustive-deps.ts:37:13
   40 |   }, []);
 
 Inferred dependencies: `[ref]`
-
-Compilation Skipped: Existing memoization could not be preserved
-
-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 `x.y.z.a.b`, but the source dependencies were [x?.y.z.a?.b.z]. Inferred different dependency than source.
-
-error.invalid-exhaustive-deps.ts:14:20
-  12 |     // ok, not our job to type check nullability
-  13 |   }, [x.y.z.a]);
-> 14 |   const c = useMemo(() => {
-     |                     ^^^^^^^
-> 15 |     return x?.y.z.a?.b;
-     | ^^^^^^^^^^^^^^^^^^^^^^^
-> 16 |     // error: too precise
-     | ^^^^^^^^^^^^^^^^^^^^^^^
-> 17 |   }, [x?.y.z.a?.b.z]);
-     | ^^^^ Could not preserve existing manual memoization
-  18 |   const d = useMemo(() => {
-  19 |     return x?.y?.[(console.log(y), z?.b)];
-  20 |     // ok
-
-Compilation Skipped: Existing memoization could not be preserved
-
-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`, but the source dependencies were []. Inferred dependency not present in source.
-
-error.invalid-exhaustive-deps.ts:35:21
-  33 |   const ref2 = useRef(null);
-  34 |   const ref = z ? ref1 : ref2;
-> 35 |   const cb = useMemo(() => {
-     |                      ^^^^^^^
-> 36 |     return () => {
-     | ^^^^^^^^^^^^^^^^^^
-> 37 |       return ref.current;
-     | ^^^^^^^^^^^^^^^^^^
-> 38 |     };
-     | ^^^^^^^^^^^^^^^^^^
-> 39 |     // error: ref is a stable type but reactive
-     | ^^^^^^^^^^^^^^^^^^
-> 40 |   }, []);
-     | ^^^^ Could not preserve existing manual memoization
-  41 |   return <Stringify results={[a, b, c, d, e, f, cb]} />;
-  42 | }
-  43 |
 ```
           
       

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-missing-nonreactive-dep-unmemoized.expect.md

@@ -22,7 +22,7 @@ function useHook() {
 ## Error
 
 ```
-Found 2 errors:
+Found 1 error:
 
 Error: Found missing memoization dependencies
 
@@ -38,19 +38,6 @@ error.invalid-missing-nonreactive-dep-unmemoized.ts:11:31
   14 |
 
 Inferred dependencies: `[object]`
-
-Compilation Skipped: Existing memoization could not be preserved
-
-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 `object`, but the source dependencies were []. Inferred dependency not present in source.
-
-error.invalid-missing-nonreactive-dep-unmemoized.ts:11:24
-   9 |   useIdentity();
-  10 |   object.x = 0;
-> 11 |   const array = useMemo(() => [object], []);
-     |                         ^^^^^^^^^^^^^^ Could not preserve existing manual memoization
-  12 |   return array;
-  13 | }
-  14 |
 ```
           
       

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-unrelated-mutation-in-depslist.expect.md

@@ -30,7 +30,7 @@ function useFoo(input1) {
 ## Error
 
 ```
-Found 2 errors:
+Found 1 error:
 
 Error: Found missing memoization dependencies
 
@@ -46,23 +46,6 @@ error.useMemo-unrelated-mutation-in-depslist.ts:18:14
   21 | }
 
 Inferred dependencies: `[x, y]`
-
-Compilation Skipped: Existing memoization could not be preserved
-
-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 `input1`, but the source dependencies were [y]. Inferred different dependency than source.
-
-error.useMemo-unrelated-mutation-in-depslist.ts:16:27
-  14 |   const x = {};
-  15 |   const y = [input1];
-> 16 |   const memoized = useMemo(() => {
-     |                            ^^^^^^^
-> 17 |     return [y];
-     | ^^^^^^^^^^^^^^^
-> 18 |   }, [(mutate(x), y)]);
-     | ^^^^ Could not preserve existing manual memoization
-  19 |
-  20 |   return [x, memoized];
-  21 | }
 ```