[compiler] Auto-fix for non-exhaustive deps

Records more information in DropManualMemoization so that we know the full span of the manual dependencies array (if present). This allows ValidateExhaustiveDeps to include a suggestion with the correct deps.

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

@@ -600,8 +600,7 @@ function printErrorSummary(category: ErrorCategory, message: string): string {
     case ErrorCategory.Suppression:
     case ErrorCategory.Syntax:
     case ErrorCategory.UseMemo:
-    case ErrorCategory.VoidUseMemo:
-    case ErrorCategory.MemoDependencies: {
+    case ErrorCategory.VoidUseMemo: {
       heading = 'Error';
       break;
     }
@@ -659,10 +658,6 @@ export enum ErrorCategory {
    * Checks that manual memoization is preserved
    */
   PreserveManualMemo = 'PreserveManualMemo',
-  /**
-   * Checks for exhaustive useMemo/useCallback dependencies without extraneous values
-   */
-  MemoDependencies = 'MemoDependencies',
   /**
    * Checks for known incompatible libraries
    */
@@ -1060,16 +1055,6 @@ function getRuleForCategoryImpl(category: ErrorCategory): LintRule {
         preset: LintRulePreset.RecommendedLatest,
       };
     }
-    case ErrorCategory.MemoDependencies: {
-      return {
-        category,
-        severity: ErrorSeverity.Error,
-        name: 'memo-dependencies',
-        description:
-          'Validates that useMemo() and useCallback() specify comprehensive dependencies without extraneous values. See [`useMemo()` docs](https://react.dev/reference/react/useMemo) for more information.',
-        preset: LintRulePreset.RecommendedLatest,
-      };
-    }
     case ErrorCategory.IncompatibleLibrary: {
       return {
         category,

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts

@@ -400,15 +400,7 @@ export function compileProgram(
    */
   const suppressions = findProgramSuppressions(
     pass.comments,
-    /*
-     * If the compiler is validating hooks rules and exhaustive memo dependencies, we don't need to check
-     * for React ESLint suppressions
-     */
-    pass.opts.environment.validateExhaustiveMemoizationDependencies &&
-      pass.opts.environment.validateHooksUsage
-      ? null
-      : (pass.opts.eslintSuppressionRules ?? DEFAULT_ESLINT_SUPPRESSIONS),
-    // Always bail on Flow suppressions
+    pass.opts.eslintSuppressionRules ?? DEFAULT_ESLINT_SUPPRESSIONS,
     pass.opts.flowSuppressions,
   );
 

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Suppression.ts

@@ -78,7 +78,7 @@ export function filterSuppressionsThatAffectFunction(
 
 export function findProgramSuppressions(
   programComments: Array<t.Comment>,
-  ruleNames: Array<string> | null,
+  ruleNames: Array<string>,
   flowSuppressions: boolean,
 ): Array<SuppressionRange> {
   const suppressionRanges: Array<SuppressionRange> = [];
@@ -89,7 +89,7 @@ export function findProgramSuppressions(
   let disableNextLinePattern: RegExp | null = null;
   let disablePattern: RegExp | null = null;
   let enablePattern: RegExp | null = null;
-  if (ruleNames != null && ruleNames.length !== 0) {
+  if (ruleNames.length !== 0) {
     const rulePattern = `(${ruleNames.join('|')})`;
     disableNextLinePattern = new RegExp(
       `eslint-disable-next-line ${rulePattern}`,

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

@@ -22,9 +22,7 @@ import {
   Identifier,
   IdentifierId,
   InstructionKind,
-  isStableType,
   isSubPath,
-  isUseRefType,
   LoadGlobal,
   ManualMemoDependency,
   Place,
@@ -48,10 +46,9 @@ const DEBUG = false;
  * or less times.
  *
  * TODOs:
- * - Handle cases of mixed optional and non-optional versions of the same path,
- *   eg referecing both x.y.z and x.y?.z in the same memo block. we should collapse
- *   this into a single canonical dep that we look for in the manual deps. see the
- *   existing exhaustive deps rule for implementation.
+ * - Better handling of cases where we infer multiple dependencies related to a single
+ *   variable. Eg if the user has dep `x` and we inferred `x.y, x.z`, the user's dep
+ *   is sufficient.
  * - Handle cases where the user deps were not simple identifiers + property chains.
  *   We try to detect this in ValidateUseMemo but we miss some cases. The problem
  *   is that invalid forms can be value blocks or function calls that don't get
@@ -111,7 +108,7 @@ export function validateExhaustiveDependencies(
     );
     visitCandidateDependency(value.decl, temporaries, dependencies, locals);
     const inferred: Array<InferredDependency> = Array.from(dependencies);
-    // Sort dependencies by name and path, with shorter/non-optional paths first
+    // Sort dependencies by name, and path, with shorter/non-optional paths first
     inferred.sort((a, b) => {
       if (a.kind === 'Global' && b.kind == 'Global') {
         return a.binding.name.localeCompare(b.binding.name);
@@ -208,31 +205,6 @@ export function validateExhaustiveDependencies(
         reason: 'Unexpected function dependency',
         loc: value.loc,
       });
-      /**
-       * Dependencies technically only need to include reactive values. However,
-       * reactivity inference for general values is subtle since it involves all
-       * of our complex control and data flow analysis. To keep results more
-       * stable and predictable to developers, we intentionally stay closer to
-       * the rules of the classic exhaustive-deps rule. Values should be included
-       * as dependencies if either of the following is true:
-       * - They're reactive
-       * - They're non-reactive and not a known-stable value type.
-       *
-       * Thus `const ref: Ref = cond ? ref1 : ref2` has to be a dependency
-       * (assuming `cond` is reactive) since it's reactive despite being a ref.
-       *
-       * Similarly, `const x = [1,2,3]` has to be a dependency since even
-       * though it's non reactive, it's not a known stable type.
-       *
-       * TODO: consider reimplementing a simpler form of reactivity inference.
-       * Ideally we'd consider `const ref: Ref = cond ? ref1 : ref2` as a required
-       * dependency even if our data/control flow tells us that `cond` is non-reactive.
-       * It's simpler for developers to reason about based on a more structural/AST
-       * driven approach.
-       */
-      const isRequiredDependency =
-        reactive.has(inferredDependency.identifier.id) ||
-        !isStableType(inferredDependency.identifier);
       let hasMatchingManualDependency = false;
       for (const manualDependency of manualDependencies) {
         if (
@@ -244,37 +216,24 @@ export function validateExhaustiveDependencies(
         ) {
           hasMatchingManualDependency = true;
           matched.add(manualDependency);
-          if (!isRequiredDependency) {
-            extra.push(manualDependency);
-          }
         }
       }
-      if (isRequiredDependency && !hasMatchingManualDependency) {
+      if (!hasMatchingManualDependency) {
         missing.push(inferredDependency);
       }
     }
 
     for (const dep of startMemo.deps ?? []) {
-      if (matched.has(dep)) {
+      if (
+        matched.has(dep) ||
+        (dep.root.kind === 'NamedLocal' &&
+          !reactive.has(dep.root.value.identifier.id))
+      ) {
         continue;
       }
       extra.push(dep);
     }
 
-    /*
-     * For compatiblity with the existing exhaustive-deps rule, we allow
-     * known-stable values as dependencies even if the value is not reactive.
-     * This allows code that takes a dep on a non-reactive setState function
-     * to pass, for example.
-     */
-    retainWhere(extra, dep => {
-      const isNonReactiveStableValue =
-        dep.root.kind === 'NamedLocal' &&
-        !dep.root.value.reactive &&
-        isStableType(dep.root.value.identifier);
-      return !isNonReactiveStableValue;
-    });
-
     if (missing.length !== 0 || extra.length !== 0) {
       let suggestions: Array<CompilerSuggestion> | null = null;
       if (startMemo.depsLoc != null && typeof startMemo.depsLoc !== 'symbol') {
@@ -288,39 +247,36 @@ export function validateExhaustiveDependencies(
         ];
       }
       if (missing.length !== 0) {
+        // Error
         const diagnostic = CompilerDiagnostic.create({
-          category: ErrorCategory.MemoDependencies,
+          category: ErrorCategory.PreserveManualMemo,
           reason: 'Found non-exhaustive dependencies',
           description:
             'Missing dependencies can cause a value not to update when those inputs change, ' +
-            'resulting in stale UI',
+            'resulting in stale UI. This memoization cannot be safely rewritten by the compiler.',
           suggestions,
         });
         for (const dep of missing) {
-          let reactiveStableValueHint = '';
-          if (isStableType(dep.identifier)) {
-            reactiveStableValueHint =
-              '. Refs, setState functions, and other "stable" values generally do not need to be added as dependencies, but this variable may change over time to point to different values';
-          }
           diagnostic.withDetails({
             kind: 'error',
-            message: `Missing dependency \`${printInferredDependency(dep)}\`${reactiveStableValueHint}`,
+            message: `Missing dependency \`${printInferredDependency(dep)}\``,
             loc: dep.loc,
           });
         }
         error.pushDiagnostic(diagnostic);
       } else if (extra.length !== 0) {
         const diagnostic = CompilerDiagnostic.create({
-          category: ErrorCategory.MemoDependencies,
+          category: ErrorCategory.PreserveManualMemo,
           reason: 'Found unnecessary memoization dependencies',
           description:
             'Unnecessary dependencies can cause a value to update more often than necessary, ' +
-            'which can cause effects to run more than expected',
+            'which can cause effects to run more than expected. This memoization cannot be safely ' +
+            'rewritten by the compiler',
         });
         diagnostic.withDetails({
           kind: 'error',
           message: `Unnecessary dependencies ${extra.map(dep => `\`${printManualMemoDependency(dep)}\``).join(', ')}`,
-          loc: startMemo.depsLoc ?? value.loc,
+          loc: value.loc,
         });
         error.pushDiagnostic(diagnostic);
       }
@@ -331,15 +287,10 @@ export function validateExhaustiveDependencies(
     startMemo = null;
   }
 
-  collectDependencies(
-    fn,
-    temporaries,
-    {
-      onStartMemoize,
-      onFinishMemoize,
-    },
-    false, // isFunctionExpression
-  );
+  collectDependencies(fn, temporaries, {
+    onStartMemoize,
+    onFinishMemoize,
+  });
   return error.asResult();
 }
 
@@ -432,20 +383,12 @@ function collectDependencies(
       locals: Set<IdentifierId>,
     ) => void;
   } | null,
-  isFunctionExpression: boolean,
 ): Extract<Temporary, {kind: 'Function'}> {
   const optionals = findOptionalPlaces(fn);
   if (DEBUG) {
     console.log(prettyFormat(optionals));
   }
   const locals: Set<IdentifierId> = new Set();
-  if (isFunctionExpression) {
-    for (const param of fn.params) {
-      const place = param.kind === 'Identifier' ? param : param.place;
-      locals.add(place.identifier.id);
-    }
-  }
-
   const dependencies: Set<InferredDependency> = new Set();
   function visit(place: Place): void {
     visitCandidateDependency(place, temporaries, dependencies, locals);
@@ -580,11 +523,7 @@ function collectDependencies(
           break;
         }
         case 'PropertyLoad': {
-          if (
-            typeof value.property === 'number' ||
-            (isUseRefType(value.object.identifier) &&
-              value.property === 'current')
-          ) {
+          if (typeof value.property === 'number') {
             visit(value.object);
             break;
           }
@@ -614,7 +553,6 @@ function collectDependencies(
             value.loweredFunc.func,
             temporaries,
             null,
-            true, // isFunctionExpression
           );
           temporaries.set(lvalue.identifier.id, functionDeps);
           addDependency(functionDeps, dependencies, locals);

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/compile-files-with-exhaustive-deps-violation-in-effects.expect.md

@@ -1,91 +0,0 @@
-
-## Input
-
-```javascript
-// @validateExhaustiveMemoizationDependencies
-
-import {useMemo} from 'react';
-import {ValidateMemoization} from 'shared-runtime';
-
-function Component({x}) {
-  useEffect(
-    () => {
-      console.log(x);
-      // eslint-disable-next-line react-hooks/exhaustive-deps
-    },
-    [
-      /* intentionally missing deps */
-    ]
-  );
-
-  const memo = useMemo(() => {
-    return [x];
-  }, [x]);
-
-  return <ValidateMemoization inputs={[x]} output={memo} />;
-}
-
-```
-
-## Code
-
-```javascript
-import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveMemoizationDependencies
-
-import { useMemo } from "react";
-import { ValidateMemoization } from "shared-runtime";
-
-function Component(t0) {
-  const $ = _c(10);
-  const { x } = t0;
-  let t1;
-  if ($[0] !== x) {
-    t1 = () => {
-      console.log(x);
-    };
-    $[0] = x;
-    $[1] = t1;
-  } else {
-    t1 = $[1];
-  }
-  let t2;
-  if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
-    t2 = [];
-    $[2] = t2;
-  } else {
-    t2 = $[2];
-  }
-  useEffect(t1, t2);
-  let t3;
-  if ($[3] !== x) {
-    t3 = [x];
-    $[3] = x;
-    $[4] = t3;
-  } else {
-    t3 = $[4];
-  }
-  const memo = t3;
-  let t4;
-  if ($[5] !== x) {
-    t4 = [x];
-    $[5] = x;
-    $[6] = t4;
-  } else {
-    t4 = $[6];
-  }
-  let t5;
-  if ($[7] !== memo || $[8] !== t4) {
-    t5 = <ValidateMemoization inputs={t4} output={memo} />;
-    $[7] = memo;
-    $[8] = t4;
-    $[9] = t5;
-  } else {
-    t5 = $[9];
-  }
-  return t5;
-}
-
-```
-      
-### Eval output
-(kind: exception) Fixture not implemented

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/compile-files-with-exhaustive-deps-violation-in-effects.js

@@ -1,22 +0,0 @@
-// @validateExhaustiveMemoizationDependencies
-
-import {useMemo} from 'react';
-import {ValidateMemoization} from 'shared-runtime';
-
-function Component({x}) {
-  useEffect(
-    () => {
-      console.log(x);
-      // eslint-disable-next-line react-hooks/exhaustive-deps
-    },
-    [
-      /* intentionally missing deps */
-    ]
-  );
-
-  const memo = useMemo(() => {
-    return [x];
-  }, [x]);
-
-  return <ValidateMemoization inputs={[x]} output={memo} />;
-}

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

@@ -26,16 +26,8 @@ function Component({x, y, z}) {
   }, [x]);
   const f = useMemo(() => {
     return [];
-  }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]);
-  const ref1 = useRef(null);
-  const ref2 = useRef(null);
-  const ref = z ? ref1 : ref2;
-  const cb = useMemo(() => {
-    return () => {
-      return ref.current;
-    };
-  }, []);
-  return <Stringify results={[a, b, c, d, e, f, cb]} />;
+  }, [x, y.z, z?.y?.a]);
+  return <Stringify results={[a, b, c, d, e, f]} />;
 }
 
 ```
@@ -44,11 +36,11 @@ function Component({x, y, z}) {
 ## Error
 
 ```
-Found 5 errors:
+Found 4 errors:
 
-Error: Found non-exhaustive dependencies
+Compilation Skipped: Found non-exhaustive dependencies
 
-Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI.
+Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI. This memoization cannot be safely rewritten by the compiler..
 
 error.invalid-exhaustive-deps.ts:7:11
    5 | function Component({x, y, z}) {
@@ -59,9 +51,9 @@ error.invalid-exhaustive-deps.ts:7:11
    9 |   const b = useMemo(() => {
   10 |     return x.y.z?.a;
 
-Error: Found non-exhaustive dependencies
+Compilation Skipped: Found non-exhaustive dependencies
 
-Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI.
+Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI. This memoization cannot be safely rewritten by the compiler..
 
 error.invalid-exhaustive-deps.ts:10:11
    8 |   }, [x?.y.z?.a.b]);
@@ -72,9 +64,9 @@ error.invalid-exhaustive-deps.ts:10:11
   12 |   const c = useMemo(() => {
   13 |     return x?.y.z.a?.b;
 
-Error: Found non-exhaustive dependencies
+Compilation Skipped: Found non-exhaustive dependencies
 
-Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI.
+Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI. This memoization cannot be safely rewritten by the compiler..
 
 error.invalid-exhaustive-deps.ts:13:11
   11 |   }, [x.y.z.a]);
@@ -85,31 +77,22 @@ error.invalid-exhaustive-deps.ts:13:11
   15 |   const d = useMemo(() => {
   16 |     return x?.y?.[(console.log(y), z?.b)];
 
-Error: Found unnecessary memoization dependencies
+Compilation Skipped: Found unnecessary memoization dependencies
 
-Unnecessary dependencies can cause a value to update more often than necessary, which can cause effects to run more than expected.
+Unnecessary dependencies can cause a value to update more often than necessary, which can cause effects to run more than expected. This memoization cannot be safely rewritten by the compiler.
 
-error.invalid-exhaustive-deps.ts:25:5
-  23 |   const f = useMemo(() => {
-  24 |     return [];
-> 25 |   }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]);
-     |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Unnecessary dependencies `x`, `y.z`, `z?.y?.a`, `UNUSED_GLOBAL`
-  26 |   const ref1 = useRef(null);
-  27 |   const ref2 = useRef(null);
-  28 |   const ref = z ? ref1 : ref2;
-
-Error: Found non-exhaustive dependencies
-
-Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI.
-
-error.invalid-exhaustive-deps.ts:31:13
-  29 |   const cb = useMemo(() => {
-  30 |     return () => {
-> 31 |       return ref.current;
-     |              ^^^ Missing dependency `ref`. Refs, setState functions, and other "stable" values generally do not need to be added as dependencies, but this variable may change over time to point to different values
-  32 |     };
-  33 |   }, []);
-  34 |   return <Stringify results={[a, b, c, d, e, f, cb]} />;
+error.invalid-exhaustive-deps.ts:23:20
+  21 |     return e;
+  22 |   }, [x]);
+> 23 |   const f = useMemo(() => {
+     |                     ^^^^^^^
+> 24 |     return [];
+     | ^^^^^^^^^^^^^^
+> 25 |   }, [x, y.z, z?.y?.a]);
+     | ^^^^ Unnecessary dependencies `x`, `y.z`, `z?.y?.a`
+  26 |   return <Stringify results={[a, b, c, d, e, f]} />;
+  27 | }
+  28 |
 ```
           
       

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

@@ -22,14 +22,6 @@ function Component({x, y, z}) {
   }, [x]);
   const f = useMemo(() => {
     return [];
-  }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]);
-  const ref1 = useRef(null);
-  const ref2 = useRef(null);
-  const ref = z ? ref1 : ref2;
-  const cb = useMemo(() => {
-    return () => {
-      return ref.current;
-    };
-  }, []);
-  return <Stringify results={[a, b, c, d, e, f, cb]} />;
+  }, [x, y.z, z?.y?.a]);
+  return <Stringify results={[a, b, c, d, e, f]} />;
 }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.expect.md

@@ -1,100 +0,0 @@
-
-## Input
-
-```javascript
-// @validateExhaustiveMemoizationDependencies
-import {
-  useCallback,
-  useTransition,
-  useState,
-  useOptimistic,
-  useActionState,
-  useRef,
-  useReducer,
-} from 'react';
-
-function useFoo() {
-  const [s, setState] = useState();
-  const ref = useRef(null);
-  const [t, startTransition] = useTransition();
-  const [u, addOptimistic] = useOptimistic();
-  const [v, dispatch] = useReducer(() => {}, null);
-  const [isPending, dispatchAction] = useActionState(() => {}, null);
-
-  return useCallback(() => {
-    dispatch();
-    startTransition(() => {});
-    addOptimistic();
-    setState(null);
-    dispatchAction();
-    ref.current = true;
-  }, [
-    // intentionally adding unnecessary deps on nonreactive stable values
-    // to check that they're allowed
-    dispatch,
-    startTransition,
-    addOptimistic,
-    setState,
-    dispatchAction,
-    ref,
-  ]);
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: useFoo,
-  params: [],
-};
-
-```
-
-## Code
-
-```javascript
-import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveMemoizationDependencies
-import {
-  useCallback,
-  useTransition,
-  useState,
-  useOptimistic,
-  useActionState,
-  useRef,
-  useReducer,
-} from "react";
-
-function useFoo() {
-  const $ = _c(1);
-  const [, setState] = useState();
-  const ref = useRef(null);
-  const [, startTransition] = useTransition();
-  const [, addOptimistic] = useOptimistic();
-  const [, dispatch] = useReducer(_temp, null);
-  const [, dispatchAction] = useActionState(_temp2, null);
-  let t0;
-  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
-    t0 = () => {
-      dispatch();
-      startTransition(_temp3);
-      addOptimistic();
-      setState(null);
-      dispatchAction();
-      ref.current = true;
-    };
-    $[0] = t0;
-  } else {
-    t0 = $[0];
-  }
-  return t0;
-}
-function _temp3() {}
-function _temp2() {}
-function _temp() {}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: useFoo,
-  params: [],
-};
-
-```
-      
-### Eval output
-(kind: ok) "[[ function params=0 ]]"

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.js

@@ -1,42 +0,0 @@
-// @validateExhaustiveMemoizationDependencies
-import {
-  useCallback,
-  useTransition,
-  useState,
-  useOptimistic,
-  useActionState,
-  useRef,
-  useReducer,
-} from 'react';
-
-function useFoo() {
-  const [s, setState] = useState();
-  const ref = useRef(null);
-  const [t, startTransition] = useTransition();
-  const [u, addOptimistic] = useOptimistic();
-  const [v, dispatch] = useReducer(() => {}, null);
-  const [isPending, dispatchAction] = useActionState(() => {}, null);
-
-  return useCallback(() => {
-    dispatch();
-    startTransition(() => {});
-    addOptimistic();
-    setState(null);
-    dispatchAction();
-    ref.current = true;
-  }, [
-    // intentionally adding unnecessary deps on nonreactive stable values
-    // to check that they're allowed
-    dispatch,
-    startTransition,
-    addOptimistic,
-    setState,
-    dispatchAction,
-    ref,
-  ]);
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: useFoo,
-  params: [],
-};

compiler/packages/snap/src/runner.ts

@@ -53,10 +53,8 @@ const opts: RunnerOptions = yargs
   .default('worker-threads', true)
   .boolean('watch')
   .describe('watch', 'Run compiler in watch mode, re-running after changes')
-  .alias('w', 'watch')
   .default('watch', false)
   .boolean('update')
-  .alias('u', 'update')
   .describe('update', 'Update fixtures')
   .default('update', false)
   .boolean('filter')

packages/react-devtools-shared/src/__tests__/profilerContext-test.js

@@ -584,75 +584,4 @@ describe('ProfilerContext', () => {
     await utils.actAsync(() => context.selectFiber(childID, 'Child'));
     expect(inspectedElementID).toBe(parentID);
   });
-
-  it('should toggle profiling when the keyboard shortcut is pressed', async () => {
-    // Context providers
-    const Profiler =
-      require('react-devtools-shared/src/devtools/views/Profiler/Profiler').default;
-    const {
-      TimelineContextController,
-    } = require('react-devtools-timeline/src/TimelineContext');
-    const {
-      SettingsContextController,
-    } = require('react-devtools-shared/src/devtools/views/Settings/SettingsContext');
-    const {
-      ModalDialogContextController,
-    } = require('react-devtools-shared/src/devtools/views/ModalDialog');
-
-    // Dom component for profiling to be enabled
-    const Component = () => null;
-    utils.act(() => render(<Component />));
-
-    const profilerContainer = document.createElement('div');
-    document.body.appendChild(profilerContainer);
-
-    // Create a root for the profiler
-    const profilerRoot = ReactDOMClient.createRoot(profilerContainer);
-
-    // Render the profiler
-    utils.act(() => {
-      profilerRoot.render(
-        <Contexts>
-          <SettingsContextController browserTheme="light">
-            <ModalDialogContextController>
-              <TimelineContextController>
-                <Profiler />
-              </TimelineContextController>
-            </ModalDialogContextController>
-          </SettingsContextController>
-        </Contexts>,
-      );
-    });
-
-    // Verify that the profiler is not profiling.
-    expect(store.profilerStore.isProfilingBasedOnUserInput).toBe(false);
-
-    // Trigger the keyboard shortcut.
-    const ownerWindow = profilerContainer.ownerDocument.defaultView;
-    const isMac =
-      typeof navigator !== 'undefined' &&
-      navigator.platform.toUpperCase().indexOf('MAC') >= 0;
-
-    const keyEvent = new KeyboardEvent('keydown', {
-      key: 'e',
-      metaKey: isMac,
-      ctrlKey: !isMac,
-      bubbles: true,
-    });
-
-    // Dispatch keyboard event to toggle profiling on
-    // Try utils.actAsync with recursivelyFlush=false
-    await utils.actAsync(() => {
-      ownerWindow.dispatchEvent(keyEvent);
-    }, false);
-    expect(store.profilerStore.isProfilingBasedOnUserInput).toBe(true);
-
-    // Dispatch keyboard event to toggle profiling off
-    await utils.actAsync(() => {
-      ownerWindow.dispatchEvent(keyEvent);
-    }, false);
-    expect(store.profilerStore.isProfilingBasedOnUserInput).toBe(false);
-
-    document.body.removeChild(profilerContainer);
-  });
 });

packages/react-devtools-shared/src/devtools/views/Profiler/Profiler.js

@@ -8,7 +8,7 @@
  */
 
 import * as React from 'react';
-import {Fragment, useContext, useEffect, useRef, useEffectEvent} from 'react';
+import {Fragment, useContext} from 'react';
 import {ModalDialog} from '../ModalDialog';
 import {ProfilerContext} from './ProfilerContext';
 import TabBar from '../TabBar';
@@ -38,11 +38,6 @@ import {TimelineContext} from 'react-devtools-timeline/src/TimelineContext';
 import styles from './Profiler.css';
 
 function Profiler(_: {}) {
-  const profilerRef = useRef<HTMLDivElement | null>(null);
-  const isMac =
-    typeof navigator !== 'undefined' &&
-    navigator.platform.toUpperCase().indexOf('MAC') >= 0;
-
   const {
     didRecordCommits,
     isProcessingData,
@@ -52,8 +47,6 @@ function Profiler(_: {}) {
     selectedTabID,
     selectTab,
     supportsProfiling,
-    startProfiling,
-    stopProfiling,
   } = useContext(ProfilerContext);
 
   const {file: timelineTraceEventData, searchInputContainerRef} =
@@ -63,32 +56,6 @@ function Profiler(_: {}) {
 
   const isLegacyProfilerSelected = selectedTabID !== 'timeline';
 
-  // Cmd+E to start/stop profiler recording
-  const handleKeyDown = useEffectEvent((event: KeyboardEvent) => {
-    const correctModifier = isMac ? event.metaKey : event.ctrlKey;
-    if (correctModifier && event.key === 'e') {
-      if (isProfiling) {
-        stopProfiling();
-      } else {
-        startProfiling();
-      }
-      event.preventDefault();
-      event.stopPropagation();
-    }
-  });
-
-  useEffect(() => {
-    const div = profilerRef.current;
-    if (!div) {
-      return;
-    }
-    const ownerWindow = div.ownerDocument.defaultView;
-    ownerWindow.addEventListener('keydown', handleKeyDown);
-    return () => {
-      ownerWindow.removeEventListener('keydown', handleKeyDown);
-    };
-  }, []);
-
   let view = null;
   if (didRecordCommits || selectedTabID === 'timeline') {
     switch (selectedTabID) {
@@ -145,7 +112,7 @@ function Profiler(_: {}) {
 
   return (
     <SettingsModalContextController>
-      <div ref={profilerRef} className={styles.Profiler}>
+      <div className={styles.Profiler}>
         <div className={styles.LeftColumn}>
           <div className={styles.Toolbar}>
             <RecordToggle disabled={!supportsProfiling} />

packages/react-devtools-shared/src/devtools/views/Profiler/RecordToggle.js

@@ -30,19 +30,13 @@ export default function RecordToggle({disabled}: Props): React.Node {
     className = styles.ActiveRecordToggle;
   }
 
-  const isMac =
-    typeof navigator !== 'undefined' &&
-    navigator.platform.toUpperCase().indexOf('MAC') >= 0;
-  const shortcut = isMac ? '⌘E' : 'Ctrl+E';
-  const title = `${isProfiling ? 'Stop' : 'Start'} profiling - ${shortcut}`;
-
   return (
     <Button
       className={className}
       disabled={disabled}
       onClick={isProfiling ? stopProfiling : startProfiling}
       testName="ProfilerToggleButton"
-      title={title}>
+      title={isProfiling ? 'Stop profiling' : 'Start profiling'}>
       <ButtonIcon type="record" />
     </Button>
   );