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 92bfe913d7e..fc5ba403817 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 55bcd0bc5ce..3e4cbb93b44 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 bcfc53413ab..647562aee59 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 ca2f6e00a5d..7330b63ddce 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 b3a68fc0134..2e6217bb51a 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 19af0ed0801..9b1bf223332 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 e9475a070b8..8a3d0fac963 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 5bdeeaee1a8..9f0653d9d38 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 9994a6536f4..c3bc1d16238 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 888bdbcb8b9..686e8744077 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 ce8e7b42232..8c2b57cbad5 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 6b005c0e046..9448a284dd6 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 407fdcb0488..636bc53a172 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 5ed1a9157bd..49cf3364b1c 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 ed9f73a0163..df3524de027 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 331e551596b..b9344d663b9 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 ed317be118e..00000000000 --- 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 be22558e3c7..c4150096508 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 861dd92cf55..682c8119110 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 9b7883f6172..a812cfac3ad 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 2388488eb9e..d08f3f2f6f4 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 22e2f41f799..2558d10d19c 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 71c133b0edc..60ab46e4eb4 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 fbde27f77e6..2c2f725ec84 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 bd67fe2a9d1..f0e0b584e9f 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 92c0d5ab1a0..00000000000 --- 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