From b754e44e260ba888035b8ec077b4da8ea4483e48 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Tue, 18 Aug 2026 21:51:25 -0300 Subject: [PATCH] test(guard): widen the client-bundle guard to every "use client" entry point (#10692) (#10700) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard shipped with #10695 watched two hand-picked modules. It now walks the static import graph from all 753 "use client" files in src/ (plus the two originally pinned entries), so the invariant is verified across the repo instead of where someone remembered to look. Full sweep runs in ~750ms. Two exclusions make that practical: - `import type` is not an edge — TypeScript erases it before the bundler sees it. Counting type imports turns 3 real findings into 29; a guard that cries wolf gets switched off. - Dynamic `import()` is still not followed. It does not break a bundle edge (that was tried for #10692 and failed) but it does move the module into a chunk the browser fetches on demand, which is a legitimate boundary. The widened sweep immediately found what the narrow one could not: five value-form imports of `db/batches` / `db/files` across three files under dashboard/batch, each reaching db/core → the SQLite driver. All five bind only interfaces (BatchRecord, FileRecord) used in type position, so the compiler was eliding them and the build stayed green — the same latent shape as #10692 before #10647 removed the toolchain's tolerance. Marking them `import type` makes the elision explicit instead of incidental. Refs #10692 Co-authored-by: Xiangzhe --- .../dashboard/batch/batch-utils.ts | 4 +- .../dashboard/batch/files/page.tsx | 4 +- src/app/(dashboard)/dashboard/batch/page.tsx | 4 +- ...client-bundle-no-server-only-10692.test.ts | 170 ++++++++++++------ 4 files changed, 124 insertions(+), 58 deletions(-) diff --git a/src/app/(dashboard)/dashboard/batch/batch-utils.ts b/src/app/(dashboard)/dashboard/batch/batch-utils.ts index 5f63d9f12a..48e3803475 100644 --- a/src/app/(dashboard)/dashboard/batch/batch-utils.ts +++ b/src/app/(dashboard)/dashboard/batch/batch-utils.ts @@ -1,5 +1,5 @@ -import { BatchRecord } from "@/lib/db/batches"; -import { FileRecord } from "@/lib/db/files"; +import type { BatchRecord } from "@/lib/db/batches"; +import type { FileRecord } from "@/lib/db/files"; export function mapBatchApiToRecord(b: any): BatchRecord { return { diff --git a/src/app/(dashboard)/dashboard/batch/files/page.tsx b/src/app/(dashboard)/dashboard/batch/files/page.tsx index 96608d63d8..4f0732cba5 100644 --- a/src/app/(dashboard)/dashboard/batch/files/page.tsx +++ b/src/app/(dashboard)/dashboard/batch/files/page.tsx @@ -6,8 +6,8 @@ import FilesListTab from "../FilesListTab"; import FilesConceptCard from "../components/FilesConceptCard"; import UploadFileModal from "../components/UploadFileModal"; import { mapFileApiToRecord, mapBatchApiToRecord } from "../batch-utils"; -import { FileRecord } from "@/lib/db/files"; -import { BatchRecord } from "@/lib/db/batches"; +import type { FileRecord } from "@/lib/db/files"; +import type { BatchRecord } from "@/lib/db/batches"; export default function BatchFilesPage() { const t = useTranslations("common"); diff --git a/src/app/(dashboard)/dashboard/batch/page.tsx b/src/app/(dashboard)/dashboard/batch/page.tsx index 0bf8b23a46..0100b8e948 100644 --- a/src/app/(dashboard)/dashboard/batch/page.tsx +++ b/src/app/(dashboard)/dashboard/batch/page.tsx @@ -3,8 +3,8 @@ import { useState, useEffect, useCallback, useRef } from "react"; import { useTranslations } from "next-intl"; import BatchListTab from "./BatchListTab"; -import { FileRecord } from "@/lib/db/files"; -import { BatchRecord } from "@/lib/db/batches"; +import type { FileRecord } from "@/lib/db/files"; +import type { BatchRecord } from "@/lib/db/batches"; import { mapBatchApiToRecord, mapFileApiToRecord } from "./batch-utils"; import BatchConceptCard from "./components/BatchConceptCard"; import NewBatchWizard from "./components/NewBatchWizard"; diff --git a/tests/unit/client-bundle-no-server-only-10692.test.ts b/tests/unit/client-bundle-no-server-only-10692.test.ts index 558ae7bda4..e29ae9d2fa 100644 --- a/tests/unit/client-bundle-no-server-only-10692.test.ts +++ b/tests/unit/client-bundle-no-server-only-10692.test.ts @@ -5,38 +5,52 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; /** - * #10692: `src/app/(dashboard)/dashboard/providers/page.tsx` is a `"use client"` page. - * Through `serviceKindIndex → mediaServiceKinds → imageRegistry → aihorde/imageModels → - * aihordeImageCatalog → safeOutboundFetch → proxyFetch → featureFlags → db/core` it reached - * the SQLite driver, so the production build tried to bundle `fs`/`net`/`tls` for the browser - * and failed with 28 `Module not found` errors (`Build App` red for 60 consecutive runs). + * #10692: a `"use client"` page reached the SQLite driver through + * `serviceKindIndex → mediaServiceKinds → imageRegistry → aihorde/imageModels → + * aihordeImageCatalog → safeOutboundFetch → proxyFetch → featureFlags → db/core`, so the + * production build tried to bundle `fs`/`net`/`tls` for the browser and failed with 28 + * `Module not found` errors (`Build App` red for 60 consecutive runs). * - * `serviceKindIndex.ts` already documents the invariant this guard enforces: - * "Client-safe: `mediaServiceKinds` only pulls in the pure-data media registries - * (no server-only deps)." + * `serviceKindIndex.ts` had stated the invariant in a comment — *"Client-safe: + * `mediaServiceKinds` only pulls in the pure-data media registries (no server-only deps)"* — + * and a comment cannot fail a build, so #10542 broke it unnoticed. * - * That was a comment, so nothing stopped #10542 from breaking it. This walks the real - * static-import graph instead — the same edges the bundler follows. Dynamic `import()` is - * deliberately NOT followed: deferring a server-only module behind one is exactly how the - * leak is fixed, and the bundler splits it into a chunk the browser never loads. + * This walks the real static-import graph, the same edges the bundler follows, from EVERY + * `"use client"` file in the repo rather than a hand-picked pair. + * + * Two deliberate exclusions, both load-bearing: + * + * - **`import type` is not an edge.** TypeScript erases it before the bundler sees it. A scan + * that counts type imports reports 26 phantom leaks against 2 real ones here — a guard that + * cries wolf gets switched off. + * - **Dynamic `import()` is not followed.** It does not actually break a bundle edge (that was + * tried for #10692 and failed), but it does move the module into a chunk the browser only + * fetches on demand, which is a legitimate boundary for a lazily-used server path. */ const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../.."); -/** Entry points that end up in a client bundle and must stay free of server-only code. */ -const CLIENT_SAFE_ENTRIES = [ +/** Modules that pull in Node builtins (fs/net/tls) and must never be statically reachable. */ +const SERVER_ONLY = new Set([ + "src/lib/db/core.ts", + "src/lib/db/adapters/driverFactory.ts", + "src/lib/db/adapters/sqljsAdapter.ts", + "src/lib/db/migrationRunner.ts", + "open-sse/utils/proxyFetch.ts", + "open-sse/utils/tlsClient.ts", +]); + +/** + * Non-`"use client"` entry points that still end up in a client bundle because client + * components import them. Kept explicit so the original #10692 chain stays pinned even if the + * page that exposed it is refactored. + */ +const EXTRA_ENTRIES = [ "src/lib/providers/serviceKindIndex.ts", "open-sse/config/mediaServiceKinds.ts", ]; -/** Modules that pull in Node builtins (fs/net/tls) and must never be statically reachable. */ -const SERVER_ONLY = [ - "src/lib/db/core.ts", - "src/lib/db/adapters/driverFactory.ts", - "src/lib/db/adapters/sqljsAdapter.ts", - "open-sse/utils/proxyFetch.ts", -]; - const EXTENSIONS = [".ts", ".tsx", ".mts", ".js"]; +const SKIP_DIRS = new Set(["node_modules", ".git", ".build", "dist", ".next", ".claude"]); /** Resolve an import specifier to a repo-relative file, or null when it leaves the repo. */ function resolveSpecifier(fromFile: string, specifier: string): string | null { @@ -68,53 +82,105 @@ function resolveSpecifier(fromFile: string, specifier: string): string | null { return null; } -/** Static import/export specifiers only — `import(...)` expressions are intentionally skipped. */ +/** True when the import clause contributes no runtime binding (pure `import type`). */ +function isTypeOnlyClause(clause: string): boolean { + if (/^\s*type\s/.test(clause)) return true; + const named = /\{([^}]*)\}/.exec(clause); + if (!named) return false; + // `import Default, { type A }` still emits an edge for the default binding. + const outsideBraces = clause.replace(/\{[^}]*\}/, "").trim(); + if (/[A-Za-z_$*]/.test(outsideBraces)) return false; + const bindings = named[1] + .split(",") + .map((binding) => binding.trim()) + .filter(Boolean); + return bindings.length > 0 && bindings.every((binding) => /^type\s/.test(binding)); +} + +/** Value-carrying static specifiers only. */ function staticSpecifiers(source: string): string[] { const withoutDynamic = source.replace(/\bimport\s*\(/g, "__dynamic_import__("); const out: string[] = []; - const patterns = [ - /(?:^|\n)\s*import\s+[^;'"]*from\s*["']([^"']+)["']/g, - /(?:^|\n)\s*import\s*["']([^"']+)["']/g, - /(?:^|\n)\s*export\s+[^;'"]*from\s*["']([^"']+)["']/g, - ]; - for (const pattern of patterns) { - for (const match of withoutDynamic.matchAll(pattern)) out.push(match[1]); + for (const pattern of [ + /(?:^|\n)\s*import\s+([^;'"]*)from\s*["']([^"']+)["']/g, + /(?:^|\n)\s*export\s+([^;'"]*)from\s*["']([^"']+)["']/g, + ]) { + for (const match of withoutDynamic.matchAll(pattern)) { + if (isTypeOnlyClause(match[1])) continue; + out.push(match[2]); + } + } + // Side-effect imports (`import "./x"`) always emit an edge. + for (const match of withoutDynamic.matchAll(/(?:^|\n)\s*import\s*["']([^"']+)["']/g)) { + out.push(match[1]); } return out; } +const specifierCache = new Map(); +function edgesOf(file: string): string[] { + const cached = specifierCache.get(file); + if (cached) return cached; + const absolute = path.join(REPO_ROOT, file); + let edges: string[] = []; + if (fs.existsSync(absolute)) { + edges = staticSpecifiers(fs.readFileSync(absolute, "utf8")) + .map((specifier) => resolveSpecifier(file, specifier)) + .filter((resolved): resolved is string => resolved !== null); + } + specifierCache.set(file, edges); + return edges; +} + /** BFS over static imports; returns the first path reaching a server-only module. */ function findServerOnlyPath(entry: string): string[] | null { const seen = new Set([entry]); const queue: Array = [[entry]]; while (queue.length > 0) { const trail = queue.shift()!; - const current = trail[trail.length - 1]; - const absolute = path.join(REPO_ROOT, current); - if (!fs.existsSync(absolute)) continue; - - for (const specifier of staticSpecifiers(fs.readFileSync(absolute, "utf8"))) { - const resolved = resolveSpecifier(current, specifier); - if (!resolved || seen.has(resolved)) continue; - const next = [...trail, resolved]; - if (SERVER_ONLY.includes(resolved)) return next; + for (const resolved of edgesOf(trail[trail.length - 1])) { + if (seen.has(resolved)) continue; + if (SERVER_ONLY.has(resolved)) return [...trail, resolved]; seen.add(resolved); - queue.push(next); + queue.push([...trail, resolved]); } } return null; } -for (const entry of CLIENT_SAFE_ENTRIES) { - test(`${entry} does not statically reach server-only code`, () => { - const trail = findServerOnlyPath(entry); - assert.equal( - trail, - null, - trail - ? `A client bundle would have to include a server-only module. Static import chain:\n ${trail.join("\n → ")}\n` + - `Break the chain (a dynamic import at the boundary is enough) rather than widening this guard.` - : "" - ); - }); +function walk(dir: string, acc: string[] = []): string[] { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (SKIP_DIRS.has(entry.name)) continue; + walk(full, acc); + } else if (/\.tsx?$/.test(entry.name)) { + acc.push(path.relative(REPO_ROOT, full)); + } + } + return acc; } + +function clientEntryPoints(): string[] { + return walk(path.join(REPO_ROOT, "src")).filter((file) => + /^\s*["']use client["']/m.test(fs.readFileSync(path.join(REPO_ROOT, file), "utf8").slice(0, 200)) + ); +} + +test("no client entry point statically reaches server-only code", () => { + const entries = [...clientEntryPoints(), ...EXTRA_ENTRIES]; + assert.ok(entries.length > 100, `expected the repo's client components, found ${entries.length}`); + + const offenders = entries + .map((entry) => ({ entry, trail: findServerOnlyPath(entry) })) + .filter((row): row is { entry: string; trail: string[] } => row.trail !== null); + + assert.deepEqual( + offenders.map((o) => o.entry), + [], + "A client bundle would have to include server-only modules:\n" + + offenders.map((o) => ` ${o.trail.join("\n → ")}`).join("\n\n") + + "\nBreak the chain — or, when the binding is only a type, mark it `import type` so it " + + "carries no runtime edge." + ); +});