@samitouri / QOS-React / commits / 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

Joseph Savona committed Nov 20, 2025 at 19:28 UTC df75af4edca7f316e6bfcfcde67197e8a57d1101
3 files changed +107 -80
compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
+5
@@ -817,6 +817,11 @@ export type StartMemoize = {
817 * (e.g. useMemo without a second arg)
818 */
819 deps: Array<ManualMemoDependency> | null;
820 + /**
821 + * The source location of the dependencies argument. Used for
822 + * emitting diagnostics with a suggested replacement
823 + */
824 + depsLoc: SourceLocation | null;
825 loc: SourceLocation;
826 };
827 export type FinishMemoize = {
compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts
+53 -50
@@ -42,7 +42,7 @@ type IdentifierSidemap = {
42 functions: Map<IdentifierId, TInstruction<FunctionExpression>>;
43 manualMemos: Map<IdentifierId, ManualMemoCallee>;
44 react: Set<IdentifierId>;
45 - maybeDepsLists: Map<IdentifierId, Array<Place>>;
45 + maybeDepsLists: Map<IdentifierId, {loc: SourceLocation; deps: Array<Place>}>;
46 maybeDeps: Map<IdentifierId, ManualMemoDependency>;
47 optionals: Set<IdentifierId>;
48 };
@@ -159,10 +159,10 @@ function collectTemporaries(
159 }
160 case 'ArrayExpression': {
161 if (value.elements.every(e => e.kind === 'Identifier')) {
162 - sidemap.maybeDepsLists.set(
163 - instr.lvalue.identifier.id,
164 - value.elements as Array<Place>,
165 - );
162 + sidemap.maybeDepsLists.set(instr.lvalue.identifier.id, {
163 + loc: value.loc,
164 + deps: value.elements as Array<Place>,
165 + });
166 }
167 break;
168 }
@@ -182,6 +182,7 @@ function makeManualMemoizationMarkers(
182 fnExpr: Place,
183 env: Environment,
184 depsList: Array<ManualMemoDependency> | null,
185 + depsLoc: SourceLocation | null,
186 memoDecl: Place,
187 manualMemoId: number,
188 ): [TInstruction<StartMemoize>, TInstruction<FinishMemoize>] {
@@ -197,6 +198,7 @@ function makeManualMemoizationMarkers(
198 * as dependencies
199 */
200 deps: depsList,
201 + depsLoc,
202 loc: fnExpr.loc,
203 },
204 effects: null,
@@ -287,86 +289,85 @@ function extractManualMemoizationArgs(
289 sidemap: IdentifierSidemap,
290 errors: CompilerError,
291 ): {
290 - fnPlace: Place | null;
292 + fnPlace: Place;
293 depsList: Array<ManualMemoDependency> | null;
292 -} {
294 + depsLoc: SourceLocation | null;
295 +} | null {
296 const [fnPlace, depsListPlace] = instr.value.args as Array<
297 Place | SpreadPattern | undefined
298 >;
296 - if (fnPlace == null) {
299 + if (fnPlace == null || fnPlace.kind !== 'Identifier') {
300 errors.pushDiagnostic(
301 CompilerDiagnostic.create({
302 category: ErrorCategory.UseMemo,
303 reason: `Expected a callback function to be passed to ${kind}`,
301 - description: `Expected a callback function to be passed to ${kind}`,
304 + description:
305 + kind === 'useCallback'
306 + ? 'The first argument to useCallback() must be a function to cache'
307 + : 'The first argument to useMemo() must be a function that calculates a result to cache',
308 suggestions: null,
309 }).withDetails({
310 kind: 'error',
311 loc: instr.value.loc,
306 - message: `Expected a callback function to be passed to ${kind}`,
312 + message:
313 + kind === 'useCallback'
314 + ? `Expected a callback function`
315 + : `Expected a memoization function`,
316 }),
317 );
309 - return {fnPlace: null, depsList: null};
318 + return null;
319 }
311 - if (fnPlace.kind === 'Spread' || depsListPlace?.kind === 'Spread') {
320 + if (depsListPlace == null) {
321 + return {
322 + fnPlace,
323 + depsList: null,
324 + depsLoc: null,
325 + };
326 + }
327 + const maybeDepsList =
328 + depsListPlace.kind === 'Identifier'
329 + ? sidemap.maybeDepsLists.get(depsListPlace.identifier.id)
330 + : null;
331 + if (maybeDepsList == null) {
332 errors.pushDiagnostic(
333 CompilerDiagnostic.create({
334 category: ErrorCategory.UseMemo,
315 - reason: `Unexpected spread argument to ${kind}`,
316 - description: `Unexpected spread argument to ${kind}`,
335 + reason: `Expected the dependency list for ${kind} to be an array literal`,
336 + description: `Expected the dependency list for ${kind} to be an array literal`,
337 suggestions: null,
338 }).withDetails({
339 kind: 'error',
320 - loc: instr.value.loc,
321 - message: `Unexpected spread argument to ${kind}`,
340 + loc:
341 + depsListPlace?.kind === 'Identifier' ? depsListPlace.loc : instr.loc,
342 + message: `Expected the dependency list for ${kind} to be an array literal`,
343 }),
344 );
324 - return {fnPlace: null, depsList: null};
345 + return null;
346 }
326 - let depsList: Array<ManualMemoDependency> | null = null;
327 - if (depsListPlace != null) {
328 - const maybeDepsList = sidemap.maybeDepsLists.get(
329 - depsListPlace.identifier.id,
330 - );
331 - if (maybeDepsList == null) {
347 + const depsList: Array<ManualMemoDependency> = [];
348 + for (const dep of maybeDepsList.deps) {
349 + const maybeDep = sidemap.maybeDeps.get(dep.identifier.id);
350 + if (maybeDep == null) {
351 errors.pushDiagnostic(
352 CompilerDiagnostic.create({
353 category: ErrorCategory.UseMemo,
335 - reason: `Expected the dependency list for ${kind} to be an array literal`,
336 - description: `Expected the dependency list for ${kind} to be an array literal`,
354 + reason: `Expected the dependency list to be an array of simple expressions (e.g. \`x\`, \`x.y.z\`, \`x?.y?.z\`)`,
355 + description: `Expected the dependency list to be an array of simple expressions (e.g. \`x\`, \`x.y.z\`, \`x?.y?.z\`)`,
356 suggestions: null,
357 }).withDetails({
358 kind: 'error',
340 - loc: depsListPlace.loc,
341 - message: `Expected the dependency list for ${kind} to be an array literal`,
359 + loc: dep.loc,
360 + message: `Expected the dependency list to be an array of simple expressions (e.g. \`x\`, \`x.y.z\`, \`x?.y?.z\`)`,
361 }),
362 );
344 - return {fnPlace, depsList: null};
345 - }
346 - depsList = [];
347 - for (const dep of maybeDepsList) {
348 - const maybeDep = sidemap.maybeDeps.get(dep.identifier.id);
349 - if (maybeDep == null) {
350 - errors.pushDiagnostic(
351 - CompilerDiagnostic.create({
352 - category: ErrorCategory.UseMemo,
353 - reason: `Expected the dependency list to be an array of simple expressions (e.g. \`x\`, \`x.y.z\`, \`x?.y?.z\`)`,
354 - description: `Expected the dependency list to be an array of simple expressions (e.g. \`x\`, \`x.y.z\`, \`x?.y?.z\`)`,
355 - suggestions: null,
356 - }).withDetails({
357 - kind: 'error',
358 - loc: dep.loc,
359 - message: `Expected the dependency list to be an array of simple expressions (e.g. \`x\`, \`x.y.z\`, \`x?.y?.z\`)`,
360 - }),
361 - );
362 - } else {
363 - depsList.push(maybeDep);
364 - }
363 + } else {
364 + depsList.push(maybeDep);
365 }
366 }
367 return {
368 fnPlace,
369 depsList,
370 + depsLoc: maybeDepsList.loc,
371 };
372 }
373
@@ -427,16 +428,17 @@ export function dropManualMemoization(
428
429 const manualMemo = sidemap.manualMemos.get(id);
430 if (manualMemo != null) {
430 - const {fnPlace, depsList} = extractManualMemoizationArgs(
431 + const memoDetails = extractManualMemoizationArgs(
432 instr as TInstruction<CallExpression> | TInstruction<MethodCall>,
433 manualMemo.kind,
434 sidemap,
435 errors,
436 );
437
437 - if (fnPlace == null) {
438 + if (memoDetails == null) {
439 continue;
440 }
441 + const {fnPlace, depsList, depsLoc} = memoDetails;
442
443 instr.value = getManualMemoizationReplacement(
444 fnPlace,
@@ -487,6 +489,7 @@ export function dropManualMemoization(
489 fnPlace,
490 func.env,
491 depsList,
492 + depsLoc,
493 memoDecl,
494 nextManualMemoId++,
495 );
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts
+49 -30
@@ -6,8 +6,13 @@
6 */
7
8 import prettyFormat from 'pretty-format';
9 -import {CompilerDiagnostic, CompilerError, SourceLocation} from '..';
10 -import {ErrorCategory} from '../CompilerError';
9 +import {
10 + CompilerDiagnostic,
11 + CompilerError,
12 + CompilerSuggestionOperation,
13 + SourceLocation,
14 +} from '..';
15 +import {CompilerSuggestion, ErrorCategory} from '../CompilerError';
16 import {
17 areEqualPaths,
18 BlockId,
@@ -229,38 +234,52 @@ export function validateExhaustiveDependencies(
234 extra.push(dep);
235 }
236
232 - if (missing.length !== 0) {
233 - // Error
234 - const diagnostic = CompilerDiagnostic.create({
235 - category: ErrorCategory.PreserveManualMemo,
236 - reason: 'Found non-exhaustive dependencies',
237 - description:
238 - 'Missing dependencies can cause a value not to update when those inputs change, ' +
239 - 'resulting in stale UI. This memoization cannot be safely rewritten by the compiler.',
240 - });
241 - for (const dep of missing) {
237 + if (missing.length !== 0 || extra.length !== 0) {
238 + let suggestions: Array<CompilerSuggestion> | null = null;
239 + if (startMemo.depsLoc != null && typeof startMemo.depsLoc !== 'symbol') {
240 + suggestions = [
241 + {
242 + description: 'Update dependencies',
243 + range: [startMemo.depsLoc.start.index, startMemo.depsLoc.end.index],
244 + op: CompilerSuggestionOperation.Replace,
245 + text: `[${inferred.map(printInferredDependency).join(', ')}]`,
246 + },
247 + ];
248 + }
249 + if (missing.length !== 0) {
250 + // Error
251 + const diagnostic = CompilerDiagnostic.create({
252 + category: ErrorCategory.PreserveManualMemo,
253 + reason: 'Found non-exhaustive dependencies',
254 + description:
255 + 'Missing dependencies can cause a value not to update when those inputs change, ' +
256 + 'resulting in stale UI. This memoization cannot be safely rewritten by the compiler.',
257 + suggestions,
258 + });
259 + for (const dep of missing) {
260 + diagnostic.withDetails({
261 + kind: 'error',
262 + message: `Missing dependency \`${printInferredDependency(dep)}\``,
263 + loc: dep.loc,
264 + });
265 + }
266 + error.pushDiagnostic(diagnostic);
267 + } else if (extra.length !== 0) {
268 + const diagnostic = CompilerDiagnostic.create({
269 + category: ErrorCategory.PreserveManualMemo,
270 + reason: 'Found unnecessary memoization dependencies',
271 + description:
272 + 'Unnecessary dependencies can cause a value to update more often than necessary, ' +
273 + 'which can cause effects to run more than expected. This memoization cannot be safely ' +
274 + 'rewritten by the compiler',
275 + });
276 diagnostic.withDetails({
277 kind: 'error',
244 - message: `Missing dependency \`${printInferredDependency(dep)}\``,
245 - loc: dep.loc,
278 + message: `Unnecessary dependencies ${extra.map(dep => `\`${printManualMemoDependency(dep)}\``).join(', ')}`,
279 + loc: value.loc,
280 });
281 + error.pushDiagnostic(diagnostic);
282 }
248 - error.pushDiagnostic(diagnostic);
249 - } else if (extra.length !== 0) {
250 - const diagnostic = CompilerDiagnostic.create({
251 - category: ErrorCategory.PreserveManualMemo,
252 - reason: 'Found unnecessary memoization dependencies',
253 - description:
254 - 'Unnecessary dependencies can cause a value to update more often than necessary, ' +
255 - 'which can cause effects to run more than expected. This memoization cannot be safely ' +
256 - 'rewritten by the compiler',
257 - });
258 - diagnostic.withDetails({
259 - kind: 'error',
260 - message: `Unnecessary dependencies ${extra.map(dep => `\`${printManualMemoDependency(dep)}\``).join(', ')}`,
261 - loc: value.loc,
262 - });
263 - error.pushDiagnostic(diagnostic);
283 }
284
285 dependencies.clear();