diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index 92bfe913d7e9..fc5ba4038170 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -221,7 +221,7 @@ export const EnvironmentConfigSchema = z.object({ /** * Validate that dependencies supplied to manual memoization calls are exhaustive. */ - validateExhaustiveMemoizationDependencies: z.boolean().default(false), + validateExhaustiveMemoizationDependencies: z.boolean().default(true), /** * When this is true, rather than pruning existing manual memoization but ensuring or validating diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts index 55bcd0bc5ce8..3e4cbb93b444 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -803,9 +803,11 @@ export type ManualMemoDependency = { | { kind: 'NamedLocal'; value: Place; + constant: boolean; } | {kind: 'Global'; identifierName: string}; path: DependencyPath; + loc: SourceLocation; }; export type StartMemoize = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts index bcfc53413ab4..647562aee591 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts @@ -65,6 +65,7 @@ export function collectMaybeMemoDependencies( identifierName: value.binding.name, }, path: [], + loc: value.loc, }; } case 'PropertyLoad': { @@ -74,6 +75,7 @@ export function collectMaybeMemoDependencies( root: object.root, // TODO: determine if the access is optional path: [...object.path, {property: value.property, optional}], + loc: value.loc, }; } break; @@ -92,8 +94,10 @@ export function collectMaybeMemoDependencies( root: { kind: 'NamedLocal', value: {...value.place}, + constant: false, }, path: [], + loc: value.place.loc, }; } break; diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts index ca2f6e00a5d0..7330b63ddce4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts @@ -609,6 +609,19 @@ function evaluateInstruction( constantPropagationImpl(value.loweredFunc.func, constants); return null; } + case 'StartMemoize': { + if (value.deps != null) { + for (const dep of value.deps) { + if (dep.root.kind === 'NamedLocal') { + const placeValue = read(constants, dep.root.value); + if (placeValue != null && placeValue.kind === 'Primitive') { + dep.root.constant = true; + } + } + } + } + return null; + } default: { // TODO: handle more cases return null; 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 b3a68fc0134b..2e6217bb51a5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts @@ -22,6 +22,7 @@ import { Identifier, IdentifierId, InstructionKind, + isPrimitiveType, isStableType, isSubPath, isSubPathIgnoringOptionals, @@ -53,20 +54,18 @@ const DEBUG = false; * - If the manual dependencies had extraneous deps, then auto memoization * will remove them and cause the value to update *less* frequently. * - * 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. + * The implementation compares the manual dependencies against the values + * actually used within the memoization function + * - For each value V referenced in the memo function, either: + * - If the value is non-reactive *and* a known stable type, then the + * value may optionally be specified as an exact dependency. + * - Otherwise, report an error unless there is a manual dependency that will + * invalidate whenever V invalidates. If `x.y.z` is referenced, there must + * be a manual dependency for `x.y.z`, `x.y`, or `x`. Note that we assume + * no interior mutability, ie we assume that any changes to inner paths must + * always cause the other path to change as well. + * - Any dependencies that do not correspond to a value referenced in the memo + * function are considered extraneous and throw an error * * ## TODO: Invalid, Complex Deps * @@ -226,9 +225,6 @@ export function validateExhaustiveDependencies( reason: 'Unexpected function dependency', loc: value.loc, }); - const isRequiredDependency = reactive.has( - inferredDependency.identifier.id, - ); let hasMatchingManualDependency = false; for (const manualDependency of manualDependencies) { if ( @@ -243,81 +239,140 @@ export function validateExhaustiveDependencies( ) { hasMatchingManualDependency = true; matched.add(manualDependency); - if (!isRequiredDependency) { - extra.push(manualDependency); - } } } - if (isRequiredDependency && !hasMatchingManualDependency) { - missing.push(inferredDependency); + if ( + hasMatchingManualDependency || + isOptionalDependency(inferredDependency, reactive) + ) { + continue; } + missing.push(inferredDependency); } for (const dep of startMemo.deps ?? []) { if (matched.has(dep)) { continue; } + if (dep.root.kind === 'NamedLocal' && dep.root.constant) { + CompilerError.simpleInvariant( + !dep.root.value.reactive && + isPrimitiveType(dep.root.value.identifier), + { + reason: 'Expected constant-folded dependency to be non-reactive', + loc: dep.root.value.loc, + }, + ); + /* + * Constant primitives can get constant-folded, which means we won't + * see a LoadLocal for the value within the memo function. + */ + continue; + } extra.push(dep); } - /** - * 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 => { - return dep.root.kind === 'Global' || dep.root.value.reactive; - }); - if (missing.length !== 0 || extra.length !== 0) { - let suggestions: Array | null = null; + let suggestion: CompilerSuggestion | null = null; if (startMemo.depsLoc != null && typeof startMemo.depsLoc !== 'symbol') { - suggestions = [ - { - description: 'Update dependencies', - range: [startMemo.depsLoc.start.index, startMemo.depsLoc.end.index], - op: CompilerSuggestionOperation.Replace, - text: `[${inferred.map(printInferredDependency).join(', ')}]`, - }, - ]; + suggestion = { + description: 'Update dependencies', + range: [startMemo.depsLoc.start.index, startMemo.depsLoc.end.index], + op: CompilerSuggestionOperation.Replace, + text: `[${inferred + .filter( + dep => + dep.kind === 'Local' && !isOptionalDependency(dep, reactive), + ) + .map(printInferredDependency) + .join(', ')}]`, + }; } - if (missing.length !== 0) { - const diagnostic = CompilerDiagnostic.create({ - category: ErrorCategory.MemoDependencies, - reason: 'Found missing memoization dependencies', - description: - 'Missing dependencies can cause a value not to update when those inputs change, ' + - 'resulting in stale UI', - suggestions, + const diagnostic = CompilerDiagnostic.create({ + category: ErrorCategory.MemoDependencies, + reason: 'Found missing/extra memoization dependencies', + description: [ + missing.length !== 0 + ? 'Missing dependencies can cause a value to update less often than it should, ' + + 'resulting in stale UI' + : null, + extra.length !== 0 + ? 'Extra dependencies can cause a value to update more often than it should, ' + + 'resulting in performance problems such as excessive renders or effects firing too often' + : null, + ] + .filter(Boolean) + .join('. '), + suggestions: suggestion != null ? [suggestion] : null, + }); + 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}`, + loc: dep.loc, }); - 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'; - } + } + for (const dep of extra) { + if (dep.root.kind === 'Global') { diagnostic.withDetails({ kind: 'error', - message: `Missing dependency \`${printInferredDependency(dep)}\`${reactiveStableValueHint}`, - loc: dep.loc, + message: + `Unnecessary dependency \`${printManualMemoDependency(dep)}\`. ` + + 'Values declared outside of a component/hook should not be listed as ' + + 'dependencies as the component will not re-render if they change', + loc: dep.loc ?? startMemo.depsLoc ?? value.loc, }); + error.pushDiagnostic(diagnostic); + } else { + const root = dep.root.value; + const matchingInferred = inferred.find( + ( + inferredDep, + ): inferredDep is Extract => { + return ( + inferredDep.kind === 'Local' && + inferredDep.identifier.id === root.identifier.id && + isSubPathIgnoringOptionals(inferredDep.path, dep.path) + ); + }, + ); + if ( + matchingInferred != null && + !isOptionalDependency(matchingInferred, reactive) + ) { + diagnostic.withDetails({ + kind: 'error', + message: + `Overly precise dependency \`${printManualMemoDependency(dep)}\`, ` + + `use \`${printInferredDependency(matchingInferred)}\` instead`, + loc: dep.loc ?? startMemo.depsLoc ?? value.loc, + }); + } else { + /** + * Else this dependency doesn't correspond to anything referenced in the memo function, + * or is an optional dependency so we don't want to suggest adding it + */ + diagnostic.withDetails({ + kind: 'error', + message: `Unnecessary dependency \`${printManualMemoDependency(dep)}\``, + loc: dep.loc ?? startMemo.depsLoc ?? value.loc, + }); + } } - error.pushDiagnostic(diagnostic); - } else if (extra.length !== 0) { - const diagnostic = CompilerDiagnostic.create({ - category: ErrorCategory.MemoDependencies, - reason: 'Found unnecessary memoization dependencies', - description: - 'Unnecessary dependencies can cause a value to update more often than necessary, ' + - 'causing performance regressions and effects to fire more often than expected', - }); + } + if (suggestion != null) { diagnostic.withDetails({ - kind: 'error', - message: `Unnecessary dependencies ${extra.map(dep => `\`${printManualMemoDependency(dep)}\``).join(', ')}`, - loc: startMemo.depsLoc ?? value.loc, + kind: 'hint', + message: `Inferred dependencies: \`${suggestion.text}\``, }); - error.pushDiagnostic(diagnostic); } + error.pushDiagnostic(diagnostic); } dependencies.clear(); @@ -822,3 +877,14 @@ export function findOptionalPlaces( } return optionals; } + +function isOptionalDependency( + inferredDependency: Extract, + reactive: Set, +): boolean { + return ( + !reactive.has(inferredDependency.identifier.id) && + (isStableType(inferredDependency.identifier) || + isPrimitiveType(inferredDependency.identifier)) + ); +} 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 19af0ed08014..9b1bf223332c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts @@ -242,6 +242,7 @@ function validateInferredDep( normalizedDep = { root: maybeNormalizedRoot.root, path: [...maybeNormalizedRoot.path, ...dep.path], + loc: maybeNormalizedRoot.loc, }; } else { CompilerError.invariant(dep.identifier.name?.kind === 'named', { @@ -267,8 +268,10 @@ function validateInferredDep( effect: Effect.Read, reactive: false, }, + constant: false, }, path: [...dep.path], + loc: GeneratedSource, }; } for (const decl of declsWithinMemoBlock) { @@ -379,8 +382,10 @@ class Visitor extends ReactiveFunctionVisitor { root: { kind: 'NamedLocal', value: storeTarget, + constant: false, }, path: [], + loc: storeTarget.loc, }); } } @@ -408,8 +413,10 @@ class Visitor extends ReactiveFunctionVisitor { root: { kind: 'NamedLocal', value: {...lvalue}, + constant: false, }, path: [], + loc: lvalue.loc, }); } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.expect.md index e9475a070b88..8a3d0fac9637 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @enablePreserveExistingMemoizationGuarantees:false +// @enablePreserveExistingMemoizationGuarantees:false @validateExhaustiveMemoizationDependencies:false import {useMemo} from 'react'; const someGlobal = {value: 0}; @@ -33,7 +33,7 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; // @enablePreserveExistingMemoizationGuarantees:false +import { c as _c } from "react/compiler-runtime"; // @enablePreserveExistingMemoizationGuarantees:false @validateExhaustiveMemoizationDependencies:false import { useMemo } from "react"; const someGlobal = { value: 0 }; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.js index 5bdeeaee1a8e..9f0653d9d382 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.js @@ -1,4 +1,4 @@ -// @enablePreserveExistingMemoizationGuarantees:false +// @enablePreserveExistingMemoizationGuarantees:false @validateExhaustiveMemoizationDependencies:false import {useMemo} from 'react'; const someGlobal = {value: 0}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-pattern-spread-creates-array.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-pattern-spread-creates-array.expect.md index 9994a6536f41..c3bc1d162385 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-pattern-spread-creates-array.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-pattern-spread-creates-array.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @validatePreserveExistingMemoizationGuarantees +// @validatePreserveExistingMemoizationGuarantees @validateExhaustiveMemoizationDependencies:false import {useMemo} from 'react'; import {makeObject_Primitives, ValidateMemoization} from 'shared-runtime'; @@ -36,7 +36,7 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees +import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees @validateExhaustiveMemoizationDependencies:false import { useMemo } from "react"; import { makeObject_Primitives, ValidateMemoization } from "shared-runtime"; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-pattern-spread-creates-array.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-pattern-spread-creates-array.js index 888bdbcb8b93..686e87440776 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-pattern-spread-creates-array.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-pattern-spread-creates-array.js @@ -1,4 +1,4 @@ -// @validatePreserveExistingMemoizationGuarantees +// @validatePreserveExistingMemoizationGuarantees @validateExhaustiveMemoizationDependencies:false import {useMemo} from 'react'; import {makeObject_Primitives, ValidateMemoization} from 'shared-runtime'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.expect.md index ce8e7b422329..8c2b57cbad5f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +// @validateExhaustiveMemoizationDependencies:false import {useMemo} from 'react'; function Component(props) { @@ -30,7 +31,7 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; +import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveMemoizationDependencies:false import { useMemo } from "react"; function Component(props) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.js index 6b005c0e0461..9448a284dd63 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.js @@ -1,3 +1,4 @@ +// @validateExhaustiveMemoizationDependencies:false import {useMemo} from 'react'; function Component(props) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/context-variable-as-jsx-element-tag.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/context-variable-as-jsx-element-tag.expect.md index 407fdcb0488f..636bc53a172e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/context-variable-as-jsx-element-tag.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/context-variable-as-jsx-element-tag.expect.md @@ -11,7 +11,7 @@ function Component(props) { Component = useMemo(() => { return Component; - }); + }, [Component]); return ; } @@ -36,6 +36,7 @@ function Component(props) { if ($[0] === Symbol.for("react.memo_cache_sentinel")) { Component = Stringify; + Component; Component = Component; $[0] = Component; } else { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/context-variable-as-jsx-element-tag.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/context-variable-as-jsx-element-tag.js index 5ed1a9157bdf..49cf3364b1c4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/context-variable-as-jsx-element-tag.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/context-variable-as-jsx-element-tag.js @@ -7,7 +7,7 @@ function Component(props) { Component = useMemo(() => { return Component; - }); + }, [Component]); return ; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bailout-on-suppression-of-custom-rule.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bailout-on-suppression-of-custom-rule.expect.md index ed9f73a01637..df3524de027e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bailout-on-suppression-of-custom-rule.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bailout-on-suppression-of-custom-rule.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @eslintSuppressionRules:["my-app","react-rule"] +// @eslintSuppressionRules:["my-app","react-rule"] @validateExhaustiveMemoizationDependencies:false /* eslint-disable my-app/react-rule */ function lowercasecomponent() { @@ -26,7 +26,7 @@ Error: React Compiler has skipped optimizing this component because one or more React Compiler only works when your components follow all the rules of React, disabling them may result in unexpected or incorrect behavior. Found suppression `eslint-disable my-app/react-rule`. error.bailout-on-suppression-of-custom-rule.ts:3:0 - 1 | // @eslintSuppressionRules:["my-app","react-rule"] + 1 | // @eslintSuppressionRules:["my-app","react-rule"] @validateExhaustiveMemoizationDependencies:false 2 | > 3 | /* eslint-disable my-app/react-rule */ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Found React rule suppression diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bailout-on-suppression-of-custom-rule.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bailout-on-suppression-of-custom-rule.js index 331e551596b7..b9344d663b90 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bailout-on-suppression-of-custom-rule.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bailout-on-suppression-of-custom-rule.js @@ -1,4 +1,4 @@ -// @eslintSuppressionRules:["my-app","react-rule"] +// @eslintSuppressionRules:["my-app","react-rule"] @validateExhaustiveMemoizationDependencies:false /* eslint-disable my-app/react-rule */ function lowercasecomponent() { 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 deleted file mode 100644 index ed317be118e1..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md +++ /dev/null @@ -1,109 +0,0 @@ - -## Input - -```javascript -// @validateExhaustiveMemoizationDependencies -import {useMemo} from 'react'; -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); - const ref = z ? ref1 : ref2; - const cb = useMemo(() => { - return () => { - return ref.current; - }; - // error: ref is a stable type but reactive - }, []); - return ; -} - -``` - - -## Error - -``` -Found 4 errors: - -Error: Found missing memoization dependencies - -Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI. - -error.invalid-exhaustive-deps.ts:7:11 - 5 | function Component({x, y, z}) { - 6 | const a = useMemo(() => { -> 7 | return x?.y.z?.a; - | ^^^^^^^^^ Missing dependency `x?.y.z?.a` - 8 | // error: too precise - 9 | }, [x?.y.z?.a.b]); - 10 | const b = useMemo(() => { - -Error: Found missing memoization dependencies - -Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI. - -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` - 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, causing performance regressions and effects to fire more often than expected. - -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` - 32 | const ref1 = useRef(null); - 33 | const ref2 = useRef(null); - 34 | const ref = z ? ref1 : ref2; - -Error: Found missing memoization dependencies - -Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI. - -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 - 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-sketchy-code-use-forget.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-sketchy-code-use-forget.expect.md index be22558e3c71..c41500965080 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-sketchy-code-use-forget.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-sketchy-code-use-forget.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +// @validateExhaustiveMemoizationDependencies:false /* eslint-disable react-hooks/rules-of-hooks */ function lowercasecomponent() { 'use forget'; @@ -23,25 +24,26 @@ Error: React Compiler has skipped optimizing this component because one or more React Compiler only works when your components follow all the rules of React, disabling them may result in unexpected or incorrect behavior. Found suppression `eslint-disable react-hooks/rules-of-hooks`. -error.invalid-sketchy-code-use-forget.ts:1:0 -> 1 | /* eslint-disable react-hooks/rules-of-hooks */ +error.invalid-sketchy-code-use-forget.ts:2:0 + 1 | // @validateExhaustiveMemoizationDependencies:false +> 2 | /* eslint-disable react-hooks/rules-of-hooks */ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Found React rule suppression - 2 | function lowercasecomponent() { - 3 | 'use forget'; - 4 | const x = []; + 3 | function lowercasecomponent() { + 4 | 'use forget'; + 5 | const x = []; Error: React Compiler has skipped optimizing this component because one or more React ESLint rules were disabled React Compiler only works when your components follow all the rules of React, disabling them may result in unexpected or incorrect behavior. Found suppression `eslint-disable-next-line react-hooks/rules-of-hooks`. -error.invalid-sketchy-code-use-forget.ts:5:2 - 3 | 'use forget'; - 4 | const x = []; -> 5 | // eslint-disable-next-line react-hooks/rules-of-hooks +error.invalid-sketchy-code-use-forget.ts:6:2 + 4 | 'use forget'; + 5 | const x = []; +> 6 | // eslint-disable-next-line react-hooks/rules-of-hooks | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Found React rule suppression - 6 | return
{x}
; - 7 | } - 8 | /* eslint-enable react-hooks/rules-of-hooks */ + 7 | return
{x}
; + 8 | } + 9 | /* eslint-enable react-hooks/rules-of-hooks */ ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-sketchy-code-use-forget.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-sketchy-code-use-forget.js index 861dd92cf55c..682c81191109 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-sketchy-code-use-forget.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-sketchy-code-use-forget.js @@ -1,3 +1,4 @@ +// @validateExhaustiveMemoizationDependencies:false /* eslint-disable react-hooks/rules-of-hooks */ function lowercasecomponent() { 'use forget'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unclosed-eslint-suppression.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unclosed-eslint-suppression.expect.md index 9b7883f61722..a812cfac3ad6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unclosed-eslint-suppression.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unclosed-eslint-suppression.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// Note: Everything below this is sketchy +// Note: Everything below this is sketchy @validateExhaustiveMemoizationDependencies:false /* eslint-disable react-hooks/rules-of-hooks */ function lowercasecomponent() { 'use forget'; @@ -43,7 +43,7 @@ Error: React Compiler has skipped optimizing this component because one or more React Compiler only works when your components follow all the rules of React, disabling them may result in unexpected or incorrect behavior. Found suppression `eslint-disable react-hooks/rules-of-hooks`. error.invalid-unclosed-eslint-suppression.ts:2:0 - 1 | // Note: Everything below this is sketchy + 1 | // Note: Everything below this is sketchy @validateExhaustiveMemoizationDependencies:false > 2 | /* eslint-disable react-hooks/rules-of-hooks */ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Found React rule suppression 3 | function lowercasecomponent() { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unclosed-eslint-suppression.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unclosed-eslint-suppression.js index 2388488eb9e3..d08f3f2f6f4a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unclosed-eslint-suppression.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unclosed-eslint-suppression.js @@ -1,4 +1,4 @@ -// Note: Everything below this is sketchy +// Note: Everything below this is sketchy @validateExhaustiveMemoizationDependencies:false /* eslint-disable react-hooks/rules-of-hooks */ function lowercasecomponent() { 'use forget'; 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 22e2f41f7999..2558d10d19cb 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 @@ -2,7 +2,7 @@ ## Input ```javascript -// @validatePreserveExistingMemoizationGuarantees +// @validatePreserveExistingMemoizationGuarantees @validateExhaustiveMemoizationDependencies:false import {useCallback, useRef} from 'react'; function useCustomRef() { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-Ref.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-Ref.js index 71c133b0edcf..60ab46e4eb47 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-Ref.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-Ref.js @@ -1,4 +1,4 @@ -// @validatePreserveExistingMemoizationGuarantees +// @validatePreserveExistingMemoizationGuarantees @validateExhaustiveMemoizationDependencies:false import {useCallback, useRef} from 'react'; function useCustomRef() { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-a-ref.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-a-ref.expect.md index fbde27f77e62..2c2f725ec84b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-a-ref.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-a-ref.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @validatePreserveExistingMemoizationGuarantees +// @validatePreserveExistingMemoizationGuarantees @validateExhaustiveMemoizationDependencies:false import {useCallback, useRef} from 'react'; function useCustomRef() { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-a-ref.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-a-ref.js index bd67fe2a9d15..f0e0b584e9fc 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-a-ref.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-a-ref.js @@ -1,4 +1,4 @@ -// @validatePreserveExistingMemoizationGuarantees +// @validatePreserveExistingMemoizationGuarantees @validateExhaustiveMemoizationDependencies:false import {useCallback, useRef} from 'react'; function useCustomRef() { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.sketchy-code-exhaustive-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.sketchy-code-exhaustive-deps.expect.md deleted file mode 100644 index 92c0d5ab1a09..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.sketchy-code-exhaustive-deps.expect.md +++ /dev/null @@ -1,39 +0,0 @@ - -## Input - -```javascript -function Component() { - const item = []; - const foo = useCallback( - () => { - item.push(1); - }, // eslint-disable-next-line react-hooks/exhaustive-deps - [] - ); - - return