Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/finite-next-data-redirect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"oxlint-plugin-react-doctor": patch
"eslint-plugin-react-doctor": patch
"react-doctor": patch
---

Preserve Next Pages data redirects when an effect only replaces the route using props explicitly returned by the page data function.
7 changes: 7 additions & 0 deletions .changeset/semantic-runtime-reference-resolution.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"oxlint-plugin-react-doctor": patch
"eslint-plugin-react-doctor": patch
"react-doctor": patch
---

Preserve runtime value references and writes when erased TypeScript declarations share their names, so provider exposure cannot bypass diagnostic proofs.
29 changes: 19 additions & 10 deletions packages/evals/tests/delete-vercel-snapshot-before-deadline.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import { Snapshot } from "@vercel/sandbox";
import { afterEach, describe, expect, it, vi } from "vite-plus/test";
import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test";
import { deleteVercelSnapshotBeforeDeadline } from "../src/utils/delete-vercel-snapshot-before-deadline.js";
const CONTROL_PLANE_TEST_TIMEOUT_MS = 5;
afterEach(() => vi.restoreAllMocks());
beforeEach(() => {
vi.useFakeTimers({ toFake: ["performance", "setTimeout", "clearTimeout"] });
});
afterEach(() => {
vi.useRealTimers();
vi.restoreAllMocks();
});

describe("deleteVercelSnapshotBeforeDeadline", () => {
it("recovers snapshots when the create response was lost", async () => {
Expand Down Expand Up @@ -32,13 +38,16 @@ describe("deleteVercelSnapshotBeforeDeadline", () => {
snapshotId: { value: "snapshot-name" },
delete: { value: () => new Promise<never>(() => undefined) },
});
await expect(
deleteVercelSnapshotBeforeDeadline({
credentials: {},
snapshotName: "builder",
snapshot,
deadlineMilliseconds: globalThis.performance.now() + CONTROL_PLANE_TEST_TIMEOUT_MS,
}),
).rejects.toThrow("Timed out deleting Vercel snapshot snapshot-name");
const deletion = deleteVercelSnapshotBeforeDeadline({
credentials: {},
snapshotName: "builder",
snapshot,
deadlineMilliseconds: globalThis.performance.now() + CONTROL_PLANE_TEST_TIMEOUT_MS,
});
const deletionAssertion = expect(deletion).rejects.toThrow(
"Timed out deleting Vercel snapshot snapshot-name",
);
await vi.advanceTimersByTimeAsync(CONTROL_PLANE_TEST_TIMEOUT_MS);
await deletionAssertion;
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// rule: no-effect-event-handler
// verdict: pass
// weakness: framework-gating
// source: independently synthetic
// file-path: src/pages/library.tsx
import { useEffect } from "react";
import { useRouter } from "next/router";
export const getServerSideProps = async () => ({ props: { nextPath: "/library", item: null } });
const LibraryPage = ({ nextPath, item }) => {
const router = useRouter();
useEffect(() => {
if (nextPath && !item) {
router.replace(nextPath);
}
}, [nextPath, item]);
return null;
};
export default LibraryPage;
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// rule: no-effect-event-handler
// weakness: provider-identity
// source: independent synthetic runtime binding regression
// verdict: pass
// file-path: src/pages/library.tsx
const require = (_source: string) => ({ useRouter: () => ({ replace: () => {} }) });
interface require {}
require("next/router").useRouter = () => ({ replace: () => {} });
import { useEffect } from "react";
import { useRouter } from "next/router";
export const getServerSideProps = async () => ({ props: { nextPath: "/library", item: null } });
const LibraryPage = ({ nextPath, item }) => {
const router = useRouter();
useEffect(() => {
if (nextPath && !item) {
router.replace(nextPath);
}
}, [nextPath, item]);
return null;
};
export default LibraryPage;
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// rule: no-effect-event-handler
// verdict: fail
// weakness: framework-gating
// source: independently synthetic
// file-path: src/pages/library.tsx
import { useEffect } from "react";
import { useRouter } from "next/router";
export const getServerSideProps = async () => ({ props: { nextPath: "/library", item: null } });
const LibraryPage = ({ nextPath, item }) => {
const router = useRouter();
useEffect(() => {
if (nextPath && !item) {
router.replace(nextPath);
toast("Moving");
}
}, [nextPath, item]);
return null;
};
export default LibraryPage;
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// rule: no-effect-event-handler
// weakness: provider-identity
// source: independent synthetic erased-provider regression
// verdict: fail
// file-path: src/pages/library.tsx
declare const globalThis: any;
globalThis.require("next/router").useRouter = () => ({ replace: () => {} });
import { useEffect } from "react";
import { useRouter } from "next/router";
export const getServerSideProps = async () => ({ props: { nextPath: "/library", item: null } });
const LibraryPage = ({ nextPath, item }) => {
const router = useRouter();
useEffect(() => {
if (nextPath && !item) {
router.replace(nextPath);
}
}, [nextPath, item]);
return null;
};
export default LibraryPage;
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// rule: no-effect-event-handler
// weakness: provider-identity
// source: independent synthetic merged imported provider exposure
// verdict: fail
// file-path: src/pages/library.tsx
const rewriteProvider = () => {
interface useRouter {}
mutate(useRouter);
};
rewriteProvider();
import { useEffect } from "react";
import { useRouter } from "next/router";
export const getServerSideProps = async () => ({ props: { nextPath: "/library", item: null } });
const LibraryPage = ({ nextPath, item }) => {
const router = useRouter();
useEffect(() => {
if (nextPath && !item) {
router.replace(nextPath);
}
}, [nextPath, item]);
return null;
};
export default LibraryPage;
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// rule: no-effect-event-handler
// weakness: provider-identity
// source: independent synthetic runtime binding regression
// verdict: fail
// file-path: src/pages/library.tsx
import type require = require("provider-types");
require("next/router").useRouter = () => ({ replace: () => {} });
import { useEffect } from "react";
import { useRouter } from "next/router";
export const getServerSideProps = async () => ({ props: { nextPath: "/library", item: null } });
const LibraryPage = ({ nextPath, item }) => {
const router = useRouter();
useEffect(() => {
if (nextPath && !item) {
router.replace(nextPath);
}
}, [nextPath, item]);
return null;
};
export default LibraryPage;
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// rule: no-effect-event-handler
// weakness: provider-identity
// source: independent synthetic erased namespace regression
// verdict: fail
// file-path: src/pages/library.tsx
namespace require {
export interface Marker {}
}
require("next/router").useRouter = () => ({ replace: () => {} });
import { useEffect } from "react";
import { useRouter } from "next/router";
export const getServerSideProps = async () => ({ props: { nextPath: "/library", item: null } });
const LibraryPage = ({ nextPath, item }) => {
const router = useRouter();
useEffect(() => {
if (nextPath && !item) {
router.replace(nextPath);
}
}, [nextPath, item]);
return null;
};
export default LibraryPage;
2 changes: 2 additions & 0 deletions packages/fuzz/src/snippet-pools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,8 @@ export const MODULE_SCOPE_SNIPPET_POOL = [
] as const;

export const SERVER_MODULE_PROGRAM_POOL = [
'import { useEffect } from "react";\nimport { useRouter } from "next/router";\nexport const getServerSideProps = async () => ({ props: { nextPath: "/library", item: null } });\nconst LibraryPage = ({ nextPath, item }) => {\n const router = useRouter();\n useEffect(() => {\n if (nextPath && !item) {\n router.replace(nextPath);\n }\n }, [nextPath, item]);\n return null;\n};\nexport default LibraryPage;\n',
'import { useEffect } from "react";\nimport { useRouter } from "next/router";\nexport const getServerSideProps = async () => ({ props: { nextPath: "/library", item: null } });\nconst LibraryPage = ({ nextPath, item }) => {\n const router = useRouter();\n useEffect(() => {\n if (nextPath && !item) {\n router.replace(nextPath);\n toast("Moving");\n }\n }, [nextPath, item]);\n return null;\n};\nexport default LibraryPage;\n',
`export default function FuzzDedupPage({ items }) {
return <FuzzList items={items} sortedItems={items.toSorted()} />;
}`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,7 @@ export const livenessFixtures: Readonly<Record<string, LivenessFixture>> = {
code: "function Game({ card }) {\n const [goldCardCount, setGoldCardCount] = useState(0);\n const [round, setRound] = useState(1);\n useEffect(() => { if (card.gold) setGoldCardCount(goldCardCount + 1); }, [card]);\n useEffect(() => { if (goldCardCount > 3) setRound(round + 1); }, [goldCardCount]);\n return null;\n }",
},
"no-effect-event-handler": {
settings: { "react-doctor": { capabilities: ["react", "nextjs"] } },
code: "\nimport { useEffect } from \"react\";\nconst Checkout = ({ status }) => {\n useEffect(() => {\n if (status !== 'submitted') {\n return;\n }\n toast('Order submitted!');\n }, [status]);\n return null;\n};\n",
},
"no-effect-event-in-deps": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,25 @@
import { describe, expect, it } from "vite-plus/test";
import { describe, expect, it, vi } from "vite-plus/test";
import { runRule } from "../../../test-utils/run-rule.js";
import { effectNeedsCleanup } from "./effect-needs-cleanup.js";

const SMALL_HANDLER_COUNT = 500;
const LARGE_HANDLER_COUNT = 2500;
const MEASUREMENT_SAMPLE_COUNT = 7;
const MAXIMUM_SCALING_MULTIPLIER = 15;

const traversalCounter = vi.hoisted(() => vi.fn());

vi.mock("../../utils/walk-ast.js", async (importOriginal) => {
const originalModule = await importOriginal<typeof import("../../utils/walk-ast.js")>();
return {
...originalModule,
walkAst: ((node, visitor) =>
originalModule.walkAst(node, (candidate) => {
traversalCounter();
return visitor(candidate);
})) satisfies typeof originalModule.walkAst,
};
});

const buildRetainedHandlersSource = (
handlerCount: number,
buildRegistration: (handlerIndex: number) => string,
Expand All @@ -23,20 +36,16 @@ const buildRetainedHandlersSource = (
return `const Component = () => {\n${handlers}\nreturn <>${buttons}</>;\n};`;
};

const measureDuration = (
const measureTraversalWork = (
handlerCount: number,
buildRegistration: (handlerIndex: number) => string,
): number => {
const source = buildRetainedHandlersSource(handlerCount, buildRegistration);
const sampleDurations = Array.from({ length: MEASUREMENT_SAMPLE_COUNT }, () => {
const startedAt = process.hrtime.bigint();
const result = runRule(effectNeedsCleanup, source);
expect(result.parseErrors).toEqual([]);
expect(result.diagnostics).toHaveLength(handlerCount);
return Number(process.hrtime.bigint() - startedAt);
});
sampleDurations.sort((firstDuration, secondDuration) => firstDuration - secondDuration);
return sampleDurations[Math.floor(sampleDurations.length / 2)] ?? Number.POSITIVE_INFINITY;
traversalCounter.mockClear();
const result = runRule(effectNeedsCleanup, source);
expect(result.parseErrors).toEqual([]);
expect(result.diagnostics).toHaveLength(handlerCount);
return traversalCounter.mock.calls.length;
};

describe("effect-needs-cleanup performance", () => {
Expand All @@ -52,11 +61,17 @@ describe("effect-needs-cleanup performance", () => {
},
]) {
it(`scales near-linearly across retained ${performanceCase.name}`, () => {
measureDuration(SMALL_HANDLER_COUNT, performanceCase.buildRegistration);
measureDuration(LARGE_HANDLER_COUNT, performanceCase.buildRegistration);
const smallDuration = measureDuration(SMALL_HANDLER_COUNT, performanceCase.buildRegistration);
const largeDuration = measureDuration(LARGE_HANDLER_COUNT, performanceCase.buildRegistration);
expect(largeDuration).toBeLessThan(smallDuration * MAXIMUM_SCALING_MULTIPLIER);
const smallTraversalCount = measureTraversalWork(
SMALL_HANDLER_COUNT,
performanceCase.buildRegistration,
);
const largeTraversalCount = measureTraversalWork(
LARGE_HANDLER_COUNT,
performanceCase.buildRegistration,
);
expect(smallTraversalCount).toBeGreaterThan(SMALL_HANDLER_COUNT);
expect(largeTraversalCount).toBeGreaterThan(smallTraversalCount);
expect(largeTraversalCount).toBeLessThan(smallTraversalCount * MAXIMUM_SCALING_MULTIPLIER);
});
}
});
Original file line number Diff line number Diff line change
@@ -1,48 +1,48 @@
import { describe, expect, it } from "vite-plus/test";
import { describe, expect, it, vi } from "vite-plus/test";
import { runRule } from "../../../test-utils/run-rule.js";
import { noBooleanToggleWithoutFunctionalUpdate } from "./no-boolean-toggle-without-functional-update.js";

const SMALL_SETTER_COUNT = 2_000;
const LARGE_SETTER_COUNT = 10_000;
const MEASUREMENT_SAMPLE_COUNT = 5;
const MAXIMUM_SCALING_MULTIPLIER = 18;
const INDEPENDENT_FUNCTION_COUNT = 2;
const asyncFunctionScanCounter = vi.hoisted(() => vi.fn());

vi.mock("../../utils/walk-ast.js", async (importOriginal) => {
const originalModule = await importOriginal<typeof import("../../utils/walk-ast.js")>();
return {
...originalModule,
walkAst: ((node, visitor) => {
if (node.type === "ArrowFunctionExpression" && "async" in node && node.async)
asyncFunctionScanCounter();
return originalModule.walkAst(node, visitor);
}) satisfies typeof originalModule.walkAst,
};
});

const buildAwaitedTogglesSource = (setterCount: number): string =>
`const C=()=>{const[open,setOpen]=useState(false);const run=async()=>{await load();${"setOpen(!open);".repeat(setterCount)}}}`;

const measureDuration = (source: string, setterCount: number): number => {
const startedAt = process.hrtime.bigint();
const measureFunctionScans = (source: string, setterCount: number): number => {
asyncFunctionScanCounter.mockClear();
const result = runRule(noBooleanToggleWithoutFunctionalUpdate, source);
const duration = Number(process.hrtime.bigint() - startedAt);
expect(result.parseErrors).toEqual([]);
expect(result.diagnostics).toHaveLength(setterCount);
return duration;
};

const medianDuration = (durations: readonly number[]): number => {
const sortedDurations = [...durations].sort(
(firstDuration, secondDuration) => firstDuration - secondDuration,
);
return sortedDurations[Math.floor(sortedDurations.length / 2)] ?? Number.POSITIVE_INFINITY;
return asyncFunctionScanCounter.mock.calls.length;
};

describe("no-boolean-toggle-without-functional-update performance", () => {
it("caches await reachability across many setters", () => {
const smallSource = buildAwaitedTogglesSource(SMALL_SETTER_COUNT);
const largeSource = buildAwaitedTogglesSource(LARGE_SETTER_COUNT);
measureDuration(smallSource, SMALL_SETTER_COUNT);
measureDuration(largeSource, LARGE_SETTER_COUNT);
const smallDurations: number[] = [];
const largeDurations: number[] = [];
// Interleaving keeps both sizes under the same runner load, and the median
// (unlike the minimum) cannot be won by a single quiet window that only a
// short run is brief enough to fit inside.
for (let sampleIndex = 0; sampleIndex < MEASUREMENT_SAMPLE_COUNT; sampleIndex += 1) {
smallDurations.push(measureDuration(smallSource, SMALL_SETTER_COUNT));
largeDurations.push(measureDuration(largeSource, LARGE_SETTER_COUNT));
}
expect(medianDuration(largeDurations)).toBeLessThan(
medianDuration(smallDurations) * MAXIMUM_SCALING_MULTIPLIER,
const smallFunctionScans = measureFunctionScans(smallSource, SMALL_SETTER_COUNT);
const largeFunctionScans = measureFunctionScans(largeSource, LARGE_SETTER_COUNT);
expect(smallFunctionScans).toBeGreaterThan(0);
expect(largeFunctionScans).toBe(smallFunctionScans);
const independentSource = smallSource + ";" + smallSource.replace("const C=", "const Other=");
const independentFunctionScans = measureFunctionScans(
independentSource,
SMALL_SETTER_COUNT * INDEPENDENT_FUNCTION_COUNT,
);
expect(independentFunctionScans).toBe(smallFunctionScans * INDEPENDENT_FUNCTION_COUNT);
});
});
Loading
Loading