Compare commits
8 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
290174c38f | ||
|
|
67c1487ffd | ||
|
|
454e01e603 | ||
|
|
c9a8cf3411 | ||
|
|
fca172e3f3 | ||
|
|
fd524fe02a | ||
|
|
40b4a5bf71 | ||
|
|
df75af4edc |
@@ -600,7 +600,8 @@ function printErrorSummary(category: ErrorCategory, message: string): string {
|
||||
case ErrorCategory.Suppression:
|
||||
case ErrorCategory.Syntax:
|
||||
case ErrorCategory.UseMemo:
|
||||
case ErrorCategory.VoidUseMemo: {
|
||||
case ErrorCategory.VoidUseMemo:
|
||||
case ErrorCategory.MemoDependencies: {
|
||||
heading = 'Error';
|
||||
break;
|
||||
}
|
||||
@@ -658,6 +659,10 @@ export enum ErrorCategory {
|
||||
* Checks that manual memoization is preserved
|
||||
*/
|
||||
PreserveManualMemo = 'PreserveManualMemo',
|
||||
/**
|
||||
* Checks for exhaustive useMemo/useCallback dependencies without extraneous values
|
||||
*/
|
||||
MemoDependencies = 'MemoDependencies',
|
||||
/**
|
||||
* Checks for known incompatible libraries
|
||||
*/
|
||||
@@ -1055,6 +1060,24 @@ function getRuleForCategoryImpl(category: ErrorCategory): LintRule {
|
||||
preset: LintRulePreset.RecommendedLatest,
|
||||
};
|
||||
}
|
||||
case ErrorCategory.MemoDependencies: {
|
||||
return {
|
||||
category,
|
||||
severity: ErrorSeverity.Error,
|
||||
name: 'memo-dependencies',
|
||||
description:
|
||||
'Validates that useMemo() and useCallback() specify comprehensive dependencies without extraneous values. See [`useMemo()` docs](https://react.dev/reference/react/useMemo) for more information.',
|
||||
/**
|
||||
* 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: {
|
||||
return {
|
||||
category,
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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,
|
||||
);
|
||||
|
||||
|
||||
@@ -78,7 +78,7 @@ export function filterSuppressionsThatAffectFunction(
|
||||
|
||||
export function findProgramSuppressions(
|
||||
programComments: Array<t.Comment>,
|
||||
ruleNames: Array<string>,
|
||||
ruleNames: Array<string> | null,
|
||||
flowSuppressions: boolean,
|
||||
): Array<SuppressionRange> {
|
||||
const suppressionRanges: Array<SuppressionRange> = [];
|
||||
@@ -89,7 +89,7 @@ export function findProgramSuppressions(
|
||||
let disableNextLinePattern: RegExp | null = null;
|
||||
let disablePattern: RegExp | null = null;
|
||||
let enablePattern: RegExp | null = null;
|
||||
if (ruleNames.length !== 0) {
|
||||
if (ruleNames != null && ruleNames.length !== 0) {
|
||||
const rulePattern = `(${ruleNames.join('|')})`;
|
||||
disableNextLinePattern = new RegExp(
|
||||
`eslint-disable-next-line ${rulePattern}`,
|
||||
|
||||
@@ -22,7 +22,10 @@ import {
|
||||
Identifier,
|
||||
IdentifierId,
|
||||
InstructionKind,
|
||||
isStableType,
|
||||
isSubPath,
|
||||
isSubPathIgnoringOptionals,
|
||||
isUseRefType,
|
||||
LoadGlobal,
|
||||
ManualMemoDependency,
|
||||
Place,
|
||||
@@ -40,19 +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:
|
||||
* - Better handling of cases where we infer multiple dependencies related to a single
|
||||
* variable. Eg if the user has dep `x` and we inferred `x.y, x.z`, the user's dep
|
||||
* is sufficient.
|
||||
* - Handle cases where the user deps were not simple identifiers + property chains.
|
||||
* We try to detect this in ValidateUseMemo but we miss some cases. The problem
|
||||
* is that invalid forms can be value blocks or function calls that don't get
|
||||
* 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 = <value to memoize>
|
||||
@@ -108,7 +129,7 @@ export function validateExhaustiveDependencies(
|
||||
);
|
||||
visitCandidateDependency(value.decl, temporaries, dependencies, locals);
|
||||
const inferred: Array<InferredDependency> = Array.from(dependencies);
|
||||
// Sort dependencies by name, and path, with shorter/non-optional paths first
|
||||
// Sort dependencies by name and path, with shorter/non-optional paths first
|
||||
inferred.sort((a, b) => {
|
||||
if (a.kind === 'Global' && b.kind == 'Global') {
|
||||
return a.binding.name.localeCompare(b.binding.name);
|
||||
@@ -205,6 +226,9 @@ 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 (
|
||||
@@ -212,28 +236,39 @@ 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);
|
||||
if (!isRequiredDependency) {
|
||||
extra.push(manualDependency);
|
||||
}
|
||||
}
|
||||
}
|
||||
if (!hasMatchingManualDependency) {
|
||||
if (isRequiredDependency && !hasMatchingManualDependency) {
|
||||
missing.push(inferredDependency);
|
||||
}
|
||||
}
|
||||
|
||||
for (const dep of startMemo.deps ?? []) {
|
||||
if (
|
||||
matched.has(dep) ||
|
||||
(dep.root.kind === 'NamedLocal' &&
|
||||
!reactive.has(dep.root.value.identifier.id))
|
||||
) {
|
||||
if (matched.has(dep)) {
|
||||
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<CompilerSuggestion> | null = null;
|
||||
if (startMemo.depsLoc != null && typeof startMemo.depsLoc !== 'symbol') {
|
||||
@@ -247,36 +282,39 @@ export function validateExhaustiveDependencies(
|
||||
];
|
||||
}
|
||||
if (missing.length !== 0) {
|
||||
// Error
|
||||
const diagnostic = CompilerDiagnostic.create({
|
||||
category: ErrorCategory.PreserveManualMemo,
|
||||
reason: 'Found non-exhaustive dependencies',
|
||||
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. This memoization cannot be safely rewritten by the compiler.',
|
||||
'resulting in stale UI',
|
||||
suggestions,
|
||||
});
|
||||
for (const dep of missing) {
|
||||
let reactiveStableValueHint = '';
|
||||
if (isStableType(dep.identifier)) {
|
||||
reactiveStableValueHint =
|
||||
'. Refs, setState functions, and other "stable" values generally do not need to be added as dependencies, but this variable may change over time to point to different values';
|
||||
}
|
||||
diagnostic.withDetails({
|
||||
kind: 'error',
|
||||
message: `Missing dependency \`${printInferredDependency(dep)}\``,
|
||||
message: `Missing dependency \`${printInferredDependency(dep)}\`${reactiveStableValueHint}`,
|
||||
loc: dep.loc,
|
||||
});
|
||||
}
|
||||
error.pushDiagnostic(diagnostic);
|
||||
} else if (extra.length !== 0) {
|
||||
const diagnostic = CompilerDiagnostic.create({
|
||||
category: ErrorCategory.PreserveManualMemo,
|
||||
category: ErrorCategory.MemoDependencies,
|
||||
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. This memoization cannot be safely ' +
|
||||
'rewritten by the compiler',
|
||||
'causing performance regressions and effects to fire more often than expected',
|
||||
});
|
||||
diagnostic.withDetails({
|
||||
kind: 'error',
|
||||
message: `Unnecessary dependencies ${extra.map(dep => `\`${printManualMemoDependency(dep)}\``).join(', ')}`,
|
||||
loc: value.loc,
|
||||
loc: startMemo.depsLoc ?? value.loc,
|
||||
});
|
||||
error.pushDiagnostic(diagnostic);
|
||||
}
|
||||
@@ -287,10 +325,15 @@ export function validateExhaustiveDependencies(
|
||||
startMemo = null;
|
||||
}
|
||||
|
||||
collectDependencies(fn, temporaries, {
|
||||
onStartMemoize,
|
||||
onFinishMemoize,
|
||||
});
|
||||
collectDependencies(
|
||||
fn,
|
||||
temporaries,
|
||||
{
|
||||
onStartMemoize,
|
||||
onFinishMemoize,
|
||||
},
|
||||
false, // isFunctionExpression
|
||||
);
|
||||
return error.asResult();
|
||||
}
|
||||
|
||||
@@ -383,12 +426,20 @@ function collectDependencies(
|
||||
locals: Set<IdentifierId>,
|
||||
) => void;
|
||||
} | null,
|
||||
isFunctionExpression: boolean,
|
||||
): Extract<Temporary, {kind: 'Function'}> {
|
||||
const optionals = findOptionalPlaces(fn);
|
||||
if (DEBUG) {
|
||||
console.log(prettyFormat(optionals));
|
||||
}
|
||||
const locals: Set<IdentifierId> = new Set();
|
||||
if (isFunctionExpression) {
|
||||
for (const param of fn.params) {
|
||||
const place = param.kind === 'Identifier' ? param : param.place;
|
||||
locals.add(place.identifier.id);
|
||||
}
|
||||
}
|
||||
|
||||
const dependencies: Set<InferredDependency> = new Set();
|
||||
function visit(place: Place): void {
|
||||
visitCandidateDependency(place, temporaries, dependencies, locals);
|
||||
@@ -523,7 +574,11 @@ function collectDependencies(
|
||||
break;
|
||||
}
|
||||
case 'PropertyLoad': {
|
||||
if (typeof value.property === 'number') {
|
||||
if (
|
||||
typeof value.property === 'number' ||
|
||||
(isUseRefType(value.object.identifier) &&
|
||||
value.property === 'current')
|
||||
) {
|
||||
visit(value.object);
|
||||
break;
|
||||
}
|
||||
@@ -553,6 +608,7 @@ function collectDependencies(
|
||||
value.loweredFunc.func,
|
||||
temporaries,
|
||||
null,
|
||||
true, // isFunctionExpression
|
||||
);
|
||||
temporaries.set(lvalue.identifier.id, functionDeps);
|
||||
addDependency(functionDeps, dependencies, locals);
|
||||
|
||||
@@ -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 <ValidateMemoization inputs={[x]} output={memo} />;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
## Code
|
||||
|
||||
```javascript
|
||||
import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveMemoizationDependencies
|
||||
|
||||
import { useMemo } from "react";
|
||||
import { ValidateMemoization } from "shared-runtime";
|
||||
|
||||
function Component(t0) {
|
||||
const $ = _c(10);
|
||||
const { x } = t0;
|
||||
let t1;
|
||||
if ($[0] !== x) {
|
||||
t1 = () => {
|
||||
console.log(x);
|
||||
};
|
||||
$[0] = x;
|
||||
$[1] = t1;
|
||||
} else {
|
||||
t1 = $[1];
|
||||
}
|
||||
let t2;
|
||||
if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
|
||||
t2 = [];
|
||||
$[2] = t2;
|
||||
} else {
|
||||
t2 = $[2];
|
||||
}
|
||||
useEffect(t1, t2);
|
||||
let t3;
|
||||
if ($[3] !== x) {
|
||||
t3 = [x];
|
||||
$[3] = x;
|
||||
$[4] = t3;
|
||||
} else {
|
||||
t3 = $[4];
|
||||
}
|
||||
const memo = t3;
|
||||
let t4;
|
||||
if ($[5] !== x) {
|
||||
t4 = [x];
|
||||
$[5] = x;
|
||||
$[6] = t4;
|
||||
} else {
|
||||
t4 = $[6];
|
||||
}
|
||||
let t5;
|
||||
if ($[7] !== memo || $[8] !== t4) {
|
||||
t5 = <ValidateMemoization inputs={t4} output={memo} />;
|
||||
$[7] = memo;
|
||||
$[8] = t4;
|
||||
$[9] = t5;
|
||||
} else {
|
||||
t5 = $[9];
|
||||
}
|
||||
return t5;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
### Eval output
|
||||
(kind: exception) Fixture not implemented
|
||||
@@ -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 <ValidateMemoization inputs={[x]} output={memo} />;
|
||||
}
|
||||
@@ -9,25 +9,40 @@ 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 [];
|
||||
}, [x, y.z, z?.y?.a]);
|
||||
return <Stringify results={[a, b, c, d, e, f]} />;
|
||||
// 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 <Stringify results={[a, b, c, d, e, f, cb]} />;
|
||||
}
|
||||
|
||||
```
|
||||
@@ -38,61 +53,57 @@ function Component({x, y, z}) {
|
||||
```
|
||||
Found 4 errors:
|
||||
|
||||
Compilation Skipped: 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. This memoization cannot be safely rewritten by the compiler..
|
||||
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 | }, [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(() => {
|
||||
|
||||
Compilation Skipped: 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. This memoization cannot be safely rewritten by the compiler..
|
||||
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;
|
||||
|
||||
Compilation Skipped: Found non-exhaustive dependencies
|
||||
|
||||
Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI. This memoization cannot be safely rewritten by the compiler..
|
||||
|
||||
error.invalid-exhaustive-deps.ts:13:11
|
||||
11 | }, [x.y.z.a]);
|
||||
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(() => {
|
||||
|
||||
Compilation Skipped: Found unnecessary memoization dependencies
|
||||
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. This memoization cannot be safely rewritten by the compiler.
|
||||
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:23:20
|
||||
21 | return e;
|
||||
22 | }, [x]);
|
||||
> 23 | const f = useMemo(() => {
|
||||
| ^^^^^^^
|
||||
> 24 | return [];
|
||||
| ^^^^^^^^^^^^^^
|
||||
> 25 | }, [x, y.z, z?.y?.a]);
|
||||
| ^^^^ Unnecessary dependencies `x`, `y.z`, `z?.y?.a`
|
||||
26 | return <Stringify results={[a, b, c, d, e, f]} />;
|
||||
27 | }
|
||||
28 |
|
||||
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 | }, []);
|
||||
```
|
||||
|
||||
|
||||
@@ -5,23 +5,38 @@ 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 [];
|
||||
}, [x, y.z, z?.y?.a]);
|
||||
return <Stringify results={[a, b, c, d, e, f]} />;
|
||||
// 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 <Stringify results={[a, b, c, d, e, f, cb]} />;
|
||||
}
|
||||
|
||||
@@ -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 ]]"
|
||||
@@ -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: [],
|
||||
};
|
||||
@@ -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 <Stringify results={[a, b, c, d, e, f]} />;
|
||||
const g = useHook7(x);
|
||||
return <Stringify results={[a, b, c, d, e, f, g]} />;
|
||||
}
|
||||
|
||||
```
|
||||
@@ -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 = <Stringify results={[a, b, c, d, e, f]} />;
|
||||
t1 = <Stringify results={[a, b, c, d, e, f, g]} />;
|
||||
$[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;
|
||||
}
|
||||
|
||||
@@ -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 <Stringify results={[a, b, c, d, e, f]} />;
|
||||
const g = useHook7(x);
|
||||
return <Stringify results={[a, b, c, d, e, f, g]} />;
|
||||
}
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -584,4 +584,75 @@ describe('ProfilerContext', () => {
|
||||
await utils.actAsync(() => context.selectFiber(childID, 'Child'));
|
||||
expect(inspectedElementID).toBe(parentID);
|
||||
});
|
||||
|
||||
it('should toggle profiling when the keyboard shortcut is pressed', async () => {
|
||||
// Context providers
|
||||
const Profiler =
|
||||
require('react-devtools-shared/src/devtools/views/Profiler/Profiler').default;
|
||||
const {
|
||||
TimelineContextController,
|
||||
} = require('react-devtools-timeline/src/TimelineContext');
|
||||
const {
|
||||
SettingsContextController,
|
||||
} = require('react-devtools-shared/src/devtools/views/Settings/SettingsContext');
|
||||
const {
|
||||
ModalDialogContextController,
|
||||
} = require('react-devtools-shared/src/devtools/views/ModalDialog');
|
||||
|
||||
// Dom component for profiling to be enabled
|
||||
const Component = () => null;
|
||||
utils.act(() => render(<Component />));
|
||||
|
||||
const profilerContainer = document.createElement('div');
|
||||
document.body.appendChild(profilerContainer);
|
||||
|
||||
// Create a root for the profiler
|
||||
const profilerRoot = ReactDOMClient.createRoot(profilerContainer);
|
||||
|
||||
// Render the profiler
|
||||
utils.act(() => {
|
||||
profilerRoot.render(
|
||||
<Contexts>
|
||||
<SettingsContextController browserTheme="light">
|
||||
<ModalDialogContextController>
|
||||
<TimelineContextController>
|
||||
<Profiler />
|
||||
</TimelineContextController>
|
||||
</ModalDialogContextController>
|
||||
</SettingsContextController>
|
||||
</Contexts>,
|
||||
);
|
||||
});
|
||||
|
||||
// Verify that the profiler is not profiling.
|
||||
expect(store.profilerStore.isProfilingBasedOnUserInput).toBe(false);
|
||||
|
||||
// Trigger the keyboard shortcut.
|
||||
const ownerWindow = profilerContainer.ownerDocument.defaultView;
|
||||
const isMac =
|
||||
typeof navigator !== 'undefined' &&
|
||||
navigator.platform.toUpperCase().indexOf('MAC') >= 0;
|
||||
|
||||
const keyEvent = new KeyboardEvent('keydown', {
|
||||
key: 'e',
|
||||
metaKey: isMac,
|
||||
ctrlKey: !isMac,
|
||||
bubbles: true,
|
||||
});
|
||||
|
||||
// Dispatch keyboard event to toggle profiling on
|
||||
// Try utils.actAsync with recursivelyFlush=false
|
||||
await utils.actAsync(() => {
|
||||
ownerWindow.dispatchEvent(keyEvent);
|
||||
}, false);
|
||||
expect(store.profilerStore.isProfilingBasedOnUserInput).toBe(true);
|
||||
|
||||
// Dispatch keyboard event to toggle profiling off
|
||||
await utils.actAsync(() => {
|
||||
ownerWindow.dispatchEvent(keyEvent);
|
||||
}, false);
|
||||
expect(store.profilerStore.isProfilingBasedOnUserInput).toBe(false);
|
||||
|
||||
document.body.removeChild(profilerContainer);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
*/
|
||||
|
||||
import * as React from 'react';
|
||||
import {Fragment, useContext} from 'react';
|
||||
import {Fragment, useContext, useEffect, useRef, useEffectEvent} from 'react';
|
||||
import {ModalDialog} from '../ModalDialog';
|
||||
import {ProfilerContext} from './ProfilerContext';
|
||||
import TabBar from '../TabBar';
|
||||
@@ -38,6 +38,11 @@ import {TimelineContext} from 'react-devtools-timeline/src/TimelineContext';
|
||||
import styles from './Profiler.css';
|
||||
|
||||
function Profiler(_: {}) {
|
||||
const profilerRef = useRef<HTMLDivElement | null>(null);
|
||||
const isMac =
|
||||
typeof navigator !== 'undefined' &&
|
||||
navigator.platform.toUpperCase().indexOf('MAC') >= 0;
|
||||
|
||||
const {
|
||||
didRecordCommits,
|
||||
isProcessingData,
|
||||
@@ -47,6 +52,8 @@ function Profiler(_: {}) {
|
||||
selectedTabID,
|
||||
selectTab,
|
||||
supportsProfiling,
|
||||
startProfiling,
|
||||
stopProfiling,
|
||||
} = useContext(ProfilerContext);
|
||||
|
||||
const {file: timelineTraceEventData, searchInputContainerRef} =
|
||||
@@ -56,6 +63,32 @@ function Profiler(_: {}) {
|
||||
|
||||
const isLegacyProfilerSelected = selectedTabID !== 'timeline';
|
||||
|
||||
// Cmd+E to start/stop profiler recording
|
||||
const handleKeyDown = useEffectEvent((event: KeyboardEvent) => {
|
||||
const correctModifier = isMac ? event.metaKey : event.ctrlKey;
|
||||
if (correctModifier && event.key === 'e') {
|
||||
if (isProfiling) {
|
||||
stopProfiling();
|
||||
} else {
|
||||
startProfiling();
|
||||
}
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
}
|
||||
});
|
||||
|
||||
useEffect(() => {
|
||||
const div = profilerRef.current;
|
||||
if (!div) {
|
||||
return;
|
||||
}
|
||||
const ownerWindow = div.ownerDocument.defaultView;
|
||||
ownerWindow.addEventListener('keydown', handleKeyDown);
|
||||
return () => {
|
||||
ownerWindow.removeEventListener('keydown', handleKeyDown);
|
||||
};
|
||||
}, []);
|
||||
|
||||
let view = null;
|
||||
if (didRecordCommits || selectedTabID === 'timeline') {
|
||||
switch (selectedTabID) {
|
||||
@@ -112,7 +145,7 @@ function Profiler(_: {}) {
|
||||
|
||||
return (
|
||||
<SettingsModalContextController>
|
||||
<div className={styles.Profiler}>
|
||||
<div ref={profilerRef} className={styles.Profiler}>
|
||||
<div className={styles.LeftColumn}>
|
||||
<div className={styles.Toolbar}>
|
||||
<RecordToggle disabled={!supportsProfiling} />
|
||||
|
||||
@@ -30,13 +30,19 @@ export default function RecordToggle({disabled}: Props): React.Node {
|
||||
className = styles.ActiveRecordToggle;
|
||||
}
|
||||
|
||||
const isMac =
|
||||
typeof navigator !== 'undefined' &&
|
||||
navigator.platform.toUpperCase().indexOf('MAC') >= 0;
|
||||
const shortcut = isMac ? '⌘E' : 'Ctrl+E';
|
||||
const title = `${isProfiling ? 'Stop' : 'Start'} profiling - ${shortcut}`;
|
||||
|
||||
return (
|
||||
<Button
|
||||
className={className}
|
||||
disabled={disabled}
|
||||
onClick={isProfiling ? stopProfiling : startProfiling}
|
||||
testName="ProfilerToggleButton"
|
||||
title={isProfiling ? 'Stop profiling' : 'Start profiling'}>
|
||||
title={title}>
|
||||
<ButtonIcon type="record" />
|
||||
</Button>
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user