diff --git a/.changeset/calm-timers-listen.md b/.changeset/calm-timers-listen.md new file mode 100644 index 0000000000..fe086db916 --- /dev/null +++ b/.changeset/calm-timers-listen.md @@ -0,0 +1,5 @@ +--- +"oxlint-plugin-react-doctor": patch +--- + +Avoid `effect-needs-cleanup` diagnostics for owned chained timers, guarded post-await timers, and listeners released through an abort handler. diff --git a/.changeset/curly-hounds-invite.md b/.changeset/curly-hounds-invite.md new file mode 100644 index 0000000000..d3015d31dc --- /dev/null +++ b/.changeset/curly-hounds-invite.md @@ -0,0 +1,5 @@ +--- +"oxlint-plugin-react-doctor": patch +--- + +Fix an `async-defer-await` false positive on exact `live` and `isLive` liveness guards without exempting unrelated names that only contain the same text. diff --git a/.changeset/detect-sentry-wrapped-compiler-config.md b/.changeset/detect-sentry-wrapped-compiler-config.md new file mode 100644 index 0000000000..5df8e37a98 --- /dev/null +++ b/.changeset/detect-sentry-wrapped-compiler-config.md @@ -0,0 +1,5 @@ +--- +"@react-doctor/core": patch +--- + +Detect React Compiler configuration passed through `withSentryConfig`. diff --git a/.changeset/fix-1757-headers-helper.md b/.changeset/fix-1757-headers-helper.md new file mode 100644 index 0000000000..02413836cf --- /dev/null +++ b/.changeset/fix-1757-headers-helper.md @@ -0,0 +1,9 @@ +--- +"oxlint-plugin-react-doctor": patch +--- + +Fix false positive in `nextjs-no-side-effect-in-get-handler` when locally-built `Headers` object is passed to a same-file helper that mutates it. + +The rule now transfers locally-created response object safety through the exact same-file helper call. Calls that pass external state to the same helper remain reportable. + +Fixes #1757 diff --git a/.changeset/respect-use-no-memo-directive.md b/.changeset/respect-use-no-memo-directive.md new file mode 100644 index 0000000000..a3764f2243 --- /dev/null +++ b/.changeset/respect-use-no-memo-directive.md @@ -0,0 +1,9 @@ +--- +"oxlint-plugin-react-doctor": patch +--- + +fix: respect "use no memo" directive in react-compiler-no-manual-memoization rule + +When a function or module has a React Compiler opt-out directive, the compiler skips optimization, so manual memoization can still be necessary. The rule now respects `"use no memo"`, its `"use no forget"` alias, and local components passed to `memo`. + +Fixes #1749 diff --git a/.changeset/sanity-blueprint-convention.md b/.changeset/sanity-blueprint-convention.md new file mode 100644 index 0000000000..bdd69c915e --- /dev/null +++ b/.changeset/sanity-blueprint-convention.md @@ -0,0 +1,9 @@ +--- +"@react-doctor/core": patch +--- + +Treat Sanity blueprint files as convention entries + +`sanity.blueprint.ts` is a Sanity Studio convention file loaded by filename by the Sanity CLI (`sanity blueprints deploy`), similar to `sanity.config.ts` and `sanity.cli.ts`. It was incorrectly reported as unused by `react-doctor/unused-file`. + +Fixes #1747 diff --git a/.changeset/send-resend-notify-email-exemption.md b/.changeset/send-resend-notify-email-exemption.md new file mode 100644 index 0000000000..fabd2bbd28 --- /dev/null +++ b/.changeset/send-resend-notify-email-exemption.md @@ -0,0 +1,5 @@ +--- +"oxlint-plugin-react-doctor": patch +--- + +Exempt magic-link delivery mutations from `query-mutation-missing-invalidation` while keeping generic send, notification, and email mutations reportable. diff --git a/.changeset/trace-timeout-increase.md b/.changeset/trace-timeout-increase.md new file mode 100644 index 0000000000..fe14bb3917 --- /dev/null +++ b/.changeset/trace-timeout-increase.md @@ -0,0 +1,5 @@ +--- +"react-doctor": patch +--- + +Increase runtime trace finalization timeout from 10 to 60 seconds to handle large traces from longer recording sessions diff --git a/packages/core/src/project-analysis/collect/entries.ts b/packages/core/src/project-analysis/collect/entries.ts index f151140227..464429192c 100644 --- a/packages/core/src/project-analysis/collect/entries.ts +++ b/packages/core/src/project-analysis/collect/entries.ts @@ -2726,7 +2726,7 @@ const FRAMEWORK_PATTERNS: ToolingPluginDefinition[] = [ enablers: ["sanity", "@sanity/cli"], enablerPrefixes: ["@sanity/"], entryPatterns: [], - alwaysUsed: ["sanity.config.{ts,js}", "sanity.cli.{ts,js}"], + alwaysUsed: ["sanity.config.{ts,js}", "sanity.cli.{ts,js}", "sanity.blueprint.{ts,js}"], }, { enablers: ["astro"], diff --git a/packages/core/src/project-info/react-compiler-config-evaluator.ts b/packages/core/src/project-info/react-compiler-config-evaluator.ts index c2955fd8f0..3421a0acea 100644 --- a/packages/core/src/project-info/react-compiler-config-evaluator.ts +++ b/packages/core/src/project-info/react-compiler-config-evaluator.ts @@ -1491,6 +1491,17 @@ const analyzeConfigIdentifier = ( ); }; +const hasReactCompilerConfigInSentryWrapperArgument = ( + callExpression: ts.CallExpression, + analysis: ConfigExpressionAnalysis, + moduleSpecifier: string, + exportName: string, +): boolean => { + if (moduleSpecifier !== "@sentry/nextjs" || exportName !== "withSentryConfig") return false; + const [configArgument] = callExpression.arguments; + return Boolean(configArgument && analyzeConfigNode(configArgument, analysis, false)); +}; + const analyzeConfigCallTarget = ( callExpression: ts.CallExpression, analysis: ConfigExpressionAnalysis, @@ -1543,6 +1554,16 @@ const analyzeConfigCallTarget = ( ) { return true; } + if ( + hasReactCompilerConfigInSentryWrapperArgument( + callExpression, + analysis, + requiredModuleSpecifier, + propertyName, + ) + ) { + return true; + } const hasCompilerTransform = analyzeImportedConfig({ analysis, moduleSpecifier: requiredModuleSpecifier, @@ -1570,6 +1591,16 @@ const analyzeConfigCallTarget = ( ) { return true; } + if ( + hasReactCompilerConfigInSentryWrapperArgument( + callExpression, + analysis, + importBinding.moduleSpecifier, + propertyName, + ) + ) { + return true; + } const hasCompilerTransform = analyzeImportedConfig({ analysis, moduleSpecifier: importBinding.moduleSpecifier, @@ -1864,6 +1895,16 @@ const analyzeConfigNode = ( ) { return true; } + if ( + hasReactCompilerConfigInSentryWrapperArgument( + node, + analysis, + importBinding.moduleSpecifier, + importBinding.exportName, + ) + ) { + return true; + } if ( allowCompilerTransform && importBinding.moduleSpecifier === "@rolldown/plugin-babel" && diff --git a/packages/core/tests/discover-project.test.ts b/packages/core/tests/discover-project.test.ts index 26f770868c..605d97a558 100644 --- a/packages/core/tests/discover-project.test.ts +++ b/packages/core/tests/discover-project.test.ts @@ -2704,6 +2704,75 @@ describe("discoverProject", () => { }, ); + it.each([ + { + name: "named-import", + config: + "import { withSentryConfig } from '@sentry/nextjs'; const nextConfig = { reactCompiler: true }; export default withSentryConfig(nextConfig, { org: 'x' });", + helper: null, + expected: true, + }, + { + name: "namespace-import", + config: + "import * as Sentry from '@sentry/nextjs'; const nextConfig = { reactCompiler: true }; export default Sentry.withSentryConfig(nextConfig, { org: 'x' });", + helper: null, + expected: true, + }, + { + name: "commonjs-require", + config: + "const Sentry = require('@sentry/nextjs'); const nextConfig = { reactCompiler: true }; module.exports = Sentry.withSentryConfig(nextConfig, { org: 'x' });", + helper: null, + expected: true, + }, + { + name: "local-wrapper", + config: + "import { wrap } from './wrapper'; const nextConfig = { reactCompiler: true }; export default wrap(nextConfig);", + helper: + "import { withSentryConfig } from '@sentry/nextjs'; export const wrap = (config) => withSentryConfig(config, { org: 'x' });", + expected: true, + }, + { + name: "compiler-only-in-options", + config: + "import { withSentryConfig } from '@sentry/nextjs'; export default withSentryConfig({ reactCompiler: false }, { reactCompiler: true });", + helper: null, + expected: false, + }, + ])( + "detects React Compiler through Sentry config wrappers: $name", + ({ name, config, helper, expected }) => { + const projectDirectory = path.join(tempDirectory, `nextjs-sentry-wrapper-${name}`); + const wrapperDirectory = path.join(projectDirectory, "node_modules", "@sentry", "nextjs"); + fs.mkdirSync(wrapperDirectory, { recursive: true }); + fs.writeFileSync( + path.join(projectDirectory, "package.json"), + JSON.stringify({ + name: `nextjs-sentry-wrapper-${name}`, + dependencies: { next: "^16.0.0", react: "^19.0.0", "@sentry/nextjs": "^10.0.0" }, + }), + ); + fs.writeFileSync( + path.join(wrapperDirectory, "package.json"), + JSON.stringify({ + name: "@sentry/nextjs", + type: "module", + exports: "./index.js", + }), + ); + fs.writeFileSync( + path.join(wrapperDirectory, "index.js"), + "export const withSentryConfig = (_config, _options) => ({ sentry: true });\n", + ); + fs.writeFileSync(path.join(projectDirectory, "next.config.ts"), config); + if (helper) fs.writeFileSync(path.join(projectDirectory, "wrapper.ts"), helper); + + expect(discoverProject(projectDirectory).hasReactCompiler).toBe(expected); + }, + ); + it("detects the Rsbuild React Compiler transform", () => { const projectDirectory = path.join(tempDirectory, "rsbuild-react-compiler"); fs.mkdirSync(projectDirectory, { recursive: true }); diff --git a/packages/core/tests/project-analysis.test.ts b/packages/core/tests/project-analysis.test.ts index 6949c6b0bb..34f7d2bf8e 100644 --- a/packages/core/tests/project-analysis.test.ts +++ b/packages/core/tests/project-analysis.test.ts @@ -638,6 +638,28 @@ describe("analyzeProject", () => { expect(relativePaths(rootDirectory, result.unusedFiles)).toEqual(["src/orphan.jsx"]); }); + it("treats Sanity blueprint configuration as a convention entry", async () => { + const rootDirectory = createProject( + { + "sanity.config.ts": `export default { name: "studio", title: "Studio" };`, + "sanity.blueprint.ts": ` + import { defineBlueprint } from "@sanity/blueprints"; + import { blueprintHelper } from "./lib/blueprint-helper"; + export default defineBlueprint({ + resources: blueprintHelper, + }); + `, + "lib/blueprint-helper.ts": "export const blueprintHelper = [];", + "src/orphan.ts": "export const orphan = true;", + }, + { dependencies: { sanity: "1.0.0", "@sanity/blueprints": "1.0.0" } }, + ); + + const result = await analyzeProject({ rootDirectory }); + + expect(relativePaths(rootDirectory, result.unusedFiles)).toEqual(["src/orphan.ts"]); + }); + it("discovers static entries from CoffeeScript interpolated require factories", async () => { const rootDirectory = createProject( { diff --git a/packages/fuzz/corpus/regressions/async-defer-await--run-live-guard.ts b/packages/fuzz/corpus/regressions/async-defer-await--run-live-guard.ts new file mode 100644 index 0000000000..9a23aab6cf --- /dev/null +++ b/packages/fuzz/corpus/regressions/async-defer-await--run-live-guard.ts @@ -0,0 +1,15 @@ +// rule: async-defer-await +// verdict: pass +// weakness: name-heuristic +// source: issue #1758 + +declare const refreshSession: () => Promise; +declare const setOk: (value: boolean) => void; + +const run = { live: true }; + +export const effect = async () => { + const refreshed = await refreshSession(); + if (!run.live) return; + setOk(refreshed); +}; diff --git a/packages/fuzz/corpus/regressions/effect-needs-cleanup--issue-1756-abort-handler.tsx b/packages/fuzz/corpus/regressions/effect-needs-cleanup--issue-1756-abort-handler.tsx new file mode 100644 index 0000000000..ff03df36d8 --- /dev/null +++ b/packages/fuzz/corpus/regressions/effect-needs-cleanup--issue-1756-abort-handler.tsx @@ -0,0 +1,18 @@ +// rule: effect-needs-cleanup +// weakness: cleanup-provenance +// source: issue #1756 +// verdict: pass +import { useEffect } from "react"; + +export const AbortHandlerCleanup = () => { + useEffect(() => { + const controller = new AbortController(); + const onChange = () => {}; + document.addEventListener("visibilitychange", onChange); + controller.signal.addEventListener("abort", () => { + document.removeEventListener("visibilitychange", onChange); + }); + return () => controller.abort(); + }, []); + return null; +}; diff --git a/packages/fuzz/corpus/regressions/effect-needs-cleanup--issue-1756-chained-timer.tsx b/packages/fuzz/corpus/regressions/effect-needs-cleanup--issue-1756-chained-timer.tsx new file mode 100644 index 0000000000..ebf3f9bf5a --- /dev/null +++ b/packages/fuzz/corpus/regressions/effect-needs-cleanup--issue-1756-chained-timer.tsx @@ -0,0 +1,27 @@ +// rule: effect-needs-cleanup +// weakness: cleanup-provenance +// source: issue #1756 +// verdict: pass +import { useEffect } from "react"; + +export const ChainedTimer = () => { + useEffect(() => { + let timer: ReturnType | null = null; + const clearTimer = () => { + if (timer !== null) { + clearTimeout(timer); + timer = null; + } + }; + const schedule = (callback: () => void) => { + timer = setTimeout(callback, 1000); + }; + const advance = () => { + timer = null; + schedule(advance); + }; + schedule(advance); + return () => clearTimer(); + }, []); + return null; +}; diff --git a/packages/fuzz/corpus/regressions/effect-needs-cleanup--issue-1756-guarded-async-timer.tsx b/packages/fuzz/corpus/regressions/effect-needs-cleanup--issue-1756-guarded-async-timer.tsx new file mode 100644 index 0000000000..1dc042348f --- /dev/null +++ b/packages/fuzz/corpus/regressions/effect-needs-cleanup--issue-1756-guarded-async-timer.tsx @@ -0,0 +1,27 @@ +// rule: effect-needs-cleanup +// weakness: async-lifecycle-provenance +// source: issue #1756 +// verdict: pass +import { useEffect } from "react"; + +declare const refresh: () => Promise; + +export const GuardedAsyncTimer = () => { + useEffect(() => { + const run = { live: true }; + let timer: ReturnType | null = null; + const schedule = () => { + timer = setTimeout(() => {}, 1000); + }; + void (async () => { + await refresh(); + if (!run.live) return; + schedule(); + })(); + return () => { + run.live = false; + if (timer !== null) clearTimeout(timer); + }; + }, []); + return null; +}; diff --git a/packages/fuzz/corpus/regressions/nextjs-no-side-effect-in-get-handler--local-headers-helper.ts b/packages/fuzz/corpus/regressions/nextjs-no-side-effect-in-get-handler--local-headers-helper.ts new file mode 100644 index 0000000000..2b20a72715 --- /dev/null +++ b/packages/fuzz/corpus/regressions/nextjs-no-side-effect-in-get-handler--local-headers-helper.ts @@ -0,0 +1,15 @@ +// verdict: pass +// rule: nextjs-no-side-effect-in-get-handler +// weakness: copy-tracking +// source: GitHub issue #1757 +// file-path: src/app/api/proxy/route.ts + +const applyCachePolicy = (responseHeaders: Headers) => { + responseHeaders.set("Cache-Control", "max-age=60"); +}; + +export const GET = () => { + const responseHeaders = new Headers(); + applyCachePolicy(responseHeaders); + return new Response(null, { headers: responseHeaders }); +}; diff --git a/packages/fuzz/corpus/regressions/query-mutation-missing-invalidation--magic-link-delivery.tsx b/packages/fuzz/corpus/regressions/query-mutation-missing-invalidation--magic-link-delivery.tsx new file mode 100644 index 0000000000..92d44ccdec --- /dev/null +++ b/packages/fuzz/corpus/regressions/query-mutation-missing-invalidation--magic-link-delivery.tsx @@ -0,0 +1,13 @@ +// verdict: pass +// rule: query-mutation-missing-invalidation +// weakness: name-heuristic +// source: GitHub issue #1759 + +import { useMutation } from "@tanstack/react-query"; + +declare const sendMagicLink: (email: string) => Promise; + +export const useSendMagicLink = () => + useMutation({ + mutationFn: sendMagicLink, + }); diff --git a/packages/fuzz/corpus/regressions/react-compiler-no-manual-memoization--compiler-opt-out-directive.tsx b/packages/fuzz/corpus/regressions/react-compiler-no-manual-memoization--compiler-opt-out-directive.tsx new file mode 100644 index 0000000000..a55ef34eea --- /dev/null +++ b/packages/fuzz/corpus/regressions/react-compiler-no-manual-memoization--compiler-opt-out-directive.tsx @@ -0,0 +1,12 @@ +// verdict: pass +// rule: react-compiler-no-manual-memoization +// weakness: framework-gating +// source: GitHub issue #1749 + +import { useMemo } from "react"; + +export const LegacyComponent = () => { + "use no memo"; + const cachedValue = useMemo(() => getValue(), []); + return {cachedValue}; +}; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/react-compiler-no-manual-memoization.regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/react-compiler-no-manual-memoization.regressions.test.ts index 9df3efcd24..614bc6626c 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/react-compiler-no-manual-memoization.regressions.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/react-compiler-no-manual-memoization.regressions.test.ts @@ -168,4 +168,55 @@ export default function ({ onPress }) { ); expect(result.diagnostics).toEqual([]); }); + + it("does not flag useMemo inside a component with 'use no memo' directive (issue #1749)", () => { + const result = run( + `import { useMemo } from "react"; +export function SomeComponent() { + "use no memo"; + const something = useMemo(() => getSomething(), []); + return
{something}
; +}`, + ); + expect(result.diagnostics).toEqual([]); + }); + + it("does not flag useCallback inside a component with 'use no memo' directive", () => { + const result = run( + `import { useCallback } from "react"; +export function LegacyComponent() { + "use no memo"; + const handler = useCallback(() => console.log("click"), []); + return ; +}`, + ); + expect(result.diagnostics).toEqual([]); + }); + + it("does not flag memo wrapping a component with 'use no memo' directive", () => { + const result = run( + `import { memo } from "react"; +const Component = memo(function Inner({ value }) { + "use no memo"; + return {value}; +});`, + ); + expect(result.diagnostics).toEqual([]); + }); + + it("still flags useMemo in a nested component without 'use no memo' inside a parent with the directive", () => { + const result = run( + `import { useMemo } from "react"; +export function OuterComponent() { + "use no memo"; + const InnerComponent = () => { + const cached = useMemo(() => 1, []); + return
{cached}
; + }; + return ; +}`, + ); + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0]?.message).toContain("useMemo"); + }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/react-compiler-no-manual-memoization.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/react-compiler-no-manual-memoization.test.ts index d0f0fd48a4..cfddec6114 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/react-compiler-no-manual-memoization.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/react-compiler-no-manual-memoization.test.ts @@ -315,4 +315,153 @@ export default Wrapped;`, 0, ); }); + + it("does not flag `useMemo` in a component with 'use no memo' directive", () => { + expectDiagnosticCount( + `import { useMemo } from "react"; +export function Component() { + "use no memo"; + const cachedValue = useMemo(() => 1, []); + return {cachedValue}; +}`, + 0, + ); + }); + + it("does not flag `useCallback` in a component with 'use no memo' directive", () => { + expectDiagnosticCount( + `import { useCallback } from "react"; +export function Component() { + "use no memo"; + const handler = useCallback(() => undefined, []); + return ; +}`, + 0, + ); + }); + + it("does not flag `React.useMemo` in a component with 'use no memo' directive (namespace import)", () => { + expectDiagnosticCount( + `import * as React from "react"; +export function Component() { + "use no memo"; + const cachedValue = React.useMemo(() => 1, []); + return {cachedValue}; +}`, + 0, + ); + }); + + it("does not flag when 'use no memo' appears in single quotes", () => { + expectDiagnosticCount( + `import { useMemo } from "react"; +export function Component() { + 'use no memo'; + const cachedValue = useMemo(() => 1, []); + return {cachedValue}; +}`, + 0, + ); + }); + + it("still flags `useMemo` in a nested function without 'use no memo' inside a component that has the directive", () => { + expectDiagnosticCount( + `import { useMemo } from "react"; +export function OuterComponent() { + "use no memo"; + const InnerComponent = () => { + const cachedValue = useMemo(() => 1, []); + return {cachedValue}; + }; + return ; +}`, + 1, + ); + }); + + it("does not flag `useMemo` in arrow function component with 'use no memo'", () => { + expectDiagnosticCount( + `import { useMemo } from "react"; +export const Component = () => { + "use no memo"; + const cachedValue = useMemo(() => 1, []); + return {cachedValue}; +};`, + 0, + ); + }); + + it("does not flag manual memoization when the module has a compiler opt-out directive", () => { + expectDiagnosticCount( + `"use no memo"; +import { memo, useMemo } from "react"; +export const Component = memo(function Component() { + const cachedValue = useMemo(() => 1, []); + return {cachedValue}; +});`, + 0, + ); + }); + + it("does not flag manual memoization with the 'use no forget' alias", () => { + expectDiagnosticCount( + `import { useMemo } from "react"; +export function Component() { + "use no forget"; + const cachedValue = useMemo(() => 1, []); + return {cachedValue}; +}`, + 0, + ); + }); + + it("does not flag memo around a local component with a compiler opt-out directive", () => { + expectDiagnosticCount( + `import { memo } from "react"; +function Component() { + "use no memo"; + return Value; +} +export default memo(Component);`, + 0, + ); + }); + + it("still flags manual memoization when the string is outside the directive prologue", () => { + expectDiagnosticCount( + `import { useMemo } from "react"; +export function Component() { + prepare(); + "use no memo"; + const cachedValue = useMemo(() => 1, []); + return {cachedValue}; +}`, + 1, + ); + }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/react-compiler-no-manual-memoization.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/react-compiler-no-manual-memoization.ts index af686ced3f..8b6933220a 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/react-compiler-no-manual-memoization.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/react-compiler-no-manual-memoization.ts @@ -2,10 +2,12 @@ import { defineRule } from "../../utils/define-rule.js"; import type { EsTreeNode } from "../../utils/es-tree-node.js"; import type { EsTreeNodeOfType } from "../../utils/es-tree-node-of-type.js"; import { findEnclosingFunction } from "../../utils/find-enclosing-function.js"; +import { findVariableInitializer } from "../../utils/find-variable-initializer.js"; import { getImportedNameFromModule, isImportedFromModule, } from "../../utils/find-import-source-for-name.js"; +import { hasReactCompilerOptOutDirective } from "../../utils/has-react-compiler-opt-out-directive.js"; import { isCanonicalReactNamespaceName } from "../../utils/is-canonical-react-namespace-name.js"; import { isNodeOfType } from "../../utils/is-node-of-type.js"; import { isReactComponentOrHookName } from "../../utils/is-react-component-or-hook-name.js"; @@ -147,31 +149,50 @@ export const reactCompilerNoManualMemoization = defineRule({ requires: ["react-compiler"], recommendation: "Profile compiler-managed code and remove `useMemo`, `useCallback`, or `memo` only when the manual cache no longer carries behavioral or performance intent.", - create: (context: RuleContext) => ({ - CallExpression(node: EsTreeNodeOfType<"CallExpression">) { - const apiName = resolveReactApiNameForCallee(node.callee, context); - if (!apiName) return; - // `memo(Component, areEqual)` with a custom comparator encodes - // bespoke equality the compiler can't replicate, so it isn't - // redundant — leave it alone. A nullish second arg is no comparator - // at all, so it doesn't earn the exemption. - if (apiName === "memo") { - const comparatorArgument = node.arguments?.[1]; - if (comparatorArgument && !isNullishComparatorArgument(comparatorArgument)) return; - } else { - // `useMemo` / `useCallback` are only redundant inside a function - // the compiler will actually compile. Inside a function it skips - // (an anonymous arrow handed to a non-React HOC, a non-component - // helper) nothing is auto-cached, so the manual memoization stays. - const enclosingFunction = findEnclosingFunction(node); - if (!enclosingFunction || !isCompilerInferableFunction(enclosingFunction)) return; - } - const removalMessage = REMOVAL_MESSAGE_BY_REACT_API_NAME.get(apiName); - if (!removalMessage) return; - context.report({ - node, - message: removalMessage, - }); - }, - }), + create: (context: RuleContext) => { + let doesModuleOptOutOfReactCompiler = false; + return { + Program(node: EsTreeNodeOfType<"Program">) { + doesModuleOptOutOfReactCompiler = hasReactCompilerOptOutDirective(node); + }, + CallExpression(node: EsTreeNodeOfType<"CallExpression">) { + if (doesModuleOptOutOfReactCompiler) return; + const apiName = resolveReactApiNameForCallee(node.callee, context); + if (!apiName) return; + // `memo(Component, areEqual)` with a custom comparator encodes + // bespoke equality the compiler can't replicate, so it isn't + // redundant — leave it alone. A nullish second arg is no comparator + // at all, so it doesn't earn the exemption. + if (apiName === "memo") { + const comparatorArgument = node.arguments?.[1]; + if (comparatorArgument && !isNullishComparatorArgument(comparatorArgument)) return; + let wrappedComponent = stripParenExpression(node.arguments?.[0]); + if (wrappedComponent && isNodeOfType(wrappedComponent, "Identifier")) { + const componentBinding = findVariableInitializer( + wrappedComponent, + wrappedComponent.name, + ); + if (componentBinding?.initializer) { + wrappedComponent = stripParenExpression(componentBinding.initializer); + } + } + if (wrappedComponent && hasReactCompilerOptOutDirective(wrappedComponent)) return; + } else { + // `useMemo` / `useCallback` are only redundant inside a function + // the compiler will actually compile. Inside a function it skips + // (an anonymous arrow handed to a non-React HOC, a non-component + // helper) nothing is auto-cached, so the manual memoization stays. + const enclosingFunction = findEnclosingFunction(node); + if (!enclosingFunction || !isCompilerInferableFunction(enclosingFunction)) return; + if (hasReactCompilerOptOutDirective(enclosingFunction)) return; + } + const removalMessage = REMOVAL_MESSAGE_BY_REACT_API_NAME.get(apiName); + if (!removalMessage) return; + context.report({ + node, + message: removalMessage, + }); + }, + }; + }, }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/nextjs/nextjs-no-side-effect-in-get-handler.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/nextjs/nextjs-no-side-effect-in-get-handler.ts index 4e8f921d7d..8f717b8f1f 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/nextjs/nextjs-no-side-effect-in-get-handler.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/nextjs/nextjs-no-side-effect-in-get-handler.ts @@ -4,6 +4,7 @@ import { ROUTE_HANDLER_FILE_PATTERN, } from "../../constants/nextjs.js"; import { GET_HANDLER_BINDING_RESOLUTION_DEPTH } from "../../constants/thresholds.js"; +import { collectHelperParameterSafeBindings } from "../../utils/collect-helper-parameter-safe-bindings.js"; import { collectLocallyScopedCookieBindings } from "../../utils/collect-locally-scoped-cookie-bindings.js"; import { collectLocallyScopedSafeBindings } from "../../utils/collect-locally-scoped-safe-bindings.js"; import { defineRule } from "../../utils/define-rule.js"; @@ -203,24 +204,31 @@ const resolveGetHandlerBodies = ( // `destroySession()` whose body does `cookies().delete(...)`). Collect those // helper bodies so they're scanned alongside the handler body; helpers called // from within helpers are NOT followed. -const collectCalledSameFileHelperBodies = ( +interface HelperBodyInfo { + body: EsTreeNode; + helperFunction: EsTreeNode; + callExpression: EsTreeNodeOfType<"CallExpression">; +} + +const collectCalledSameFileHelpers = ( handlerBody: EsTreeNode, resolveBinding: (identifierName: string) => EsTreeNode | null, -): EsTreeNode[] => { - const helperBodies: EsTreeNode[] = []; - const visitedHelperNames = new Set(); +): HelperBodyInfo[] => { + const helpers: HelperBodyInfo[] = []; walkAst(handlerBody, (child: EsTreeNode) => { if (!isNodeOfType(child, "CallExpression")) return; if (!isNodeOfType(child.callee, "Identifier")) return; const helperName = child.callee.name; - if (visitedHelperNames.has(helperName)) return; - visitedHelperNames.add(helperName); const helperBinding = resolveBinding(helperName); if (!isFunctionLike(helperBinding) || !helperBinding.body) return; if (helperBinding.body === handlerBody) return; - helperBodies.push(helperBinding.body); + helpers.push({ + body: helperBinding.body, + helperFunction: helperBinding, + callExpression: child, + }); }); - return helperBodies; + return helpers; }; export const nextjsNoSideEffectInGetHandler = defineRule({ @@ -257,22 +265,45 @@ export const nextjsNoSideEffectInGetHandler = defineRule({ const handlerBodies = resolveGetHandlerBodies(node, resolveBinding); for (const handlerBody of handlerBodies) { - const bodiesToScan = [ - handlerBody, - ...collectCalledSameFileHelperBodies(handlerBody, resolveBinding), - ]; - for (const scanBody of bodiesToScan) { - const sideEffect = findSideEffect(scanBody, { - locallyScopedSafeBindings: collectLocallyScopedSafeBindings(scanBody), - locallyScopedCookieBindings: collectLocallyScopedCookieBindings(scanBody), - }); - if (!sideEffect) continue; + const locallyScopedSafeBindings = collectLocallyScopedSafeBindings(handlerBody); + const locallyScopedCookieBindings = collectLocallyScopedCookieBindings(handlerBody); + + const sideEffectInHandler = findSideEffect(handlerBody, { + locallyScopedSafeBindings, + locallyScopedCookieBindings, + }); + if (sideEffectInHandler) { const message = mutatingSegment - ? `This GET handler on the "/${mutatingSegment}" route performs a side effect (${sideEffect}) and is prone to CSRF vulnerabilities, since prefetching or a forged request can trigger it.` - : `This GET handler's side effect (${sideEffect}) is prone to CSRF vulnerabilities, since prefetching or a forged request can trigger it.`; + ? `This GET handler on the "/${mutatingSegment}" route performs a side effect (${sideEffectInHandler}) and is prone to CSRF vulnerabilities, since prefetching or a forged request can trigger it.` + : `This GET handler's side effect (${sideEffectInHandler}) is prone to CSRF vulnerabilities, since prefetching or a forged request can trigger it.`; context.report({ node, message }); return; } + + const helpers = collectCalledSameFileHelpers(handlerBody, resolveBinding); + for (const { body: helperBody, helperFunction, callExpression } of helpers) { + const helperParameterSafeBindings = collectHelperParameterSafeBindings( + callExpression, + helperFunction, + locallyScopedSafeBindings, + ); + const effectiveSafeBindings = new Set([ + ...locallyScopedSafeBindings, + ...helperParameterSafeBindings, + ]); + + const sideEffectInHelper = findSideEffect(helperBody, { + locallyScopedSafeBindings: effectiveSafeBindings, + locallyScopedCookieBindings, + }); + if (sideEffectInHelper) { + const message = mutatingSegment + ? `This GET handler on the "/${mutatingSegment}" route performs a side effect (${sideEffectInHelper}) and is prone to CSRF vulnerabilities, since prefetching or a forged request can trigger it.` + : `This GET handler's side effect (${sideEffectInHelper}) is prone to CSRF vulnerabilities, since prefetching or a forged request can trigger it.`; + context.report({ node, message }); + return; + } + } } }, }; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/performance/async-defer-await.regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/performance/async-defer-await.regressions.test.ts index f03c9a076e..f5b5ab7563 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/performance/async-defer-await.regressions.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/performance/async-defer-await.regressions.test.ts @@ -498,4 +498,39 @@ describe("performance/async-defer-await — regressions", () => { expect(result.parseErrors).toEqual([]); expect(result.diagnostics).toHaveLength(1); }); + + it("stays silent on a run.live liveness guard in a React effect", () => { + const result = runRule( + asyncDeferAwait, + ` + declare const refreshSession: () => Promise; + declare const setOk: (value: boolean) => void; + const run = { live: true }; + export const effect = async () => { + const refreshed = await refreshSession(); + if (!run.live) return; + setOk(refreshed); + }; + `, + ); + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics).toHaveLength(0); + }); + + it("still reports guards with unrelated names that contain live", () => { + const result = runRule( + asyncDeferAwait, + ` + declare const loadDelivery: () => Promise; + declare const deliverNow: boolean; + export const deliver = async () => { + const payload = await loadDelivery(); + if (!deliverNow) return; + console.log(payload); + }; + `, + ); + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics).toHaveLength(1); + }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/performance/async-defer-await.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/performance/async-defer-await.ts index 7a1a29beab..e063f0877b 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/performance/async-defer-await.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/performance/async-defer-await.ts @@ -119,6 +119,8 @@ const CANCELLATION_GUARD_NAMES: ReadonlySet = new Set([ "isActive", "stale", "isStale", + "live", + "isLive", "ignore", "signal", "abortSignal", @@ -202,7 +204,7 @@ const isCancellationGuardTest = (test: EsTreeNode | null): boolean => { // `controller.signal.aborted`, `this._destroyed`, `batch.aborted`, // `seq !== getSeq.current`, `token !== runToken`. for (const name of collectAllTestNames(test)) { - if (isCancellationLikeName(name)) return true; + if (CANCELLATION_GUARD_NAMES.has(name) || isCancellationLikeName(name)) return true; } return testReadsRefCurrent(test); }; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/effect-needs-cleanup-issue-1756.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/effect-needs-cleanup-issue-1756.test.ts new file mode 100644 index 0000000000..1f0839f6ca --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/effect-needs-cleanup-issue-1756.test.ts @@ -0,0 +1,273 @@ +import { describe, expect, it } from "vite-plus/test"; +import { runRule } from "../../../test-utils/run-rule.js"; +import { effectNeedsCleanup } from "./effect-needs-cleanup.js"; + +const runEffectNeedsCleanup = (code: string) => runRule(effectNeedsCleanup, code); + +describe("effect-needs-cleanup issue #1756", () => { + it("accepts chained timers cleared by the returned cleanup helper", () => { + const result = runEffectNeedsCleanup(` + import { useEffect, useState } from "react"; + + const STEPS = [0, 400, 800]; + + const ChainedTimer = () => { + const [frame, setFrame] = useState(0); + const [loop, setLoop] = useState(0); + useEffect(() => { + let cancelled = false; + let timer: ReturnType | null = null; + let index = 0; + const startedAt = Date.now(); + const clearTimer = () => { + if (timer !== null) { + clearTimeout(timer); + timer = null; + } + }; + const scheduleAt = (at: number, callback: () => void) => { + timer = setTimeout(callback, Math.max(0, at - (Date.now() - startedAt))); + }; + const advance = () => { + timer = null; + if (cancelled) return; + setFrame(index); + index += 1; + if (index < STEPS.length) { + scheduleAt(STEPS[index], advance); + } else { + scheduleAt(2000, () => { + if (!cancelled) setLoop((count) => count + 1); + }); + } + }; + const onVisibilityChange = () => { + if (cancelled) return; + if (document.hidden) clearTimer(); + else setLoop((count) => count + 1); + }; + document.addEventListener("visibilitychange", onVisibilityChange); + if (!document.hidden) scheduleAt(STEPS[0], advance); + return () => { + cancelled = true; + clearTimer(); + document.removeEventListener("visibilitychange", onVisibilityChange); + }; + }, [loop]); + return {frame}; + }; + `); + + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics).toHaveLength(0); + }); + + it("reports timer handle overwrites before the previous timer fires", () => { + const result = runEffectNeedsCleanup(` + import { useEffect } from "react"; + + const UnsafeTimers = () => { + useEffect(() => { + let timer: ReturnType | null = null; + const schedule = () => { + timer = setTimeout(() => {}, 1000); + }; + schedule(); + schedule(); + return () => { + if (timer !== null) clearTimeout(timer); + }; + }, []); + return null; + }; + `); + + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics.length).toBeGreaterThan(0); + }); + + it("accepts a guarded timer scheduled through a helper after an await", () => { + const result = runEffectNeedsCleanup(` + import { useEffect, useState } from "react"; + + declare function refresh(): Promise<"refreshed" | "invalid">; + + const AsyncBoundedTimer = () => { + const [unproven, setUnproven] = useState(false); + useEffect(() => { + const run = { live: true }; + let settleTimer: ReturnType | null = null; + const refreshAndBound = () => { + settleTimer = setTimeout(() => { + settleTimer = null; + if (run.live) setUnproven(true); + }, 4000); + }; + void (async () => { + const outcome = await refresh(); + if (!run.live) return; + if (outcome === "refreshed") { + refreshAndBound(); + return; + } + setUnproven(true); + })(); + return () => { + run.live = false; + if (settleTimer !== null) clearTimeout(settleTimer); + }; + }, []); + return {String(unproven)}; + }; + `); + + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics).toHaveLength(0); + }); + + it("reports a helper timer scheduled after an await without a lifecycle guard", () => { + const result = runEffectNeedsCleanup(` + import { useEffect } from "react"; + + declare function refresh(): Promise; + + const UnsafeAsyncTimer = () => { + useEffect(() => { + let timer: ReturnType | null = null; + const schedule = () => { + timer = setTimeout(() => {}, 1000); + }; + void (async () => { + await refresh(); + schedule(); + })(); + return () => { + if (timer !== null) clearTimeout(timer); + }; + }, []); + return null; + }; + `); + + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics.length).toBeGreaterThan(0); + }); + + it("reports an interruption between the lifecycle guard and timer helper", () => { + const result = runEffectNeedsCleanup(` + import { useEffect } from "react"; + + declare function refresh(): Promise; + + const UnsafeInterruptedTimer = () => { + useEffect(() => { + const run = { live: true }; + let timer: ReturnType | null = null; + const schedule = () => { + timer = setTimeout(() => {}, 1000); + }; + void (async () => { + await refresh(); + if (!run.live) return; + await refresh(); + schedule(); + })(); + return () => { + run.live = false; + if (timer !== null) clearTimeout(timer); + }; + }, []); + return null; + }; + `); + + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics.length).toBeGreaterThan(0); + }); + + it("reports guarded async timer work started repeatedly by the effect", () => { + const result = runEffectNeedsCleanup(` + import { useEffect } from "react"; + + declare function refresh(): Promise; + + const UnsafeRepeatedAsyncTimer = () => { + useEffect(() => { + const run = { live: true }; + let timer: ReturnType | null = null; + const schedule = () => { + timer = setTimeout(() => {}, 1000); + }; + for (let index = 0; index < 2; index += 1) { + void (async () => { + await refresh(); + if (!run.live) return; + schedule(); + })(); + } + return () => { + run.live = false; + if (timer !== null) clearTimeout(timer); + }; + }, []); + return null; + }; + `); + + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics.length).toBeGreaterThan(0); + }); + + it("accepts a listener released by an abort handler when cleanup aborts the controller", () => { + const result = runEffectNeedsCleanup(` + import { useEffect, useState } from "react"; + + const AbortTeardown = () => { + const [visible, setVisible] = useState(true); + useEffect(() => { + const controller = new AbortController(); + const { signal } = controller; + const onChange = () => setVisible(!document.hidden); + document.addEventListener("visibilitychange", onChange); + signal.addEventListener( + "abort", + () => { + document.removeEventListener("visibilitychange", onChange); + }, + { once: true }, + ); + return () => { + controller.abort(); + }; + }, []); + return {visible ? "visible" : "hidden"}; + }; + `); + + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics).toHaveLength(0); + }); + + it("reports an abort handler that removes a different listener", () => { + const result = runEffectNeedsCleanup(` + import { useEffect } from "react"; + + const UnsafeAbortTeardown = () => { + useEffect(() => { + const controller = new AbortController(); + const onChange = () => {}; + const otherHandler = () => {}; + document.addEventListener("visibilitychange", onChange); + controller.signal.addEventListener("abort", () => { + document.removeEventListener("visibilitychange", otherHandler); + }); + return () => controller.abort(); + }, []); + return null; + }; + `); + + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics.length).toBeGreaterThan(0); + }); +}); 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 6cbdbf6c0d..afb2184199 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 @@ -2641,6 +2641,123 @@ const getListenerAbortControllerKey = ( return null; }; +const resolveAbortSignalControllerKey = ( + signalExpression: EsTreeNode, + context: RuleContext, +): string | null => { + const unwrappedSignal = stripParenExpression(signalExpression); + if ( + isNodeOfType(unwrappedSignal, "MemberExpression") && + !unwrappedSignal.computed && + isNodeOfType(unwrappedSignal.property, "Identifier") && + unwrappedSignal.property.name === "signal" + ) { + return resolveExpressionKey(unwrappedSignal.object, context); + } + if (!isNodeOfType(unwrappedSignal, "Identifier")) return null; + const signalSymbol = context.scopes.symbolFor(unwrappedSignal); + if (!signalSymbol) return null; + const signalInitializer = signalSymbol.initializer + ? stripParenExpression(signalSymbol.initializer) + : null; + if ( + isNodeOfType(signalInitializer, "MemberExpression") && + !signalInitializer.computed && + isNodeOfType(signalInitializer.property, "Identifier") && + signalInitializer.property.name === "signal" + ) { + return resolveExpressionKey(signalInitializer.object, context); + } + const bindingProperty = signalSymbol.bindingIdentifier.parent; + if ( + !isNodeOfType(bindingProperty, "Property") || + getStaticPropertyKeyName(bindingProperty) !== "signal" || + !isNodeOfType(bindingProperty.parent, "ObjectPattern") || + !isNodeOfType(bindingProperty.parent.parent, "VariableDeclarator") || + !bindingProperty.parent.parent.init + ) { + return null; + } + return resolveExpressionKey(bindingProperty.parent.parent.init, context); +}; + +const getDelegatedListenerAbortControllerKey = ( + usage: SubscribeLikeUsage, + context: RuleContext, +): string | null => { + if ( + usage.registrationVerbName !== "addEventListener" || + !isNodeOfType(usage.node, "CallExpression") + ) { + return null; + } + const registrationCall = usage.node; + const registrationOwner = findEnclosingFunction(usage.node); + if (!registrationOwner || !isFunctionLike(registrationOwner)) return null; + let matchingControllerKey: string | null = null; + walkAst(registrationOwner.body, (child: EsTreeNode) => { + if (matchingControllerKey) return false; + if (child !== registrationOwner.body && isFunctionLike(child)) return false; + if (!isNodeOfType(child, "CallExpression") || getCalleeName(child) !== "addEventListener") { + return; + } + const callee = stripParenExpression(child.callee); + if (!isNodeOfType(callee, "MemberExpression")) return; + const eventName = child.arguments[0] ? stripParenExpression(child.arguments[0]) : null; + if (!isNodeOfType(eventName, "Literal") || eventName.value !== "abort") return; + const controllerKey = resolveAbortSignalControllerKey(callee.object, context); + if (!controllerKey) return; + const handler = resolveStableValue(child.arguments[1], context); + if (!handler || !isFunctionLike(handler) || handler.async || handler.generator) return; + const matchingRemovalCalls: EsTreeNode[] = []; + walkAst(handler.body, (handlerChild: EsTreeNode) => { + if (handlerChild !== handler.body && isFunctionLike(handlerChild)) return false; + if (!isNodeOfType(handlerChild, "CallExpression")) return; + const removalCallee = stripParenExpression(handlerChild.callee); + if ( + !isNodeOfType(removalCallee, "MemberExpression") || + getCalleeName(handlerChild) !== "removeEventListener" || + resolveResourceIdentityKey(removalCallee.object, context) !== usage.receiverKey || + resolveResourceIdentityKey(handlerChild.arguments[0], context) !== usage.eventKey || + resolveResourceIdentityKey(handlerChild.arguments[1], context) !== usage.handlerKey || + !doEventListenerCapturesMatch( + registrationCall.arguments[2], + handlerChild.arguments[2], + context, + true, + ) + ) { + return; + } + matchingRemovalCalls.push(handlerChild); + }); + if (doNodesCoverEveryPathFromFunctionEntry(handler, matchingRemovalCalls, context)) { + matchingControllerKey = controllerKey; + return false; + } + }); + return matchingControllerKey; +}; + +const getAbortSignalListenerControllerKey = ( + usage: SubscribeLikeUsage, + context: RuleContext, +): string | null => { + if ( + usage.registrationVerbName !== "addEventListener" || + !isNodeOfType(usage.node, "CallExpression") + ) { + return null; + } + const eventName = usage.node.arguments[0] ? stripParenExpression(usage.node.arguments[0]) : null; + const callee = stripParenExpression(usage.node.callee); + return isNodeOfType(eventName, "Literal") && + eventName.value === "abort" && + isNodeOfType(callee, "MemberExpression") + ? resolveAbortSignalControllerKey(callee.object, context) + : null; +}; + const findEnclosingForEachCall = (node: EsTreeNode): EsTreeNodeOfType<"CallExpression"> | null => { const callbackNode = isFunctionLike(node) ? node : findEnclosingFunction(node); if ( @@ -3990,8 +4107,9 @@ const collectDeferredUsageGuardStates = ( callback: EsTreeNode, usageNode: EsTreeNode, context: RuleContext, + allowAsyncCallback = false, ): BooleanGuardState[] => { - if (!isFunctionLike(callback) || callback.async) return []; + if (!isFunctionLike(callback) || (callback.async && !allowAsyncCallback)) return []; const guardStates: BooleanGuardState[] = []; walkAst(callback.body, (child: EsTreeNode) => { if (child !== callback.body && isFunctionLike(child)) return false; @@ -4129,6 +4247,70 @@ const isEffectLocalLifecycleGuard = ( }); }; +const isEffectLocalObjectLifecycleGuard = ( + callback: EsTreeNode, + guardState: BooleanGuardState, + cleanupFunctions: ReadonlyArray, + context: RuleContext, +): boolean => { + const guardMemberExpressions: EsTreeNodeOfType<"MemberExpression">[] = []; + walkAst(guardState.guardNode, (child: EsTreeNode) => { + if ( + guardMemberExpressions.length === 0 && + isNodeOfType(child, "MemberExpression") && + !child.computed && + isNodeOfType(child.object, "Identifier") && + isNodeOfType(child.property, "Identifier") && + resolveExpressionKey(child, context) === guardState.key + ) { + guardMemberExpressions.push(child); + return false; + } + }); + const guardMemberExpression = guardMemberExpressions[0]; + if (!guardMemberExpression) return false; + const objectSymbol = context.scopes.symbolFor(guardMemberExpression.object); + const initializer = objectSymbol?.initializer + ? stripParenExpression(objectSymbol.initializer) + : null; + if ( + !objectSymbol || + objectSymbol.kind !== "const" || + !isNodeOfType(objectSymbol.declarationNode, "VariableDeclarator") || + findEnclosingFunction(objectSymbol.declarationNode) !== callback || + !isNodeOfType(initializer, "ObjectExpression") || + initializer.properties.length !== 1 + ) { + return false; + } + const initialProperty = initializer.properties[0]; + if ( + !isNodeOfType(initialProperty, "Property") || + getStaticPropertyKeyName(initialProperty) !== getStaticPropertyKeyName(guardMemberExpression) || + readStaticBoolean(initialProperty.value) !== !guardState.value + ) { + return false; + } + return objectSymbol.references.every((reference) => { + const memberExpression = getOutermostMemberReference(reference.identifier); + if ( + !isNodeOfType(memberExpression, "MemberExpression") || + resolveExpressionKey(memberExpression, context) !== guardState.key + ) { + return false; + } + if (!isWithinAssignmentTarget(reference.identifier)) return true; + const assignment = memberExpression.parent; + return ( + isNodeOfType(assignment, "AssignmentExpression") && + assignment.operator === "=" && + assignment.left === memberExpression && + readStaticBoolean(assignment.right) === guardState.value && + cleanupFunctions.includes(findEnclosingFunction(assignment) ?? assignment) + ); + }); +}; + const hasPotentialInterruptionAfterGuard = ( callback: EsTreeNode, guardState: BooleanGuardState, @@ -4163,6 +4345,60 @@ const hasPotentialInterruptionAfterGuard = ( return hasPotentialInterruption; }; +const isDeferredHelperInvocationProtectedByEffectLifecycleGuard = ( + callback: EsTreeNode, + invocationOwner: EsTreeNode, + invocationCall: EsTreeNode, + cleanupReturns: ReadonlyArray, + context: RuleContext, +): boolean => { + const invocationOwnerRoot = findTransparentExpressionRoot(invocationOwner); + const directInvocation = + isNodeOfType(invocationOwnerRoot.parent, "CallExpression") && + invocationOwnerRoot.parent.callee === invocationOwnerRoot + ? invocationOwnerRoot.parent + : findSingleDirectInvocation(invocationOwner, callback, context); + if ( + !isFunctionLike(invocationOwner) || + !invocationOwner.async || + invocationOwner.generator || + !directInvocation || + findEnclosingFunction(directInvocation) !== callback || + !collectEffectInvokedFunctions(callback, context.scopes).has(invocationOwner) + ) { + return false; + } + let invocationAncestor = directInvocation.parent; + while (invocationAncestor && invocationAncestor !== callback) { + if ( + isNodeOfType(invocationAncestor, "ForStatement") || + isNodeOfType(invocationAncestor, "ForInStatement") || + isNodeOfType(invocationAncestor, "ForOfStatement") || + isNodeOfType(invocationAncestor, "WhileStatement") || + isNodeOfType(invocationAncestor, "DoWhileStatement") + ) { + return false; + } + invocationAncestor = invocationAncestor.parent; + } + const cleanupFunctions = cleanupReturns.flatMap((cleanupReturn) => { + if (!isNodeOfType(cleanupReturn, "ReturnStatement") || !cleanupReturn.argument) return []; + const cleanupFunction = resolveStableValue(cleanupReturn.argument, context); + return cleanupFunction && isFunctionLike(cleanupFunction) ? [cleanupFunction] : []; + }); + if (cleanupFunctions.length !== cleanupReturns.length) return false; + return collectDeferredUsageGuardStates(invocationOwner, invocationCall, context, true).some( + (guardState) => + (isEffectLocalLifecycleGuard(callback, guardState, cleanupFunctions, context) || + isEffectLocalObjectLifecycleGuard(callback, guardState, cleanupFunctions, context)) && + !hasPotentialInterruptionAfterGuard(invocationOwner, guardState, invocationCall, context) && + !deferredUsageWritesGuardBeforeUsage(invocationOwner, invocationCall, guardState, context) && + cleanupReturns.every((cleanupReturn) => + cleanupReturnInvalidatesGuard(cleanupReturn, guardState, context), + ), + ); +}; + const getNumericReactRefCurrentKey = ( expression: EsTreeNode, context: RuleContext, @@ -4781,10 +5017,128 @@ const getOutermostMemberReference = (identifier: EsTreeNode): EsTreeNode => { return findTransparentExpressionRoot(expression); }; +const getTimerCallbackIdentityKeys = ( + usage: SubscribeLikeUsage, + context: RuleContext, +): ReadonlySet => { + const callbackIdentityKeys = new Set(); + const invocationCallbackIdentityKeys = new Set(); + const callbackKey = getUsageCallbackKey(usage, context); + if (callbackKey) callbackIdentityKeys.add(callbackKey); + if (usage.kind !== "timer" || !isNodeOfType(usage.node, "CallExpression")) { + return callbackIdentityKeys; + } + const callbackArgument = usage.node.arguments[0]; + if (!callbackArgument || !isNodeOfType(callbackArgument, "Identifier")) { + return callbackIdentityKeys; + } + const callbackSymbol = context.scopes.symbolFor(callbackArgument); + const usageFunction = callbackSymbol + ? findEnclosingFunction(callbackSymbol.bindingIdentifier) + : null; + if (!callbackSymbol || !usageFunction || !isFunctionLike(usageFunction)) { + return callbackIdentityKeys; + } + const callbackParameterIndex = usageFunction.params.findIndex( + (parameter) => parameter === callbackSymbol.bindingIdentifier, + ); + const functionBindingIdentifier = getFunctionBindingIdentifier(usageFunction); + const functionSymbol = functionBindingIdentifier + ? context.scopes.symbolFor(functionBindingIdentifier) + : null; + if (callbackParameterIndex < 0 || !functionSymbol) return callbackIdentityKeys; + for (const reference of functionSymbol.references) { + const invocationCall = findDirectCallForReference(reference.identifier); + if (!isNodeOfType(invocationCall, "CallExpression")) { + return callbackIdentityKeys; + } + const invocationArgument = invocationCall.arguments[callbackParameterIndex]; + if (!invocationArgument || !isAstNode(invocationArgument)) { + return callbackIdentityKeys; + } + const invocationCallbackKey = resolveExpressionKey(invocationArgument, context); + if (invocationCallbackKey) invocationCallbackIdentityKeys.add(invocationCallbackKey); + } + for (const identityKey of invocationCallbackIdentityKeys) { + callbackIdentityKeys.add(identityKey); + } + return callbackIdentityKeys; +}; + +const isTimerCallbackForSameHandle = ( + functionNode: EsTreeNode, + usage: SubscribeLikeUsage, + allUsages: ReadonlyArray, + context: RuleContext, +): boolean => { + if (!isFunctionLike(functionNode) || usage.handleKey === null) return false; + const functionIdentityKeys = getFunctionIdentityKeys(functionNode, context); + return allUsages.some((candidateUsage) => { + if ( + candidateUsage.kind !== "timer" || + candidateUsage.handleKey !== usage.handleKey || + findEnclosingFunction(candidateUsage.node) === functionNode + ) { + return false; + } + const candidateCallbackIdentityKeys = getTimerCallbackIdentityKeys(candidateUsage, context); + return [...functionIdentityKeys].some((identityKey) => + candidateCallbackIdentityKeys.has(identityKey), + ); + }); +}; + +const hasOnlyOwnedTimerHelperInvocations = ( + callback: EsTreeNode, + usage: SubscribeLikeUsage, + usageFunction: EsTreeNode, + functionSymbol: SymbolDescriptor, + context: RuleContext, +): boolean => { + const invocationsByOwner = new Map(); + let directEffectInvocationCount = 0; + for (const reference of functionSymbol.references) { + const invocationCall = findDirectCallForReference(reference.identifier); + const invocationOwner = invocationCall ? findEnclosingFunction(invocationCall) : null; + if (!invocationCall || !invocationOwner || !isFunctionLike(invocationOwner)) return false; + if (invocationOwner === callback) { + directEffectInvocationCount += 1; + } else if (!isTimerCallbackForSameHandle(invocationOwner, usage, [usage], context)) { + return false; + } + const ownerInvocations = invocationsByOwner.get(invocationOwner) ?? []; + ownerInvocations.push(invocationCall); + invocationsByOwner.set(invocationOwner, ownerInvocations); + } + if (directEffectInvocationCount > 1) return false; + return [...invocationsByOwner.entries()].every(([invocationOwner, invocations]) => + invocations.every((invocation, invocationIndex) => + invocations + .slice(invocationIndex + 1) + .every( + (laterInvocation) => + !canNodeReachLaterNodeWithinFunction( + invocation, + laterInvocation, + invocationOwner, + context, + ) && + !canNodeReachLaterNodeWithinFunction( + laterInvocation, + invocation, + invocationOwner, + context, + ), + ), + ), + ); +}; + const hasOnlySafeHandleStorageAssignments = ( usage: SubscribeLikeUsage, handleStorageSymbol: SymbolDescriptor, usageAssignment: EsTreeNodeOfType<"AssignmentExpression">, + allUsages: ReadonlyArray, context: RuleContext, ): boolean => handleStorageSymbol.references.every((reference) => { @@ -4803,13 +5157,32 @@ const hasOnlySafeHandleStorageAssignments = ( return false; } const assignedValue = stripParenExpression(assignment.right); + const assignmentOwner = findEnclosingFunction(assignment); + const assignedTimerUsage = allUsages.find( + (candidateUsage) => + candidateUsage.kind === "timer" && + candidateUsage.node === assignedValue && + candidateUsage.handleKey === usage.handleKey, + ); + const currentUsageOwner = findEnclosingFunction(usage.node); + const currentUsageOwnerKeys = currentUsageOwner + ? getFunctionIdentityKeys(currentUsageOwner, context) + : new Set(); + if ( + assignedTimerUsage && + ((assignmentOwner && + isTimerCallbackForSameHandle(assignmentOwner, usage, allUsages, context)) || + currentUsageOwnerKeys.has(getUsageCallbackKey(assignedTimerUsage, context) ?? "")) + ) { + return true; + } const isNullishReset = (isNodeOfType(assignedValue, "Literal") && assignedValue.value === null) || (isNodeOfType(assignedValue, "Identifier") && assignedValue.name === "undefined" && context.scopes.isGlobalReference(assignedValue)); - const assignmentOwner = findEnclosingFunction(assignment); if (!isNullishReset || !assignmentOwner || !isFunctionLike(assignmentOwner)) return false; + if (isTimerCallbackForSameHandle(assignmentOwner, usage, allUsages, context)) return true; const matchingReleaseCalls: EsTreeNode[] = []; walkAst(assignmentOwner.body, (child: EsTreeNode) => { if (child !== assignmentOwner.body && isFunctionLike(child)) return false; @@ -4888,10 +5261,28 @@ const hasEffectOwnedNestedTimerCleanup = ( if ( handleStorageSymbol && isNodeOfType(usageAssignment, "AssignmentExpression") && - !hasOnlySafeHandleStorageAssignments(usage, handleStorageSymbol, usageAssignment, context) + !hasOnlySafeHandleStorageAssignments( + usage, + handleStorageSymbol, + usageAssignment, + allUsages, + context, + ) ) { return false; } + if (isTimerCallbackForSameHandle(usageFunction, usage, allUsages, context)) { + return true; + } + const functionBindingIdentifier = getFunctionBindingIdentifier(usageFunction); + const functionSymbol = functionBindingIdentifier + ? context.scopes.symbolFor(functionBindingIdentifier) + : null; + if (!functionSymbol || functionSymbol.references.length === 0) return false; + const singleInvocationCall = + functionSymbol.references.length === 1 + ? findDirectCallForReference(functionSymbol.references[0].identifier) + : null; const isSelfRescheduling = isSelfReschedulingOneShotTimer( usage, usageFunction, @@ -4901,7 +5292,9 @@ const hasEffectOwnedNestedTimerCleanup = ( if ( handleStorageSymbol && !isSelfRescheduling && - !hasLiveHandleOverwriteProtection(usageFunction, usage, context) + !hasLiveHandleOverwriteProtection(usageFunction, usage, context) && + !singleInvocationCall && + !hasOnlyOwnedTimerHelperInvocations(callback, usage, usageFunction, functionSymbol, context) ) { return false; } @@ -4919,11 +5312,6 @@ const hasEffectOwnedNestedTimerCleanup = ( } usageAncestor = usageAncestor.parent; } - const functionBindingIdentifier = getFunctionBindingIdentifier(usageFunction); - const functionSymbol = functionBindingIdentifier - ? context.scopes.symbolFor(functionBindingIdentifier) - : null; - if (!functionSymbol || functionSymbol.references.length === 0) return false; const selfSchedulingReferences = functionSymbol.references.filter( (reference) => isSelfRescheduling && @@ -4969,7 +5357,19 @@ const hasEffectOwnedNestedTimerCleanup = ( if (!invocationOwner || !isFunctionLike(invocationOwner) || invocationOwner === usageFunction) { return false; } - if (invocationOwner.async || invocationOwner.generator) return false; + if (invocationOwner.generator) return false; + if (invocationOwner.async) { + return isDeferredHelperInvocationProtectedByEffectLifecycleGuard( + callback, + invocationOwner, + invocationCall, + cleanupReturns, + context, + ); + } + if (isTimerCallbackForSameHandle(invocationOwner, usage, allUsages, context)) { + return true; + } if (invocationOwner === callback) { return doMatchingNodesCoverEveryPathAfterUsage( resolveCleanupPathAnchor(invocationCall, callback, context), @@ -7316,7 +7716,10 @@ const doesReleaseCallMatchUsage = ( } if ( releaseVerbName === "abort" && - releaseReceiverKey === getListenerAbortControllerKey(usage, context) + releaseReceiverKey === + (getListenerAbortControllerKey(usage, context) ?? + getDelegatedListenerAbortControllerKey(usage, context) ?? + getAbortSignalListenerControllerKey(usage, context)) ) { return true; } diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/tanstack-query/query-mutation-missing-invalidation.regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/tanstack-query/query-mutation-missing-invalidation.regressions.test.ts index db8670f4a1..28cbebcbd1 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/tanstack-query/query-mutation-missing-invalidation.regressions.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/tanstack-query/query-mutation-missing-invalidation.regressions.test.ts @@ -311,4 +311,57 @@ describe("tanstack-query/query-mutation-missing-invalidation — regressions", ( expect(diagnostics).toHaveLength(1); expect(diagnostics[0].message).toContain("can leave"); }); + + it("stays silent for a magic-link delivery mutation", () => { + const sendMagicLink = runRule( + queryMutationMissingInvalidation, + `import { useMutation } from "@tanstack/react-query"; + export function useSendMagicLink() { + return useMutation({ + mutationFn: (params: { email: string }) => api.sendMagicLink(params), + }); + }`, + ); + expect(sendMagicLink.diagnostics).toHaveLength(0); + }); + + it("still reports generic send, notify, and email mutations", () => { + const diagnostics = runRule( + queryMutationMissingInvalidation, + `import { useMutation } from "@tanstack/react-query"; + export function useSendMessage() { + return useMutation({ + mutationFn: (message) => api.sendMessage(message), + }); + } + + export function useNotifyUser() { + return useMutation({ + mutationFn: (userId) => api.notifyUser(userId), + }); + } + + export function useEmailInvite() { + return useMutation({ + mutationFn: (email) => api.emailInvite(email), + }); + }`, + ).diagnostics; + + expect(diagnostics).toHaveLength(3); + }); + + it("stays silent when the magic-link signal comes from the mutation function", () => { + const result = runRule( + queryMutationMissingInvalidation, + `import { useMutation } from "@tanstack/react-query"; + export function useAuthenticationEmail() { + return useMutation({ + mutationFn: (email) => api.sendMagicLink(email), + }); + }`, + ); + + expect(result.diagnostics).toHaveLength(0); + }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/tanstack-query/query-mutation-missing-invalidation.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/tanstack-query/query-mutation-missing-invalidation.ts index c0cde4cb1e..37c2cf88ed 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/tanstack-query/query-mutation-missing-invalidation.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/tanstack-query/query-mutation-missing-invalidation.ts @@ -50,6 +50,7 @@ const READ_ONLY_MUTATION_WORDS = new Set([ "oauth", "pairing", "sign", + "magiclink", ]); // `sign` followed by one of these is an auth ACTION (signIn / signUp / diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/utils/collect-helper-parameter-safe-bindings.ts b/packages/oxlint-plugin-react-doctor/src/plugin/utils/collect-helper-parameter-safe-bindings.ts new file mode 100644 index 0000000000..3c1e1a85c0 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/utils/collect-helper-parameter-safe-bindings.ts @@ -0,0 +1,31 @@ +import type { EsTreeNode } from "./es-tree-node.js"; +import { isFunctionLike } from "./is-function-like.js"; +import { isNodeOfType } from "./is-node-of-type.js"; + +export const collectHelperParameterSafeBindings = ( + callExpression: EsTreeNode, + helperFunction: EsTreeNode, + locallyScopedSafeBindings: Set, +): Set => { + const parameterSafeBindings = new Set(); + + if (!isNodeOfType(callExpression, "CallExpression") || !isFunctionLike(helperFunction)) { + return parameterSafeBindings; + } + + const helperParameters = helperFunction.params ?? []; + if (helperParameters.length === 0) return parameterSafeBindings; + + for (let argumentIndex = 0; argumentIndex < callExpression.arguments.length; argumentIndex++) { + const argument = callExpression.arguments[argumentIndex]; + if (!isNodeOfType(argument, "Identifier")) continue; + if (!locallyScopedSafeBindings.has(argument.name)) continue; + + const correspondingParameter = helperParameters[argumentIndex]; + if (!isNodeOfType(correspondingParameter, "Identifier")) continue; + + parameterSafeBindings.add(correspondingParameter.name); + } + + return parameterSafeBindings; +}; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/utils/has-directive.ts b/packages/oxlint-plugin-react-doctor/src/plugin/utils/has-directive.ts index 7d37fe5b05..66ded44124 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/utils/has-directive.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/utils/has-directive.ts @@ -1,9 +1,20 @@ import type { EsTreeNode } from "./es-tree-node.js"; import { isNodeOfType } from "./is-node-of-type.js"; -export const hasDirective = (programNode: EsTreeNode, directive: string): boolean => { - if (!isNodeOfType(programNode, "Program")) return false; - for (const statement of programNode.body) { +export const hasDirective = (node: EsTreeNode, directive: string): boolean => { + let statements: EsTreeNode[] | null = null; + if (isNodeOfType(node, "Program")) { + statements = node.body; + } else if ( + (isNodeOfType(node, "FunctionDeclaration") || + isNodeOfType(node, "FunctionExpression") || + isNodeOfType(node, "ArrowFunctionExpression")) && + isNodeOfType(node.body, "BlockStatement") + ) { + statements = node.body.body; + } + if (statements === null) return false; + for (const statement of statements) { if (!isNodeOfType(statement, "ExpressionStatement") || statement.directive === undefined) { return false; } diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/utils/has-react-compiler-opt-out-directive.ts b/packages/oxlint-plugin-react-doctor/src/plugin/utils/has-react-compiler-opt-out-directive.ts new file mode 100644 index 0000000000..fd1940e593 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/utils/has-react-compiler-opt-out-directive.ts @@ -0,0 +1,7 @@ +import type { EsTreeNode } from "./es-tree-node.js"; +import { hasDirective } from "./has-directive.js"; + +const REACT_COMPILER_OPT_OUT_DIRECTIVES = new Set(["use no memo", "use no forget"]); + +export const hasReactCompilerOptOutDirective = (node: EsTreeNode): boolean => + [...REACT_COMPILER_OPT_OUT_DIRECTIVES].some((directive) => hasDirective(node, directive)); diff --git a/packages/react-doctor/src/cli/runtime-scan/constants.ts b/packages/react-doctor/src/cli/runtime-scan/constants.ts index a93b6fd1c1..f806830d93 100644 --- a/packages/react-doctor/src/cli/runtime-scan/constants.ts +++ b/packages/react-doctor/src/cli/runtime-scan/constants.ts @@ -43,7 +43,7 @@ export const RUNTIME_SCAN_BROWSER_HEIGHT_PX = 900; export const RUNTIME_SCAN_DURATION_PRECISION_DIGITS = 1; export const RUNTIME_SCAN_TRACE_FILE_MODE = 0o600; export const RUNTIME_SCAN_TRACE_FILE_EXTENSION = ".json.gz"; -export const RUNTIME_SCAN_TRACING_COMPLETE_TIMEOUT_MS = 10_000; +export const RUNTIME_SCAN_TRACING_COMPLETE_TIMEOUT_MS = 60_000; export const RUNTIME_SCAN_PROBE_RELATIVE_PATH = "runtime-scan/browser-probe.iife.js"; export const RUNTIME_SCAN_PROBE_SNAPSHOT_BINDING_NAME_PLACEHOLDER = "__REACT_DOCTOR_RUNTIME_SCAN_CAPTURE__"; diff --git a/packages/react-doctor/src/cli/runtime-scan/record-runtime-trace.ts b/packages/react-doctor/src/cli/runtime-scan/record-runtime-trace.ts index 641c521cde..37a07f5151 100644 --- a/packages/react-doctor/src/cli/runtime-scan/record-runtime-trace.ts +++ b/packages/react-doctor/src/cli/runtime-scan/record-runtime-trace.ts @@ -158,15 +158,27 @@ const startDevtoolsTrace = async (cdpSession: CDPSession): Promise => { } }; -const endDevtoolsTrace = async (cdpSession: CDPSession): Promise => { +interface DevtoolsTraceSession { + once(event: "Tracing.tracingComplete", listener: (event: { stream?: string }) => void): void; + send(method: "Tracing.end"): Promise; +} + +export const endDevtoolsTrace = async ( + cdpSession: DevtoolsTraceSession, + timeoutMs = RUNTIME_SCAN_TRACING_COMPLETE_TIMEOUT_MS, +): Promise => { const traceComplete = new Promise<{ stream?: string }>((resolve) => { cdpSession.once("Tracing.tracingComplete", resolve); }); let timeoutHandle: ReturnType | undefined; const traceTimeout = new Promise((_resolve, reject) => { timeoutHandle = setTimeout(() => { - reject(new Error("Chrome did not finish the performance trace in time.")); - }, RUNTIME_SCAN_TRACING_COMPLETE_TIMEOUT_MS); + reject( + new CliInputError( + "Chrome did not finalize the performance trace within 60 seconds. Record a shorter interaction and retry.", + ), + ); + }, timeoutMs); }); let stream: string | undefined; try { diff --git a/packages/react-doctor/tests/regressions/js-performance-rules.test.ts b/packages/react-doctor/tests/regressions/js-performance-rules.test.ts index 924a3b7d6a..943d427cf8 100644 --- a/packages/react-doctor/tests/regressions/js-performance-rules.test.ts +++ b/packages/react-doctor/tests/regressions/js-performance-rules.test.ts @@ -1504,3 +1504,37 @@ describe("issue #543: js-tosorted-immutable is gated off for React Native / Expo expect(hits).toHaveLength(1); }); }); + +describe("performance/async-defer-await", () => { + it("stays silent on a run.live liveness guard in a React effect (issue #1758)", async () => { + const projectDir = setupReactProject(tempRoot, "async-defer-run-live", { + files: { + "src/liveness-guard.tsx": ` + "use client"; + import { useEffect, useState } from "react"; + + declare function refreshSession(): Promise; + + export function LivenessGuard() { + const [ok, setOk] = useState(null); + useEffect(() => { + const run = { live: true }; + void (async () => { + const refreshed = await refreshSession(); + if (!run.live) return; + setOk(refreshed); + })(); + return () => { + run.live = false; + }; + }, []); + return {String(ok)}; + } + `, + }, + }); + + const hits = await collectRuleHits(projectDir, "async-defer-await"); + expect(hits).toHaveLength(0); + }); +}); diff --git a/packages/react-doctor/tests/regressions/nextjs-get-side-effects.test.ts b/packages/react-doctor/tests/regressions/nextjs-get-side-effects.test.ts index e51a00e58a..3b8c1ec902 100644 --- a/packages/react-doctor/tests/regressions/nextjs-get-side-effects.test.ts +++ b/packages/react-doctor/tests/regressions/nextjs-get-side-effects.test.ts @@ -246,6 +246,106 @@ export async function GET() { expect(filterRule(diagnostics)).toHaveLength(0); }); + it("locally-built Headers passed to a helper that calls `.set()` (issue #1757)", async () => { + const diagnostics = await writeRouteAndLint( + "issue-1757-headers-helper", + "src/app/api/proxy/route.ts", + `import type { NextRequest } from "next/server"; + +function applyCachePolicy(responseHeaders: Headers) { + responseHeaders.set("Cache-Control", "public, max-age=3, stale-while-revalidate=10"); +} + +async function handler(request: NextRequest) { + const upstream = await fetch(\`https://api.example.com\${request.nextUrl.pathname}\`, { + method: request.method, + }); + const headers = new Headers(upstream.headers); + if (request.method === "GET" && upstream.ok) { + applyCachePolicy(headers); + } + return new Response(upstream.body, { status: upstream.status, headers }); +} + +export const GET = handler; +export const POST = handler; +`, + ); + expect(filterRule(diagnostics)).toHaveLength(0); + }); + + it("locally-built Headers in second parameter position", async () => { + const diagnostics = await writeRouteAndLint( + "issue-1757-second-param", + "src/app/api/cache/route.ts", + `import { NextResponse } from "next/server"; + +function setCacheHeaders(status: number, headers: Headers) { + headers.set("Cache-Control", "max-age=60"); +} + +export async function GET() { + const headers = new Headers(); + setCacheHeaders(200, headers); + return new NextResponse(null, { headers }); +} +`, + ); + expect(filterRule(diagnostics)).toHaveLength(0); + }); + + it("does not transfer a safe argument to a different helper", async () => { + const diagnostics = await writeRouteAndLint( + "issue-1757-helper-identity", + "src/app/api/config/route.ts", + `import { NextResponse } from "next/server"; + +const cache = new Map(); + +function applyHeaders(headers: Headers) { + headers.set("Cache-Control", "max-age=60"); +} + +function updateCache(cacheMap: Map) { + cacheMap.set("requests", Date.now()); +} + +export async function GET() { + const headers = new Headers(); + applyHeaders(headers); + updateCache(cache); + return new NextResponse(null, { headers }); +} +`, + ); + const hits = filterRule(diagnostics); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("cacheMap.set()"); + }); + + it("reports when one helper receives both safe and external receivers", async () => { + const diagnostics = await writeRouteAndLint( + "issue-1757-mixed-helper-calls", + "src/app/api/config/route.ts", + `import { NextResponse } from "next/server"; + +const cache = new Map(); + +function applyValue(target: Headers | Map) { + target.set("Cache-Control", "max-age=60"); +} + +export async function GET() { + const headers = new Headers(); + applyValue(headers); + applyValue(cache); + return new NextResponse(null, { headers }); +} +`, + ); + expect(filterRule(diagnostics)).toHaveLength(1); + }); + it("aliased read-only `headers()` — `const h = headers(); h.get('user-agent')`", async () => { const diagnostics = await writeRouteAndLint( "issue-206-headers-alias-read", @@ -434,6 +534,29 @@ export async function GET() { expect(hits[0].message).toContain("cookies().delete()"); }); + it("module-level Map passed to helper still fires (not locally scoped)", async () => { + const diagnostics = await writeRouteAndLint( + "issue-1757-module-map-helper", + "src/app/api/cache/route.ts", + `import { NextResponse } from "next/server"; + +const cache = new Map(); + +function updateCache(cacheMap: Map, key: string) { + cacheMap.set(key, Date.now()); +} + +export async function GET(req: Request) { + updateCache(cache, req.url); + return NextResponse.json({ ok: true }); +} +`, + ); + const hits = filterRule(diagnostics); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("cacheMap.set()"); + }); + it("read-only GET on a mutating route segment `/logout` does NOT fire", async () => { // The route name is a hint, not proof — a GET that only returns JSON has // no side effect for a forged request or prefetch to trigger. diff --git a/packages/react-doctor/tests/runtime-scan.test.ts b/packages/react-doctor/tests/runtime-scan.test.ts index 1d7cdce9ae..791c72d6c1 100644 --- a/packages/react-doctor/tests/runtime-scan.test.ts +++ b/packages/react-doctor/tests/runtime-scan.test.ts @@ -7,7 +7,9 @@ import { RUNTIME_SCAN_MAX_LOAF_ENTRIES, RUNTIME_SCAN_MAX_SNAPSHOT_PAYLOAD_BYTES, RUNTIME_SCAN_MAX_STRING_LENGTH, + RUNTIME_SCAN_TRACING_COMPLETE_TIMEOUT_MS, } from "../src/cli/runtime-scan/constants.js"; +import { endDevtoolsTrace } from "../src/cli/runtime-scan/record-runtime-trace.js"; import { formatRuntimeScanReport } from "../src/cli/runtime-scan/format-runtime-scan-report.js"; import { mergeRuntimeScanProbeSnapshots } from "../src/cli/runtime-scan/merge-runtime-scan-probe-snapshots.js"; import { @@ -20,6 +22,8 @@ import { sanitizeRuntimeUrl } from "../src/cli/runtime-scan/sanitize-runtime-url import type { RuntimeScanProbeSnapshot } from "../src/cli/runtime-scan/types.js"; import { scrubRunArguments } from "../src/cli/utils/scrub-run-arguments.js"; +const TEST_TRACE_TIMEOUT_MS = 1; + const snapshot: RuntimeScanProbeSnapshot = { timeOrigin: 1_000, finalUrl: "https://example.com/dashboard?token=secret#private", @@ -348,6 +352,38 @@ describe("runtime scan report", () => { }); describe("runtime scan input", () => { + it("allows 60 seconds for Chrome to finalize a trace", () => { + expect(RUNTIME_SCAN_TRACING_COMPLETE_TIMEOUT_MS).toBe(60_000); + }); + + it("returns the trace stream after Chrome reports completion", async () => { + let completeTrace: ((event: { stream?: string }) => void) | undefined; + const cdpSession = { + once: (_event: "Tracing.tracingComplete", listener: (event: { stream?: string }) => void) => { + completeTrace = listener; + }, + send: async (_method: "Tracing.end") => { + completeTrace?.({ stream: "trace-stream" }); + }, + }; + + await expect(endDevtoolsTrace(cdpSession)).resolves.toBe("trace-stream"); + }); + + it("reports an actionable error when Chrome does not finish a trace", async () => { + const cdpSession = { + once: ( + _event: "Tracing.tracingComplete", + _listener: (event: { stream?: string }) => void, + ) => {}, + send: async (_method: "Tracing.end") => {}, + }; + + await expect(endDevtoolsTrace(cdpSession, TEST_TRACE_TIMEOUT_MS)).rejects.toThrow( + "Record a shorter interaction and retry", + ); + }); + it("requires an explicit URL outside an interactive terminal", async () => { await expect(runtimeScanAction(undefined, { format: "text" })).rejects.toThrow( "A URL is required outside an interactive terminal",