From fca172e3f3b29aee12e4e99b8f151e7fd138a8db Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Mon, 24 Nov 2025 12:12:49 -0800 Subject: [PATCH 1/6] [compiler] Ignore ESLint suppressions when ValidateMemoDeps enabled (#35184) With `ValidateExhaustiveMemoDependencies` we can now check exhaustive dependencies for useMemo and useCallback within the compiler, without relying on the separate exhaustive-deps rule. Until now we've bailed out of any component/hook that suppresses this rule, since the suppression _might_ affect a memoization value. Compiling code with incorrect memo deps can change behavior so this wasn't safe. The downside was that a suppression within a useEffect could prevent memoization, even though non-exhaustive deps for effects do not cause problems for memoization specifically. So here, we change to ignore ESLint suppressions if we have both the compiler's hooks validation and memo deps validations enabled. Now we just have to test out the new validation and refine before we can enable this by default. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35184). * #35201 * #35202 * #35192 * #35190 * #35186 * #35185 * __->__ #35184 --- .../src/Entrypoint/Program.ts | 10 +- .../src/Entrypoint/Suppression.ts | 4 +- ...ustive-deps-violation-in-effects.expect.md | 91 +++++++++++++++++++ ...th-exhaustive-deps-violation-in-effects.js | 22 +++++ compiler/packages/snap/src/runner.ts | 2 + 5 files changed, 126 insertions(+), 3 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/compile-files-with-exhaustive-deps-violation-in-effects.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/compile-files-with-exhaustive-deps-violation-in-effects.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts index 7d12e054373..5ac580abbf0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts @@ -400,7 +400,15 @@ export function compileProgram( */ const suppressions = findProgramSuppressions( pass.comments, - pass.opts.eslintSuppressionRules ?? DEFAULT_ESLINT_SUPPRESSIONS, + /* + * 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.flowSuppressions, ); diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Suppression.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Suppression.ts index 24a9bccf426..e0f845f269b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Suppression.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Suppression.ts @@ -78,7 +78,7 @@ export function filterSuppressionsThatAffectFunction( export function findProgramSuppressions( programComments: Array, - ruleNames: Array, + ruleNames: Array | null, flowSuppressions: boolean, ): Array { const suppressionRanges: Array = []; @@ -89,7 +89,7 @@ export function findProgramSuppressions( let disableNextLinePattern: RegExp | null = null; let disablePattern: RegExp | null = null; let enablePattern: RegExp | null = null; - if (ruleNames.length !== 0) { + if (ruleNames != null && ruleNames.length !== 0) { const rulePattern = `(${ruleNames.join('|')})`; disableNextLinePattern = new RegExp( `eslint-disable-next-line ${rulePattern}`, diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/compile-files-with-exhaustive-deps-violation-in-effects.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/compile-files-with-exhaustive-deps-violation-in-effects.expect.md new file mode 100644 index 00000000000..e8e18395eca --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/compile-files-with-exhaustive-deps-violation-in-effects.expect.md @@ -0,0 +1,91 @@ + +## 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 ; +} + +``` + +## 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 = ; + $[7] = memo; + $[8] = t4; + $[9] = t5; + } else { + t5 = $[9]; + } + return t5; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/compile-files-with-exhaustive-deps-violation-in-effects.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/compile-files-with-exhaustive-deps-violation-in-effects.js new file mode 100644 index 00000000000..64817e701f0 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/compile-files-with-exhaustive-deps-violation-in-effects.js @@ -0,0 +1,22 @@ +// @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 ; +} diff --git a/compiler/packages/snap/src/runner.ts b/compiler/packages/snap/src/runner.ts index 92a0a0f82ee..478a32d426c 100644 --- a/compiler/packages/snap/src/runner.ts +++ b/compiler/packages/snap/src/runner.ts @@ -53,8 +53,10 @@ 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') From c9a8cf3411baed43d4e24fab6ec895768b297fd2 Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Mon, 24 Nov 2025 12:15:06 -0800 Subject: [PATCH 2/6] [compiler] Allow nonreactive stable types as extraneous deps (#35185) When checking ValidateExhaustiveDeps internally, this seems to be the most common case that it flags. The current exhaustive-deps rule allows extraneous deps if they are a set of stable types. So here we reuse our existing isStableType() util in the compiler to allow this case. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35185). * #35201 * #35202 * #35192 * #35190 * #35186 * __->__ #35185 --- .../ValidateExhaustiveDependencies.ts | 14 +++ ...ctive-stable-types-as-extra-deps.expect.md | 100 ++++++++++++++++++ ...-nonreactive-stable-types-as-extra-deps.js | 42 ++++++++ 3 files changed, 156 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts index 313f773e294..df85ea8648c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts @@ -261,6 +261,20 @@ export function validateExhaustiveDependencies( 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 | null = null; if (startMemo.depsLoc != null && typeof startMemo.depsLoc !== 'symbol') { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.expect.md new file mode 100644 index 00000000000..5b6319ed7cf --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.expect.md @@ -0,0 +1,100 @@ + +## 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 ]]" \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.js new file mode 100644 index 00000000000..71000afcc42 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.js @@ -0,0 +1,42 @@ +// @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: [], +}; From 454e01e603464b19ec3b6991a7a781cf1908ac84 Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Mon, 24 Nov 2025 12:17:03 -0800 Subject: [PATCH 3/6] [compiler] Allow manual dependencies to have different optionality than inferred deps (#35186) Since adding this validation we've already changed our inference to use knowledge from manual memoization to inform when values are frozen and which values are non-nullable. To align with that, if the user chooses to use different optionality btw the deps and the memo block/callback, that's fine. The key is that eg `x?.y` will invalidate whenever `x.y` would, so from a memoization correctness perspective its fine. It's not our job to be a type checker: if a value is potentially nullable, it should likely use a nullable property access in both places but TypeScript/Flow can check that. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35186). * #35201 * #35202 * #35192 * #35190 * __->__ #35186 --- .../ValidateExhaustiveDependencies.ts | 6 +- .../error.invalid-exhaustive-deps.expect.md | 70 +++++++++---------- .../compiler/error.invalid-exhaustive-deps.js | 7 ++ 3 files changed, 44 insertions(+), 39 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts index df85ea8648c..01c82a375c9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts @@ -24,6 +24,7 @@ import { InstructionKind, isStableType, isSubPath, + isSubPathIgnoringOptionals, isUseRefType, LoadGlobal, ManualMemoDependency, @@ -240,7 +241,10 @@ export function validateExhaustiveDependencies( manualDependency.root.value.identifier.id === inferredDependency.identifier.id && (areEqualPaths(manualDependency.path, inferredDependency.path) || - isSubPath(manualDependency.path, inferredDependency.path)) + isSubPathIgnoringOptionals( + manualDependency.path, + inferredDependency.path, + )) ) { hasMatchingManualDependency = true; matched.add(manualDependency); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md index 5152f4121a2..bca95a84e1e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md @@ -9,23 +9,29 @@ import {Stringify} from 'shared-runtime'; function Component({x, y, z}) { const a = useMemo(() => { return x?.y.z?.a; + // error: too precise }, [x?.y.z?.a.b]); const b = useMemo(() => { return x.y.z?.a; + // ok, not our job to type check nullability }, [x.y.z.a]); const c = useMemo(() => { return x?.y.z.a?.b; + // error: too precise }, [x?.y.z.a?.b.z]); const d = useMemo(() => { return x?.y?.[(console.log(y), z?.b)]; + // ok }, [x?.y, y, z?.b]); const e = useMemo(() => { const e = []; e.push(x); return e; + // ok }, [x]); const f = useMemo(() => { return []; + // error: unnecessary }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]); const ref1 = useRef(null); const ref2 = useRef(null); @@ -34,6 +40,7 @@ function Component({x, y, z}) { return () => { return ref.current; }; + // error: ref is a stable type but reactive }, []); return ; } @@ -44,7 +51,7 @@ function Component({x, y, z}) { ## Error ``` -Found 5 errors: +Found 4 errors: Error: Found non-exhaustive dependencies @@ -55,61 +62,48 @@ error.invalid-exhaustive-deps.ts:7:11 6 | const a = useMemo(() => { > 7 | return x?.y.z?.a; | ^^^^^^^^^ Missing dependency `x?.y.z?.a` - 8 | }, [x?.y.z?.a.b]); - 9 | const b = useMemo(() => { - 10 | return x.y.z?.a; + 8 | // error: too precise + 9 | }, [x?.y.z?.a.b]); + 10 | const b = useMemo(() => { 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:10:11 - 8 | }, [x?.y.z?.a.b]); - 9 | const b = useMemo(() => { -> 10 | return x.y.z?.a; - | ^^^^^^^^ Missing dependency `x.y.z?.a` - 11 | }, [x.y.z.a]); - 12 | const c = useMemo(() => { - 13 | return x?.y.z.a?.b; - -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:13:11 - 11 | }, [x.y.z.a]); - 12 | const c = useMemo(() => { -> 13 | return x?.y.z.a?.b; +error.invalid-exhaustive-deps.ts:15:11 + 13 | }, [x.y.z.a]); + 14 | const c = useMemo(() => { +> 15 | return x?.y.z.a?.b; | ^^^^^^^^^^^ Missing dependency `x?.y.z.a?.b` - 14 | }, [x?.y.z.a?.b.z]); - 15 | const d = useMemo(() => { - 16 | return x?.y?.[(console.log(y), z?.b)]; + 16 | // error: too precise + 17 | }, [x?.y.z.a?.b.z]); + 18 | const d = useMemo(() => { Error: 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. -error.invalid-exhaustive-deps.ts:25:5 - 23 | const f = useMemo(() => { - 24 | return []; -> 25 | }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]); +error.invalid-exhaustive-deps.ts:31:5 + 29 | return []; + 30 | // error: unnecessary +> 31 | }, [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; + 32 | const ref1 = useRef(null); + 33 | const ref2 = useRef(null); + 34 | 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; +error.invalid-exhaustive-deps.ts:37:13 + 35 | const cb = useMemo(() => { + 36 | return () => { +> 37 | 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 ; + 38 | }; + 39 | // error: ref is a stable type but reactive + 40 | }, []); ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.js index 53c189cc428..c0f8d28837a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.js @@ -5,23 +5,29 @@ import {Stringify} from 'shared-runtime'; function Component({x, y, z}) { const a = useMemo(() => { return x?.y.z?.a; + // error: too precise }, [x?.y.z?.a.b]); const b = useMemo(() => { return x.y.z?.a; + // ok, not our job to type check nullability }, [x.y.z.a]); const c = useMemo(() => { return x?.y.z.a?.b; + // error: too precise }, [x?.y.z.a?.b.z]); const d = useMemo(() => { return x?.y?.[(console.log(y), z?.b)]; + // ok }, [x?.y, y, z?.b]); const e = useMemo(() => { const e = []; e.push(x); return e; + // ok }, [x]); const f = useMemo(() => { return []; + // error: unnecessary }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]); const ref1 = useRef(null); const ref2 = useRef(null); @@ -30,6 +36,7 @@ function Component({x, y, z}) { return () => { return ref.current; }; + // error: ref is a stable type but reactive }, []); return ; } From 67c1487ffd872c95a3bb7d8104eac6eca79fe8cb Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Mon, 24 Nov 2025 12:18:49 -0800 Subject: [PATCH 4/6] [compiler] Allow extraneous non-reactive locals (#35190) The existing exhaustive-deps rule allows omitting non-reactive dependencies, even if they're not memoized. Conceptually, if a value is non-reactive then it cannot semantically change. Even if the value is a new object, that object represents the exact same value and doesn't necessitate redoing downstream computation. Thus its fine to exclude nonreactive dependencies, whether they're a stable type or not. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35190). * #35201 * #35202 * #35192 * __->__ #35190 --- .../ValidateExhaustiveDependencies.ts | 86 ++++++++----------- .../compiler/exhaustive-deps.expect.md | 58 +++++++++++-- .../fixtures/compiler/exhaustive-deps.js | 15 +++- 3 files changed, 101 insertions(+), 58 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts index 01c82a375c9..d5f9ab29323 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts @@ -43,20 +43,37 @@ import {retainWhere} from '../Utils/utils'; const DEBUG = false; /** - * Validates that existing manual memoization had exhaustive dependencies. - * Memoization with missing or extra reactive dependencies is invalid React - * and compilation can change behavior, causing a value to be computed more - * or less times. + * Validates that existing manual memoization is exhaustive and does not + * have extraneous dependencies. The goal of the validation is to ensure + * that auto-memoization will not substantially change the behavior of + * the program: + * - If the manual dependencies were non-exhaustive (missing important deps) + * then auto-memoization will include those dependencies, and cause the + * value to update *more* frequently. + * - If the manual dependencies had extraneous deps, then auto memoization + * will remove them and cause the value to update *less* frequently. * - * 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. - * - 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 - * removed by DCE, leaving a structure like: + * We consider a value V as missing if ALL of the following conditions are met: + * - V is reactive + * - There is no manual dependency path P such that whenever V would change, + * P would also change. If V is `x.y.z`, this means there must be some + * path P that is either `x.y.z`, `x.y`, or `x`. Note that we assume no + * interior mutability, such that a shorter path "covers" changes to longer + * more precise paths. + * + * We consider a value V extraneous if either of the folowing are true: + * - V is a reactive local that is unreferenced + * - V is a global that is unreferenced + * + * In other words, we allow extraneous non-reactive values since we know they cannot + * impact how often the memoization would run. + * + * ## TODO: Invalid, Complex Deps + * + * 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 + * removed by DCE, leaving a structure like: * * StartMemoize * t0 = @@ -209,31 +226,9 @@ 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); + const isRequiredDependency = reactive.has( + inferredDependency.identifier.id, + ); let hasMatchingManualDependency = false; for (const manualDependency of manualDependencies) { if ( @@ -265,18 +260,13 @@ export function validateExhaustiveDependencies( 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. + /** + * Per docblock, we only consider dependencies as extraneous if + * they are unused globals or reactive locals. Notably, this allows + * non-reactive locals. */ retainWhere(extra, dep => { - const isNonReactiveStableValue = - dep.root.kind === 'NamedLocal' && - !dep.root.value.reactive && - isStableType(dep.root.value.identifier); - return !isNonReactiveStableValue; + return dep.root.kind === 'Global' || dep.root.value.reactive; }); if (missing.length !== 0 || extra.length !== 0) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.expect.md index 655d6bff415..70d8a3abbf6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.expect.md @@ -3,7 +3,7 @@ ```javascript // @validateExhaustiveMemoizationDependencies -import {useMemo} from 'react'; +import {useCallback, useMemo} from 'react'; import {makeObject_Primitives, Stringify} from 'shared-runtime'; function useHook1(x) { @@ -47,6 +47,16 @@ function useHook6(x) { }, [x]); } +function useHook7(x) { + const [state, setState] = useState(true); + const f = () => { + setState(x => !x); + }; + return useCallback(() => { + f(); + }, [f]); +} + function Component({x, y, z}) { const a = useHook1(x); const b = useHook2(x); @@ -54,7 +64,8 @@ function Component({x, y, z}) { const d = useHook4(x, y, z); const e = useHook5(x); const f = useHook6(x); - return ; + const g = useHook7(x); + return ; } ``` @@ -63,7 +74,7 @@ function Component({x, y, z}) { ```javascript import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveMemoizationDependencies -import { useMemo } from "react"; +import { useCallback, useMemo } from "react"; import { makeObject_Primitives, Stringify } from "shared-runtime"; function useHook1(x) { @@ -121,8 +132,36 @@ function useHook6(x) { return f; } +function useHook7(x) { + const $ = _c(2); + const [, setState] = useState(true); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = () => { + setState(_temp); + }; + $[0] = t0; + } else { + t0 = $[0]; + } + const f = t0; + let t1; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + t1 = () => { + f(); + }; + $[1] = t1; + } else { + t1 = $[1]; + } + return t1; +} +function _temp(x_0) { + return !x_0; +} + function Component(t0) { - const $ = _c(7); + const $ = _c(8); const { x, y, z } = t0; const a = useHook1(x); const b = useHook2(x); @@ -130,6 +169,7 @@ function Component(t0) { const d = useHook4(x, y, z); const e = useHook5(x); const f = useHook6(x); + const g = useHook7(x); let t1; if ( $[0] !== a || @@ -137,18 +177,20 @@ function Component(t0) { $[2] !== c || $[3] !== d || $[4] !== e || - $[5] !== f + $[5] !== f || + $[6] !== g ) { - t1 = ; + t1 = ; $[0] = a; $[1] = b; $[2] = c; $[3] = d; $[4] = e; $[5] = f; - $[6] = t1; + $[6] = g; + $[7] = t1; } else { - t1 = $[6]; + t1 = $[7]; } return t1; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.js index d4c833c7d29..38e730b0d21 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.js @@ -1,5 +1,5 @@ // @validateExhaustiveMemoizationDependencies -import {useMemo} from 'react'; +import {useCallback, useMemo} from 'react'; import {makeObject_Primitives, Stringify} from 'shared-runtime'; function useHook1(x) { @@ -43,6 +43,16 @@ function useHook6(x) { }, [x]); } +function useHook7(x) { + const [state, setState] = useState(true); + const f = () => { + setState(x => !x); + }; + return useCallback(() => { + f(); + }, [f]); +} + function Component({x, y, z}) { const a = useHook1(x); const b = useHook2(x); @@ -50,5 +60,6 @@ function Component({x, y, z}) { const d = useHook4(x, y, z); const e = useHook5(x); const f = useHook6(x); - return ; + const g = useHook7(x); + return ; } From 9599e7a787cce2a41c35d783d45a160dfebab277 Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Mon, 24 Nov 2025 12:20:12 -0800 Subject: [PATCH 5/6] [compiler] Adjustments to exhaustive deps messages, disable the lint rule (#35192) Similar to ValidateHookUsage, we implement this check in the compiler for safety but (for now) continue to rely on the existing rule for actually reporting errors to users. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35192). * #35201 * #35202 * __->__ #35192 --- .../babel-plugin-react-compiler/src/CompilerError.ts | 10 +++++++++- .../src/Entrypoint/Pipeline.ts | 8 +++++--- .../src/Validation/ValidateExhaustiveDependencies.ts | 4 ++-- .../compiler/error.invalid-exhaustive-deps.expect.md | 8 ++++---- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts index 352d31b2f89..c7c8d6a161e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts @@ -1067,7 +1067,15 @@ function getRuleForCategoryImpl(category: ErrorCategory): LintRule { 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, + /** + * TODO: the "MemoDependencies" rule largely reimplements the "exhaustive-deps" non-compiler rule, + * allowing the compiler to ensure it does not regress change behavior due to different dependencies. + * We previously relied on the source having ESLint suppressions for any exhaustive-deps violations, + * but it's more reliable to verify it within the compiler. + * + * Long-term we should de-duplicate these implementations. + */ + preset: LintRulePreset.Off, }; } case ErrorCategory.IncompatibleLibrary: { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index 3d97d4ddf04..1b76dfb43e1 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -303,9 +303,11 @@ function runWithEnvironment( inferReactivePlaces(hir); log({kind: 'hir', name: 'InferReactivePlaces', value: hir}); - if (env.config.validateExhaustiveMemoizationDependencies) { - // NOTE: this relies on reactivity inference running first - validateExhaustiveDependencies(hir).unwrap(); + if (env.enableValidations) { + if (env.config.validateExhaustiveMemoizationDependencies) { + // NOTE: this relies on reactivity inference running first + validateExhaustiveDependencies(hir).unwrap(); + } } rewriteInstructionKindsBasedOnReassignment(hir); diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts index d5f9ab29323..b3a68fc0134 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts @@ -284,7 +284,7 @@ export function validateExhaustiveDependencies( if (missing.length !== 0) { const diagnostic = CompilerDiagnostic.create({ category: ErrorCategory.MemoDependencies, - reason: 'Found non-exhaustive dependencies', + reason: 'Found missing memoization dependencies', description: 'Missing dependencies can cause a value not to update when those inputs change, ' + 'resulting in stale UI', @@ -309,7 +309,7 @@ export function validateExhaustiveDependencies( 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', + 'causing performance regressions and effects to fire more often than expected', }); diagnostic.withDetails({ kind: 'error', diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md index bca95a84e1e..ed317be118e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md @@ -53,7 +53,7 @@ function Component({x, y, z}) { ``` Found 4 errors: -Error: Found non-exhaustive dependencies +Error: Found missing memoization dependencies Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI. @@ -66,7 +66,7 @@ error.invalid-exhaustive-deps.ts:7:11 9 | }, [x?.y.z?.a.b]); 10 | const b = useMemo(() => { -Error: Found non-exhaustive dependencies +Error: Found missing memoization dependencies Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI. @@ -81,7 +81,7 @@ error.invalid-exhaustive-deps.ts:15:11 Error: 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, causing performance regressions and effects to fire more often than expected. error.invalid-exhaustive-deps.ts:31:5 29 | return []; @@ -92,7 +92,7 @@ error.invalid-exhaustive-deps.ts:31:5 33 | const ref2 = useRef(null); 34 | const ref = z ? ref1 : ref2; -Error: Found non-exhaustive dependencies +Error: Found missing memoization dependencies Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI. From 16e16ec6ffe159ba831203eeeb7efe72df82c4be Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Mon, 24 Nov 2025 12:21:35 -0800 Subject: [PATCH 6/6] [compiler] Script to enable a feature by default and update tests (#35202) --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35202). * #35201 * __->__ #35202 --- compiler/.claude/settings.local.json | 9 + compiler/scripts/enable-feature-flag.js | 347 ++++++++++++++++++++++++ 2 files changed, 356 insertions(+) create mode 100644 compiler/.claude/settings.local.json create mode 100755 compiler/scripts/enable-feature-flag.js diff --git a/compiler/.claude/settings.local.json b/compiler/.claude/settings.local.json new file mode 100644 index 00000000000..9bbd18802e4 --- /dev/null +++ b/compiler/.claude/settings.local.json @@ -0,0 +1,9 @@ +{ + "permissions": { + "allow": [ + "Bash(node scripts/enable-feature-flag.js:*)" + ], + "deny": [], + "ask": [] + } +} diff --git a/compiler/scripts/enable-feature-flag.js b/compiler/scripts/enable-feature-flag.js new file mode 100755 index 00000000000..f51c5b48417 --- /dev/null +++ b/compiler/scripts/enable-feature-flag.js @@ -0,0 +1,347 @@ +#!/usr/bin/env node +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +'use strict'; + +const fs = require('fs'); +const path = require('path'); +const {execSync} = require('child_process'); +const yargs = require('yargs/yargs'); +const {hideBin} = require('yargs/helpers'); + +// Constants +const COMPILER_ROOT = path.resolve(__dirname, '..'); +const ENVIRONMENT_TS_PATH = path.join( + COMPILER_ROOT, + 'packages/babel-plugin-react-compiler/src/HIR/Environment.ts' +); +const FIXTURES_PATH = path.join( + COMPILER_ROOT, + 'packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler' +); +const FIXTURE_EXTENSIONS = ['.js', '.jsx', '.ts', '.tsx']; + +/** + * Parse command line arguments + */ +function parseArgs() { + const argv = yargs(hideBin(process.argv)) + .usage('Usage: $0 ') + .command('$0 ', 'Enable a feature flag by default', yargs => { + yargs.positional('flag-name', { + describe: 'Name of the feature flag to enable', + type: 'string', + }); + }) + .example( + '$0 validateExhaustiveMemoizationDependencies', + 'Enable the validateExhaustiveMemoizationDependencies flag' + ) + .help('h') + .alias('h', 'help') + .strict() + .parseSync(); + + return argv['flag-name']; +} + +/** + * Enable a feature flag in Environment.ts by changing default(false) to default(true) + */ +function enableFlagInEnvironment(flagName) { + console.log(`\nEnabling flag "${flagName}" in Environment.ts...`); + + const content = fs.readFileSync(ENVIRONMENT_TS_PATH, 'utf8'); + + // Check if the flag exists with default(false) + const flagPatternFalse = new RegExp( + `(${escapeRegex(flagName)}:\\s*z\\.boolean\\(\\)\\.default\\()false(\\))`, + 'g' + ); + + if (!flagPatternFalse.test(content)) { + // Check if flag exists at all + const flagExistsPattern = new RegExp( + `${escapeRegex(flagName)}:\\s*z\\.boolean\\(\\)`, + 'g' + ); + if (flagExistsPattern.test(content)) { + // Check if it's already true + const flagPatternTrue = new RegExp( + `${escapeRegex(flagName)}:\\s*z\\.boolean\\(\\)\\.default\\(true\\)`, + 'g' + ); + if (flagPatternTrue.test(content)) { + console.error(`Error: Flag "${flagName}" already has default(true)`); + process.exit(1); + } + console.error( + `Error: Flag "${flagName}" exists but doesn't have default(false)` + ); + process.exit(1); + } + console.error(`Error: Flag "${flagName}" not found in Environment.ts`); + process.exit(1); + } + + // Perform the replacement + const newContent = content.replace(flagPatternFalse, '$1true$2'); + + // Verify the replacement worked + if (content === newContent) { + console.error(`Error: Failed to replace flag "${flagName}"`); + process.exit(1); + } + + fs.writeFileSync(ENVIRONMENT_TS_PATH, newContent, 'utf8'); + console.log(`Successfully enabled "${flagName}" in Environment.ts`); +} + +/** + * Helper to escape regex special characters + */ +function escapeRegex(string) { + return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +/** + * Run yarn snap and capture output + */ +function runTests() { + console.log('\nRunning test suite (yarn snap)...'); + + try { + const output = execSync('yarn snap', { + cwd: COMPILER_ROOT, + encoding: 'utf8', + stdio: 'pipe', + maxBuffer: 10 * 1024 * 1024, // 10MB buffer + }); + return {success: true, output}; + } catch (error) { + // yarn snap exits with code 1 when tests fail, which throws an error + return {success: false, output: error.stdout || error.message}; + } +} + +/** + * Parse failing test names from test output + */ +function parseFailingTests(output) { + const failingTests = []; + + // Look for lines that contain "FAIL:" followed by the test name + // Format: "FAIL: test-name" or with ANSI codes + const lines = output.split('\n'); + for (const line of lines) { + // Remove ANSI codes for easier parsing + const cleanLine = line.replace(/\x1b\[[0-9;]*m/g, ''); + + // Match "FAIL: test-name" + const match = cleanLine.match(/^FAIL:\s*(.+)$/); + if (match) { + failingTests.push(match[1].trim()); + } + } + + return failingTests; +} + +/** + * Find the fixture file for a given test name + */ +function findFixtureFile(testName) { + const basePath = path.join(FIXTURES_PATH, testName); + + for (const ext of FIXTURE_EXTENSIONS) { + const filePath = basePath + ext; + if (fs.existsSync(filePath)) { + return filePath; + } + } + + return null; +} + +/** + * Add pragma to disable the feature flag in a fixture file + */ +function addPragmaToFixture(filePath, flagName) { + const content = fs.readFileSync(filePath, 'utf8'); + const lines = content.split('\n'); + + if (lines.length === 0) { + console.warn(`Warning: Empty file ${filePath}`); + return false; + } + + const firstLine = lines[0]; + const pragma = `@${flagName}:false`; + + // Check if pragma already exists + if (firstLine.includes(pragma)) { + return false; // Already has the pragma + } + + // Check if first line is a single-line comment + if (firstLine.trim().startsWith('//')) { + // Append pragma to existing comment + lines[0] = firstLine + ' ' + pragma; + } else if (firstLine.trim().startsWith('/*')) { + // Multi-line comment - insert new line before it + lines.unshift('// ' + pragma); + } else { + // No comment - insert new comment as first line + lines.unshift('// ' + pragma); + } + + fs.writeFileSync(filePath, lines.join('\n'), 'utf8'); + return true; +} + +/** + * Update snapshot files + */ +function updateSnapshots() { + console.log('\nUpdating snapshots (yarn snap -u)...'); + + try { + execSync('yarn snap -u', { + cwd: COMPILER_ROOT, + encoding: 'utf8', + stdio: 'pipe', + maxBuffer: 10 * 1024 * 1024, + }); + console.log('Snapshots updated successfully'); + return true; + } catch (error) { + console.error('Error updating snapshots:', error.message); + return false; + } +} + +/** + * Verify all tests pass + */ +function verifyAllTestsPass() { + console.log('\nRunning final verification (yarn snap)...'); + + const {success, output} = runTests(); + + // Parse summary line: "N Tests, N Passed, N Failed" + const summaryMatch = output.match( + /(\d+)\s+Tests,\s+(\d+)\s+Passed,\s+(\d+)\s+Failed/ + ); + + if (summaryMatch) { + const [, total, passed, failed] = summaryMatch; + console.log( + `\nTest Results: ${total} Tests, ${passed} Passed, ${failed} Failed` + ); + + if (failed === '0') { + console.log('All tests passed!'); + return true; + } else { + console.error(`${failed} tests still failing`); + const failingTests = parseFailingTests(output); + if (failingTests.length > 0) { + console.error('\nFailing tests:'); + failingTests.forEach(test => console.error(` - ${test}`)); + } + return false; + } + } + + return success; +} + +/** + * Main function + */ +async function main() { + const flagName = parseArgs(); + + console.log(`\nEnabling flag: '${flagName}'`); + + try { + // Step 1: Enable flag in Environment.ts + enableFlagInEnvironment(flagName); + + // Step 2: Run tests to find failures + const {output} = runTests(); + const failingTests = parseFailingTests(output); + + console.log(`\nFound ${failingTests.length} failing tests`); + + if (failingTests.length === 0) { + console.log('No failing tests! Feature flag enabled successfully.'); + process.exit(0); + } + + // Step 3: Add pragma to each failing fixture + console.log(`\nAdding '@${flagName}:false' pragma to failing fixtures...`); + + const notFound = []; + let notFoundCount = 0; + + for (const testName of failingTests) { + const fixturePath = findFixtureFile(testName); + + if (!fixturePath) { + console.warn(`Could not find fixture file for: ${testName}`); + notFound.push(fixturePath); + continue; + } + + const updated = addPragmaToFixture(fixturePath, flagName); + if (updated) { + updatedCount++; + console.log(` Updated: ${testName}`); + } + } + + console.log( + `\nSummary: Updated ${updatedCount} fixtures, ${notFoundCount} not found` + ); + + if (notFoundCount.length !== 0) { + console.error( + '\nFailed to update snapshots, could not find:\n' + notFound.join('\n') + ); + process.exit(1); + } + + // Step 4: Update snapshots + if (!updateSnapshots()) { + console.error('\nFailed to update snapshots'); + process.exit(1); + } + + // Step 5: Verify all tests pass + if (!verifyAllTestsPass()) { + console.error('\nVerification failed: Some tests are still failing'); + process.exit(1); + } + + console.log('\nSuccess! Feature flag enabled and all tests passing.'); + console.log(`\nSummary:`); + console.log(` - Enabled "${flagName}" in Environment.ts`); + console.log(` - Updated ${updatedCount} fixture files with pragma`); + console.log(` - All tests passing`); + + process.exit(0); + } catch (error) { + console.error('\nFatal error:', error.message); + console.error(error.stack); + process.exit(1); + } +} + +// Run the script +main();