diff --git a/.changeset/fp104-timer-cleanup-pairing.md b/.changeset/fp104-timer-cleanup-pairing.md new file mode 100644 index 000000000..a1efaf62b --- /dev/null +++ b/.changeset/fp104-timer-cleanup-pairing.md @@ -0,0 +1,7 @@ +--- +"oxlint-plugin-react-doctor": patch +"eslint-plugin-react-doctor": patch +"react-doctor": patch +--- + +Recognize either global timer-clear API as releasing a matching timeout or interval handle, including exhaustive retained timer collections. diff --git a/packages/fuzz/corpus/regressions/effect-needs-cleanup--timer-pairing.tsx b/packages/fuzz/corpus/regressions/effect-needs-cleanup--timer-pairing.tsx new file mode 100644 index 000000000..10282cfa3 --- /dev/null +++ b/packages/fuzz/corpus/regressions/effect-needs-cleanup--timer-pairing.tsx @@ -0,0 +1,8 @@ +import { useEffect } from "react"; +export const Clock = () => { + useEffect(() => { + const timer = setInterval(() => {}, 100); + return () => clearTimeout(timer); + }, []); + return null; +}; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/effect-needs-cleanup-timer-pairing.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/effect-needs-cleanup-timer-pairing.test.ts new file mode 100644 index 000000000..2d32e89ab --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/effect-needs-cleanup-timer-pairing.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it } from "vite-plus/test"; +import { runRule } from "../../../test-utils/run-rule.js"; +import { effectNeedsCleanup } from "./effect-needs-cleanup.js"; + +describe("shared timeout and interval handle cleanup", () => { + it.each([ + ["setInterval", "clearTimeout"], + ["setTimeout", "clearInterval"], + ])("accepts %s released with %s", (allocate, release) => { + const result = runRule( + effectNeedsCleanup, + ` + import { useEffect } from "react"; + const Clock = () => { useEffect(() => { + const timer = ${allocate}(() => update(), 100); + return () => ${release}(timer); + }, []); return null; }; + `, + ); + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics).toHaveLength(0); + }); + + it("accepts exhaustive collection cleanup with the other timer API", () => { + const result = runRule( + effectNeedsCleanup, + ` + import { useEffect } from "react"; + const Clock = () => { useEffect(() => { + const timers = [setInterval(update, 100), setInterval(update, 200)]; + return () => timers.forEach(clearTimeout); + }, []); return null; }; + `, + ); + expect(result.diagnostics).toHaveLength(0); + }); + + it.each([ + "timers.reduce((count, timer) => { clearTimeout(timer); return count + 1; }, 0)", + "Array.from(timers, (timer) => { clearTimeout(timer); })", + ])("preserves collection callback cleanup: %s", (cleanup) => { + const result = runRule( + effectNeedsCleanup, + ` + import { useEffect } from "react"; + const Clock = () => { useEffect(() => { + const timers = [setInterval(update, 100), setInterval(update, 200)]; + return () => { ${cleanup}; }; + }, []); return null; }; + `, + ); + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics).toHaveLength(0); + }); + + it.each([ + "return () => clearTimeout(other);", + "return () => { if (enabled) clearTimeout(timer); };", + "const clearTimeout = () => {}; return () => clearTimeout(timer);", + ])("retains reports for incomplete or invalid release: %s", (cleanup) => { + const result = runRule( + effectNeedsCleanup, + ` + import { useEffect } from "react"; + const Clock = () => { useEffect(() => { + const timer = setInterval(update, 100); + ${cleanup} + }, []); return null; }; + `, + ); + expect(result.diagnostics).toHaveLength(1); + }); +}); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/effect-needs-cleanup.regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/effect-needs-cleanup.regressions.test.ts index aaccaacd7..380b67cbc 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/effect-needs-cleanup.regressions.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/effect-needs-cleanup.regressions.test.ts @@ -4242,7 +4242,7 @@ export const Pollers = ({ items, previousTimerIds }) => { expect(result.diagnostics).toHaveLength(1); }); - it("rejects the wrong clear verb for a mapped interval collection", () => { + it("accepts either clear verb for a mapped interval collection", () => { const result = runRule( effectNeedsCleanup, `import { useEffect } from "react"; @@ -4258,7 +4258,7 @@ export const Pollers = ({ items }) => { };`, ); expect(result.parseErrors).toEqual([]); - expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics).toHaveLength(0); }); it("rejects a local mapped timer that is not returned", () => { diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/effect-needs-cleanup.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/effect-needs-cleanup.ts index bf89202db..d713d6c4d 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/effect-needs-cleanup.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/effect-needs-cleanup.ts @@ -3103,8 +3103,6 @@ const isDirectExhaustiveTimerCollectionCleanup = ( const cleanupCallback = isNodeOfType(cleanupCall, "CallExpression") ? cleanupCall.arguments[0] : null; - const expectedCleanupName = - usage.registrationVerbName === "setInterval" ? "clearInterval" : "clearTimeout"; const retainedCollectionKey = findContainingCollectionKey(usage.node, context); if ( !isNodeOfType(cleanupCall, "CallExpression") || @@ -3113,7 +3111,7 @@ const isDirectExhaustiveTimerCollectionCleanup = ( !isNodeOfType(cleanupCallee.property, "Identifier") || cleanupCallee.property.name !== "forEach" || !isNodeOfType(cleanupCallback, "Identifier") || - cleanupCallback.name !== expectedCleanupName || + !TIMER_CLEANUP_CALLEE_NAMES.has(cleanupCallback.name) || !context.scopes.isGlobalReference(cleanupCallback) || retainedCollectionKey === null || retainedCollectionKey !== resolveExpressionKey(cleanupCallee.object, context) @@ -6486,13 +6484,14 @@ const effectHasCleanupForUsage = ( : null; const requiresDirectReleasePathCoverage = usage.kind === "timer" && - findEnclosingFunction(usage.node) !== callback && - Boolean( - assignedHandleSymbol && - (assignedHandleSymbol.kind === "let" || assignedHandleSymbol.kind === "var") && - isNodeOfType(assignedHandleSymbol.declarationNode, "VariableDeclarator") && - findEnclosingFunction(assignedHandleSymbol.declarationNode) === callback, - ); + ((findEnclosingFunction(usage.node) === callback && + isNodeOfType(usageAssignment, "VariableDeclarator")) || + Boolean( + assignedHandleSymbol && + (assignedHandleSymbol.kind === "let" || assignedHandleSymbol.kind === "var") && + isNodeOfType(assignedHandleSymbol.declarationNode, "VariableDeclarator") && + findEnclosingFunction(assignedHandleSymbol.declarationNode) === callback, + )); const matchingCleanupReturns: EsTreeNode[] = []; walkInsideStatementBlocks(callback.body, (child: EsTreeNode) => { if (!isNodeOfType(child, "ReturnStatement")) return; @@ -7628,12 +7627,10 @@ const doesReleaseCallMatchUsage = ( const callee = stripParenExpression(callNode.callee); if (usage.kind === "timer") { - const expectedCleanupName = - usage.registrationVerbName === "setInterval" ? "clearInterval" : "clearTimeout"; if ( !isNodeOfType(callee, "Identifier") || !TIMER_CLEANUP_CALLEE_NAMES.has(callee.name) || - callee.name !== expectedCleanupName + !context.scopes.isGlobalReference(callee) ) { return false; } @@ -8401,9 +8398,9 @@ const fileContainsReleaseForUsage = (usage: SubscribeLikeUsage, context: RuleCon } let candidates: ReadonlyArray; if (usage.kind === "timer") { - const expectedCleanupName = - usage.registrationVerbName === "setInterval" ? "clearInterval" : "clearTimeout"; - candidates = releaseCallIndex.identifierCallsByName.get(expectedCleanupName) ?? []; + candidates = [...TIMER_CLEANUP_CALLEE_NAMES].flatMap( + (cleanupName) => releaseCallIndex.identifierCallsByName.get(cleanupName) ?? [], + ); } else { candidates = releaseCallIndex.potentialNonTimerCalls; }