Compare commits

..

8 Commits

Author SHA1 Message Date
Joe Savona
290174c38f [compiler] Adjustments to exhaustive deps messages, disable the lint rule
Similar to ValidateHookUsage, we implement this check in the compiler for safety but (for now) continue to rely on the existing rule for actually reporting errors to users.
2025-11-24 12:19:54 -08:00
Joseph Savona
67c1487ffd [compiler] Allow extraneous non-reactive locals (#35190)
The existing exhaustive-deps rule allows omitting non-reactive
dependencies, even if they're not memoized. Conceptually, if a value is
non-reactive then it cannot semantically change. Even if the value is a
new object, that object represents the exact same value and doesn't
necessitate redoing downstream computation. Thus its fine to exclude
nonreactive dependencies, whether they're a stable type or not.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35190).
* #35201
* #35202
* #35192
* __->__ #35190
2025-11-24 12:18:49 -08:00
Joseph Savona
454e01e603 [compiler] Allow manual dependencies to have different optionality than inferred deps (#35186)
Since adding this validation we've already changed our inference to use
knowledge from manual memoization to inform when values are frozen and
which values are non-nullable. To align with that, if the user chooses
to use different optionality btw the deps and the memo block/callback,
that's fine. The key is that eg `x?.y` will invalidate whenever `x.y`
would, so from a memoization correctness perspective its fine. It's not
our job to be a type checker: if a value is potentially nullable, it
should likely use a nullable property access in both places but
TypeScript/Flow can check that.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35186).
* #35201
* #35202
* #35192
* #35190
* __->__ #35186
2025-11-24 12:17:03 -08:00
Joseph Savona
c9a8cf3411 [compiler] Allow nonreactive stable types as extraneous deps (#35185)
When checking ValidateExhaustiveDeps internally, this seems to be the
most common case that it flags. The current exhaustive-deps rule allows
extraneous deps if they are a set of stable types. So here we reuse our
existing isStableType() util in the compiler to allow this case.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35185).
* #35201
* #35202
* #35192
* #35190
* #35186
* __->__ #35185
2025-11-24 12:15:06 -08:00
Joseph Savona
fca172e3f3 [compiler] Ignore ESLint suppressions when ValidateMemoDeps enabled (#35184)
With `ValidateExhaustiveMemoDependencies` we can now check exhaustive
dependencies for useMemo and useCallback within the compiler, without
relying on the separate exhaustive-deps rule. Until now we've bailed out
of any component/hook that suppresses this rule, since the suppression
_might_ affect a memoization value. Compiling code with incorrect memo
deps can change behavior so this wasn't safe. The downside was that a
suppression within a useEffect could prevent memoization, even though
non-exhaustive deps for effects do not cause problems for memoization
specifically.

So here, we change to ignore ESLint suppressions if we have both the
compiler's hooks validation and memo deps validations enabled.

Now we just have to test out the new validation and refine before we can
enable this by default.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35184).
* #35201
* #35202
* #35192
* #35190
* #35186
* #35185
* __->__ #35184
2025-11-24 12:12:49 -08:00
emily8rown
fd524fe02a [DevTools] hotkey to start/stop profiling (#35160)
## Summary

The built-in browser profiler supports starting/stopping with Cmd+E. For
Symmetry this adds the same hotkey for react devtools profiler.

## How did you test this change?
yarn build:\<browser name\> 
yarn run test:\<browser name\>

<img width="483" height="135" alt="Screenshot 2025-11-17 at 14 30 34"
src="https://github.com/user-attachments/assets/426939aa-15da-4c21-87a4-e949e6949482"
/>

firefox:

https://github.com/user-attachments/assets/6f225b90-828f-4e79-a364-59d6bc942f83

edge:

https://github.com/user-attachments/assets/5b2e9242-f0e8-481b-99a2-2dd78099f3ac

chrome:

https://github.com/user-attachments/assets/790aab02-2867-4499-aec1-e32e38c763f9

---------

Co-authored-by: Ruslan Lesiutin <28902667+hoxyq@users.noreply.github.com>
2025-11-21 11:37:10 -05:00
Joseph Savona
40b4a5bf71 [compiler] ValidateExhaustiveDeps disallows unnecessary non-reactive deps (#34472)
Just to be consistent, we disallow unnecessary deps even if they're
known to be non-reactive.
2025-11-20 19:30:35 -08:00
Joseph Savona
df75af4edc [compiler] Auto-fix for non-exhaustive deps (#34471)
Records more information in DropManualMemoization so that we know the
full span of the manual dependencies array (if present). This allows
ValidateExhaustiveDeps to include a suggestion with the correct deps.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34471).
* #34472
* __->__ #34471
2025-11-20 19:28:08 -08:00
17 changed files with 634 additions and 99 deletions

View File

@@ -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,

View File

@@ -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);

View File

@@ -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,
);

View File

@@ -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}`,

View File

@@ -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);

View File

@@ -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

View File

@@ -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} />;
}

View File

@@ -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 | }, []);
```

View File

@@ -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]} />;
}

View File

@@ -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 ]]"

View File

@@ -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: [],
};

View File

@@ -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;
}

View File

@@ -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]} />;
}

View File

@@ -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')

View File

@@ -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);
});
});

View File

@@ -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} />

View File

@@ -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>
);