test(guard): widen the client-bundle guard to every "use client" entry point (#10692) (#10700)

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 <bakryun0718@proton.me>
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-08-18 21:51:25 -03:00
committed by GitHub
parent 86fc1aade2
commit b754e44e26
4 changed files with 124 additions and 58 deletions

View File

@@ -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 {

View File

@@ -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");

View File

@@ -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";

View File

@@ -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<string, string[]>();
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<string>([entry]);
const queue: Array<string[]> = [[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."
);
});