diff --git a/docs/start/framework/react/guide/import-protection.md b/docs/start/framework/react/guide/import-protection.md index d6d36d4e121..9aabdbf82a0 100644 --- a/docs/start/framework/react/guide/import-protection.md +++ b/docs/start/framework/react/guide/import-protection.md @@ -335,13 +335,21 @@ The same idea applies to `createIsomorphicFn()`: the compiler removes the non-ta If you see an import-protection violation for a file you expected to be "compiled away", check whether the import is referenced outside a compiler-recognized environment boundary (or is otherwise kept live by surviving code). +## False Positives: Dev vs Build + +In **build mode**, the plugin defers violation checks until after tree-shaking. If an import is eliminated from the final bundle (e.g., a barrel re-exports a `.server` module but no client code actually uses that export), no violation is reported. This means build-time violations are definitive — if the build flags it, the import truly survived. + +In **dev mode**, there is no tree-shaking. The plugin uses graph reachability to filter violations, but it cannot determine whether individual bindings are unused. This means barrel re-exports of `.server` or marker-protected modules may produce warnings even when the server-only exports would be tree-shaken away in production. These dev warnings are informational — run a build to confirm whether the violation is real. + +The same applies to marker-protected files (`import '@tanstack/react-start/server-only'`). If a marked file is re-exported through a barrel but never consumed by client code, the build correctly suppresses the violation while dev may still warn. + ## The `onViolation` Callback You can hook into violations for custom reporting or to override the verdict: ```ts importProtection: { - onViolation: (info) => { + onViolation: async (info) => { // info.env -- environment name (e.g. 'client', 'ssr', ...) // info.envType -- 'client' or 'server' // info.type -- 'specifier', 'file', or 'marker' @@ -352,7 +360,7 @@ importProtection: { // info.snippet -- { lines, location } with the source code snippet (if available) // info.message -- the formatted diagnostic message - // Return false to allow this specific import (override the denial) + // Return false (or Promise) to allow this specific import (override the denial) if (info.specifier === 'some-special-case') { return false } @@ -392,7 +400,9 @@ interface ImportProtectionOptions { specifiers?: Array files?: Array } - onViolation?: (info: ViolationInfo) => boolean | void + onViolation?: ( + info: ViolationInfo, + ) => boolean | void | Promise } ``` diff --git a/e2e/react-start/import-protection/src/routes/__root.tsx b/e2e/react-start/import-protection/src/routes/__root.tsx index ebdf3be21f7..5d7fef8b067 100644 --- a/e2e/react-start/import-protection/src/routes/__root.tsx +++ b/e2e/react-start/import-protection/src/routes/__root.tsx @@ -34,6 +34,10 @@ function RootComponent() { Client-Only JSX {' | '} Beforeload Leak + {' | '} + Component Server Leak + {' | '} + Barrel False Positive diff --git a/e2e/react-start/import-protection/src/routes/barrel-false-positive.tsx b/e2e/react-start/import-protection/src/routes/barrel-false-positive.tsx new file mode 100644 index 00000000000..6f91eb50329 --- /dev/null +++ b/e2e/react-start/import-protection/src/routes/barrel-false-positive.tsx @@ -0,0 +1,43 @@ +import { createFileRoute } from '@tanstack/react-router' +import { createServerFn } from '@tanstack/react-start' +// Import from the barrel — NOT directly from .server. +// `getUsers` (from ./db.server) is only used inside a server fn (compiler strips it). +// `userColumns` (from ./shared) is a plain object used in JSX — not server-only. +// Tree-shaking should eliminate the ./db.server dependency entirely from the +// client bundle, so no import-protection violation should fire for it. +import { getUsers, userColumns, type User } from '../violations/barrel-reexport' + +const fetchUsers = createServerFn().handler(async () => { + return getUsers() +}) + +export const Route = createFileRoute('/barrel-false-positive')({ + loader: () => fetchUsers(), + component: BarrelFalsePositive, +}) + +function BarrelFalsePositive() { + const users = Route.useLoaderData() + + return ( +
+

Barrel False Positive

+ + + + + + + + + {users.map((user: User) => ( + + + + + ))} + +
{userColumns.name}{userColumns.email}
{user.name}{user.email}
+
+ ) +} diff --git a/e2e/react-start/import-protection/src/routes/component-server-leak.tsx b/e2e/react-start/import-protection/src/routes/component-server-leak.tsx new file mode 100644 index 00000000000..642dd7ce4d9 --- /dev/null +++ b/e2e/react-start/import-protection/src/routes/component-server-leak.tsx @@ -0,0 +1,19 @@ +import { createFileRoute } from '@tanstack/react-router' +// This import is used ONLY inside the route component. +// The router plugin code-splits the component into a lazy chunk, +// moving this import into the split module. +// Import protection must still catch this as a violation. +import { getComponentSecret } from '../violations/db-credentials.server' + +export const Route = createFileRoute('/component-server-leak')({ + component: ComponentServerLeak, +}) + +function ComponentServerLeak() { + return ( +
+

Component Server Leak

+

{getComponentSecret()}

+
+ ) +} diff --git a/e2e/react-start/import-protection/src/violations/barrel-reexport/db.server.ts b/e2e/react-start/import-protection/src/violations/barrel-reexport/db.server.ts new file mode 100644 index 00000000000..8b29f0337ee --- /dev/null +++ b/e2e/react-start/import-protection/src/violations/barrel-reexport/db.server.ts @@ -0,0 +1,26 @@ +/** + * Server-only database module. + * Only exports server-only values: getUsers (DB access) and User type. + * The side-effect (DATABASE_URL log) should NOT reach the client. + */ + +const DATABASE_URL = + process.env.DATABASE_URL ?? 'postgres://admin:s3cret@localhost:5432/myapp' + +console.log(`[db] connecting to ${DATABASE_URL}`) + +export interface User { + id: number + name: string + email: string +} + +const FAKE_USERS: Array = [ + { id: 1, name: 'Alice', email: 'alice@example.com' }, + { id: 2, name: 'Bob', email: 'bob@example.com' }, +] + +export async function getUsers(): Promise> { + await new Promise((r) => setTimeout(r, 50)) + return FAKE_USERS +} diff --git a/e2e/react-start/import-protection/src/violations/barrel-reexport/foo.ts b/e2e/react-start/import-protection/src/violations/barrel-reexport/foo.ts new file mode 100644 index 00000000000..a88e7e66919 --- /dev/null +++ b/e2e/react-start/import-protection/src/violations/barrel-reexport/foo.ts @@ -0,0 +1,11 @@ +/** + * Server-only module protected by the marker import pattern. + * Re-exported through the barrel (index.ts) but never imported by + * barrel-false-positive.tsx — tree-shaking eliminates it from the + * client bundle, so no import-protection violation should fire. + */ +import '@tanstack/react-start/server-only' + +export function foo(): string { + return 'server-only value from foo' +} diff --git a/e2e/react-start/import-protection/src/violations/barrel-reexport/index.ts b/e2e/react-start/import-protection/src/violations/barrel-reexport/index.ts new file mode 100644 index 00000000000..42a90279065 --- /dev/null +++ b/e2e/react-start/import-protection/src/violations/barrel-reexport/index.ts @@ -0,0 +1,17 @@ +/** + * Barrel file — NOT a .server file itself. + * Re-exports from a .server module, a marker-protected module, and a shared module. + * + * The key scenario: a consumer imports { getUsers, userColumns } from here. + * - getUsers comes from ./db.server (server-only via file suffix) + * - foo comes from ./foo (server-only via marker import) + * - userColumns comes from ./shared (client-safe) + * + * If getUsers is only used inside a createServerFn handler (compiler strips it), + * and foo is never imported by the consumer at all, tree-shaking should eliminate + * both ./db.server and ./foo from the client bundle — so the import-protection + * plugin should NOT fire violations for either. + */ +export { getUsers, type User } from './db.server' +export { foo } from './foo' +export { userColumns } from './shared' diff --git a/e2e/react-start/import-protection/src/violations/barrel-reexport/shared.ts b/e2e/react-start/import-protection/src/violations/barrel-reexport/shared.ts new file mode 100644 index 00000000000..6bfb63981b5 --- /dev/null +++ b/e2e/react-start/import-protection/src/violations/barrel-reexport/shared.ts @@ -0,0 +1,13 @@ +/** + * Shared (non-server) module with client-safe values. + * These are safe to use on the client. + */ +import type { User } from './db.server' + +export const userColumns = { + name: 'Full Name', + email: 'Email Address', +} as const + +/** Placeholder for when no users have loaded yet. */ +export const emptyUser: User = { id: 0, name: '', email: '' } diff --git a/e2e/react-start/import-protection/src/violations/db-credentials.server.ts b/e2e/react-start/import-protection/src/violations/db-credentials.server.ts new file mode 100644 index 00000000000..e02e13fd185 --- /dev/null +++ b/e2e/react-start/import-protection/src/violations/db-credentials.server.ts @@ -0,0 +1,11 @@ +/** + * Server-only secret used exclusively inside a route component. + * The route component is code-split by the router plugin into a separate + * lazy chunk, so the import to this file ends up in the split module — + * NOT the original route file. Import protection must still detect this. + */ +export const COMPONENT_SECRET = 'component-only-server-secret-99999' + +export function getComponentSecret() { + return COMPONENT_SECRET +} diff --git a/e2e/react-start/import-protection/tests/error-mode.setup.ts b/e2e/react-start/import-protection/tests/error-mode.setup.ts index 105ac304314..620360fc5a9 100644 --- a/e2e/react-start/import-protection/tests/error-mode.setup.ts +++ b/e2e/react-start/import-protection/tests/error-mode.setup.ts @@ -104,6 +104,7 @@ const routes = [ '/client-only-violations', '/client-only-jsx', '/beforeload-leak', + '/barrel-false-positive', ] async function captureDev(cwd: string): Promise { diff --git a/e2e/react-start/import-protection/tests/import-protection.spec.ts b/e2e/react-start/import-protection/tests/import-protection.spec.ts index 37d03593fd3..90cdccb474c 100644 --- a/e2e/react-start/import-protection/tests/import-protection.spec.ts +++ b/e2e/react-start/import-protection/tests/import-protection.spec.ts @@ -540,3 +540,93 @@ for (const mode of ['dev', 'dev.warm'] as const) { expect(lookupHits).toEqual([]) }) } + +// Component-level server leak: the .server import is used exclusively inside +// the route component function, which is code-split by the router plugin into +// a separate lazy chunk. Import protection must still detect this. + +test('component-server-leak route loads in mock mode', async ({ page }) => { + await page.goto('/component-server-leak') + await expect(page.getByTestId('component-leak-heading')).toContainText( + 'Component Server Leak', + ) +}) + +for (const mode of ['build', 'dev', 'dev.warm'] as const) { + test(`component-server-leak: .server import inside code-split component is caught in ${mode}`, async () => { + const violations = await readViolations(mode) + + const hits = violations.filter( + (v) => + v.envType === 'client' && + (v.specifier.includes('db-credentials.server') || + v.resolved?.includes('db-credentials.server') || + v.importer.includes('db-credentials.server') || + v.trace.some( + (s) => + s.file.includes('db-credentials.server') || + s.file.includes('component-server-leak'), + )), + ) + + expect(hits.length).toBeGreaterThanOrEqual(1) + }) +} + +// Barrel re-export false positive: the barrel re-exports from a .server +// module AND a marker-protected module (foo.ts with `import 'server-only'`), +// but the component only uses values that would be tree-shaken away +// (getUsers inside createServerFn) or originate from a safe source, and +// never imports foo at all. Both the .server module and the marker module +// should NOT survive tree-shaking in the client bundle, so import-protection +// must NOT flag violations for either in build mode. +// In dev mode there is no tree-shaking so the violation is expected (mock). + +test('barrel-false-positive route loads in mock mode', async ({ page }) => { + await page.goto('/barrel-false-positive') + await expect(page.getByTestId('barrel-heading')).toContainText( + 'Barrel False Positive', + ) +}) + +test('no false positive for barrel-reexport .server pattern in build', async () => { + const violations = await readViolations('build') + + const barrelHits = violations.filter( + (v) => + v.envType === 'client' && + (v.importer.includes('barrel-reexport') || + v.importer.includes('barrel-false-positive') || + v.specifier.includes('db.server') || + v.resolved?.includes('barrel-reexport') || + v.trace.some( + (s) => + s.file.includes('barrel-reexport') || + s.file.includes('barrel-false-positive'), + )), + ) + + expect(barrelHits).toEqual([]) +}) + +test('no false positive for barrel-reexport marker pattern in build', async () => { + const violations = await readViolations('build') + + // foo.ts uses `import '@tanstack/react-start/server-only'` marker and is + // re-exported through the barrel, but never imported by the route. + // Tree-shaking should eliminate it — no marker violation should fire. + const markerHits = violations.filter( + (v) => + v.envType === 'client' && + (v.specifier.includes('server-only') || + v.specifier.includes('foo') || + v.resolved?.includes('foo')) && + v.trace.some( + (s) => + s.file.includes('barrel-reexport') || + s.file.includes('barrel-false-positive'), + ), + ) + + expect(markerHits).toEqual([]) +}) diff --git a/e2e/react-start/import-protection/tests/violations.setup.ts b/e2e/react-start/import-protection/tests/violations.setup.ts index 07233d602c9..7ffe5fd1ef2 100644 --- a/e2e/react-start/import-protection/tests/violations.setup.ts +++ b/e2e/react-start/import-protection/tests/violations.setup.ts @@ -79,6 +79,8 @@ const routes = [ '/client-only-violations', '/client-only-jsx', '/beforeload-leak', + '/component-server-leak', + '/barrel-false-positive', ] async function navigateAllRoutes( diff --git a/packages/start-plugin-core/src/import-protection-plugin/INTERNALS.md b/packages/start-plugin-core/src/import-protection-plugin/INTERNALS.md new file mode 100644 index 00000000000..8c1a2b9a15f --- /dev/null +++ b/packages/start-plugin-core/src/import-protection-plugin/INTERNALS.md @@ -0,0 +1,290 @@ +# Import Protection Plugin — Internal Technical Documentation + +## Overview + +The import protection plugin prevents server-only code from leaking into client +bundles (and vice versa). It operates as a Vite plugin that intercepts module +resolution, detects violations, and either errors or replaces the offending +module with a safe mock. + +The plugin must handle **two axes of configuration**: + +| | **Dev** (`vite dev`) | **Build** (`vite build`) | +| --------- | -------------------------------------------------------------------- | -------------------------------------------------------------- | +| **Error** | `ctx.error()` immediately in `resolveId` | Defer to `generateBundle`; error if mock survived tree-shaking | +| **Mock** | Defer to transform-cache; warn if reachable via post-transform graph | Defer to `generateBundle`; warn if mock survived tree-shaking | + +## Plugin Architecture + +`importProtectionPlugin()` returns **three** Vite plugins: + +### 1. `tanstack-start-core:import-protection` (enforce: `'pre'`) + +The main enforcement plugin. Hooks: `applyToEnvironment`, `configResolved`, +`configureServer`, `buildStart`, `hotUpdate`, `resolveId`, `load`, and +`generateBundle`. All violation detection starts in `resolveId`. + +### 2. `tanstack-start-core:import-protection-transform-cache` + +Runs after all `enforce: 'pre'` hooks (including the Start compiler). Caches +transformed code and composed sourcemaps for accurate source-location mapping +in violation messages. Also resolves post-transform imports and triggers +`processPendingViolations()` for the dev mock deferral path. + +### 3. `tanstack-start-core:import-protection-mock-rewrite` (enforce: `'pre'`, dev only) + +Records expected named exports per importer so that dev mock-edge modules can +provide explicit ESM named exports. Only active in dev + mock mode. + +## Violation Types + +| Type | Trigger | Example | +| ----------- | --------------------------------------------------------------------------------------- | ------------------------------------------------------- | +| `file` | Resolved path matches a deny glob (e.g. `**/*.server.*`) | `import './db.server'` in client env | +| `specifier` | Import specifier matches a deny pattern | `import '@tanstack/react-start/server'` in client env | +| `marker` | File imports `'server-only'` or `'client-only'` marker, then is loaded in the wrong env | File with `import 'server-only'` resolved in client env | + +## The False-Positive Problem + +### Barrel re-exports + +A common pattern: + +```text +db/index.ts → export { getUsers } from './db.server' + export { userColumns } from './shared' + +route.tsx → import { getUsers, userColumns } from '../db' + // getUsers used only in createServerFn().handler() + // userColumns used in JSX +``` + +At `resolveId` time, `db/index.ts` imports `./db.server` — the plugin sees a +`.server` file imported in the client environment. But the Start compiler strips +the `getUsers` usage from the client bundle (it's inside a server fn handler), +and Rollup's tree-shaking then eliminates the `./db.server` dependency entirely. + +**No server code actually leaks.** But without deferral, the plugin fires at +`resolveId` time — before tree-shaking — producing a false positive. + +### Pre-transform resolves (server-fn-lookup) + +During dev, Vite's `fetchModule(?SERVER_FN_LOOKUP)` call triggers resolves for +analysing a module's exports. These are tracked via `serverFnLookupModules` and +`isPreTransformResolve`, and violations are silenced for them. + +## Violation Handling Flow + +### Central functions + +- **`handleViolation()`**: Formats + reports (or silences) the violation. Returns + a mock module ID (or `{ id, syntheticNamedExports }` in build) so `resolveId` + can substitute the offending import. May also return `undefined` (suppressed by + `onViolation` or silent+error in dev) or throw via `ctx.error()` (dev+error). +- **`reportOrDeferViolation()`**: Dispatch layer. Either defers (stores for later + verification) or reports immediately, depending on `shouldDefer`. + +### `shouldDefer` logic + +```ts +shouldDefer = (isDevMock && !isPreTransformResolve) || isBuild +``` + +- **Dev mock**: Defer to `pendingViolations` → verify via post-transform graph + reachability in `processPendingViolations()`. +- **Build (both mock and error)**: Defer to `deferredBuildViolations` → verify + via tree-shaking survival in `generateBundle`. + +All three violation types (file, specifier, AND marker) are deferred in build +mode. Marker violations through barrels can also be false positives — e.g., if +`foo.ts` has `import 'server-only'` and is re-exported through a barrel but +never used in client code, tree-shaking eliminates `foo.ts` entirely. To enable +`generateBundle` tracking for markers, `resolveId` returns the unique build +mock ID instead of the marker module. This is transparent because marker imports +are bare (`import 'server-only'` — no bindings), and the mock module is equally +side-effect-free. + +## Dev Mode Strategy + +### Dev + Error + +Violations fire immediately via `ctx.error()` in `resolveId`. No tree-shaking +is available, so false positives for barrel patterns are expected and accepted. +(Dev + error is typically used only during explicit validation.) + +### Dev + Mock + +1. `resolveId` calls `handleViolation({ silent: true })` — no warning emitted. +2. The mock module ID is returned so the dev server can serve a Proxy-based + mock instead of the real server module. +3. The violation is stored in `pendingViolations` keyed by the importer's file + path. +4. The transform-cache plugin, after resolving post-transform imports, calls + `processPendingViolations()`. +5. `processPendingViolations()` checks graph reachability from entry points + using only post-transform edges. If the violating importer is reachable + → confirm (warn). If unreachable → discard. If unknown → keep pending. + +This approach can't fully eliminate barrel false-positives in dev because +there's no tree-shaking. The barrel's import of `.server` always resolves, +and the barrel is reachable. This is a known and accepted limitation. + +### Dev mock modules + +In dev, each violation gets a **per-importer mock edge module** that: + +- Explicitly exports the names the importer expects (extracted by the + mock-rewrite plugin). +- Delegates to a **runtime mock module** that contains a recursive Proxy and + optional runtime diagnostics (console warnings when mocked values are used). + +This differs from build mode, where `syntheticNamedExports: true` lets Rollup +handle named export resolution from the silent mock. + +## Build Mode Strategy + +### The "mock first, verify later" pattern + +Both mock and error build modes follow the same pattern: + +1. **`resolveId`**: Call `handleViolation({ silent: true })`. Generate a + **unique per-violation mock module ID** (`\0tanstack-start-import-protection:mock:build:N`). + Store the violation + mock ID in `env.deferredBuildViolations`. Return the + mock ID so Rollup substitutes the offending import. + +2. **`load`**: Return a silent Proxy-based mock module (same code as + `RESOLVED_MOCK_MODULE_ID`) with `syntheticNamedExports: true`. + +3. **Tree-shaking**: Rollup processes the bundle normally. If no binding from + the mock module is actually used at runtime, the mock module is eliminated. + +4. **`generateBundle`**: Inspect the output chunks. For each deferred violation, + check whether its unique mock module ID appears in any chunk's `modules`. + - **Survived** → real violation (the import wasn't tree-shaken away). + - Error mode: `ctx.error()` — fail the build. + - Mock mode: `ctx.warn()` — emit a warning. + - **Eliminated** → false positive (tree-shaking removed it). Suppress + silently. + +### Why unique mock IDs per violation? + +The original `RESOLVED_MOCK_MODULE_ID` is a single shared virtual module used +for all mock-mode violations. If multiple violations are deferred, we need to +know _which specific ones_ survived tree-shaking. A shared ID would tell us +"something survived" but not which violation it corresponds to. The unique IDs +(`...mock:build:0`, `...mock:build:1`, etc.) provide this granularity. + +### Why mocking doesn't affect tree-shaking + +From the consumer's perspective, the import bindings are identical whether they +point to the real module or the mock. Rollup tree-shakes based on binding usage, +not module content. If a binding from the barrel's re-export of `.server` is +unused after the Start compiler strips server fn handlers, tree-shaking +eliminates it regardless of whether it points to real DB code or a Proxy mock. + +### Per-environment operation + +`generateBundle` runs once per Vite environment (client, SSR, etc.). Each +environment has its own `EnvState` with its own `deferredBuildViolations` array. +The check only inspects chunks from THAT environment's bundle, ensuring correct +per-environment verification. + +## Marker violations vs. file/specifier violations + +| | Marker | File / Specifier | +| ----------------------------- | --------------------------------------------------- | -------------------------------------------- | +| **What triggers it** | The _importer_ has a conflicting directive | The _import target_ matches a deny rule | +| **resolveId returns (dev)** | Marker module ID (`RESOLVED_MARKER_*`) \* | Mock edge module ID (per-importer) | +| **resolveId returns (build)** | Unique build mock ID (same as file/specifier) | Unique build mock ID | +| **Can be tree-shaken away?** | Yes — if the importer is eliminated by tree-shaking | Yes — if no binding from the target survives | +| **Deferred in build?** | Yes — deferred to `generateBundle` | Yes — deferred to `generateBundle` | +| **Deferred in dev mock?** | Yes — deferred to pending graph | Yes — deferred to pending graph | + +\* In dev mock, `handleViolation` internally returns a mock edge module ID +(stored as the deferred result in `pendingViolations`), but `resolveId` ignores +it for markers and falls through to return the marker module ID. The mock edge +ID is only used for deferral bookkeeping, not for module resolution. + +## State Management + +### `EnvState` (per Vite environment) + +Key fields for violation handling: + +- `seenViolations: Set` — deduplication of logged violations. +- `pendingViolations: Map>` — dev mock + deferral. Keyed by importer file path. +- `deferredBuildViolations: Array` — build mode + deferral. Each entry has `{ info, mockModuleId }`. + +Per-build/watch iteration, `buildStart` clears `pendingViolations` and resets +`deferredBuildViolations` so deferred entries don't leak across rebuilds. + +- `graph: ImportGraph` — import dependency graph for reachability checks. +- `serverFnLookupModules: Set` — modules transitively loaded during + server-fn analysis (false-positive suppression). + +### `SharedState` (cross-environment) + +- `fileMarkerKind: Map` — cached per-file marker + detection. A file's directive is inherent to the file, not the environment. + +## Virtual Module IDs + +| ID Pattern | Usage | +| -------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------- | +| `\0tanstack-start-import-protection:mock` | Shared silent mock (dev mock only) | +| `\0tanstack-start-import-protection:mock:build:N` | Per-violation build mock (unique counter) | +| `\0tanstack-start-import-protection:mock-edge:BASE64` | Per-importer dev mock with explicit named exports | +| `\0tanstack-start-import-protection:mock-runtime:BASE64` | Runtime diagnostic mock (dev client, console warnings) | +| `\0tanstack-start-import-protection:marker:*` | Marker module (empty `export {}`). Suffixed `server-only` or `client-only`; derived from `MARKER_PREFIX` in `plugin.ts` | + +## Key Design Decisions + +### Why not just skip `.server` resolves in barrels? + +The plugin doesn't know at `resolveId` time whether the barrel's re-export will +survive tree-shaking. It can't inspect the consumer's usage — it only sees the +barrel importing `.server`. Skipping it would miss real violations where the +barrel's `.server` re-export IS used in client code. + +### How marker violations are deferred in build + +Marker violations normally resolve to `RESOLVED_MARKER_*` (empty `export {}`), +not a mock. To enable `generateBundle` tracking, in build mode `resolveId` +returns the unique build mock ID instead of the marker module. This works +because: + +1. The marker import is bare (`import 'server-only'` — no bindings), so + swapping the resolution to a mock module is transparent. +2. The mock module is side-effect-free, just like the marker module. +3. If the importer file survives tree-shaking, the mock module survives → + `generateBundle` fires the violation. If the importer is tree-shaken away + (e.g., barrel re-export of a marker-protected file that's never used), the + mock is eliminated → violation suppressed. + +### Why accept false positives in dev? + +Dev mode uses Vite's ESM dev server — no bundling, no tree-shaking. The barrel +file's import of `.server` always resolves and is always reachable from entry +points. The graph-reachability check in `processPendingViolations` can only +eliminate violations where the _importer_ becomes unreachable after the Start +compiler transforms it, not where individual bindings are unused. + +This is an accepted trade-off: dev mock mode warns about potential issues, +build mode provides definitive answers via tree-shaking. + +## E2E Test Structure + +Tests live in `e2e/react-start/import-protection/`: + +- **Mock mode** (default): `globalSetup` builds the app, captures build warnings + to `violations.build.json`, starts a dev server capturing to + `violations.dev.json`. Tests read these JSON files and assert. +- **Error mode** (`BEHAVIOR=error`): `globalSetup` runs the build expecting + failure, captures exit code + output. Tests assert the build failed with the + expected violation. +- **False-positive test**: The `barrel-reexport` test case verifies that a barrel + re-exporting from both a `.server` file and a marker-protected file (`foo.ts` + with `import 'server-only'`), where all server-only bindings are tree-shaken + away, produces **zero** violations in the build log. diff --git a/packages/start-plugin-core/src/import-protection-plugin/plugin.ts b/packages/start-plugin-core/src/import-protection-plugin/plugin.ts index 73e90830675..d54e632c156 100644 --- a/packages/start-plugin-core/src/import-protection-plugin/plugin.ts +++ b/packages/start-plugin-core/src/import-protection-plugin/plugin.ts @@ -26,6 +26,7 @@ import { MOCK_MODULE_ID, MOCK_RUNTIME_PREFIX, RESOLVED_MARKER_PREFIX, + RESOLVED_MOCK_BUILD_PREFIX, RESOLVED_MOCK_EDGE_PREFIX, RESOLVED_MOCK_MODULE_ID, RESOLVED_MOCK_RUNTIME_PREFIX, @@ -122,7 +123,9 @@ interface PluginConfig { markerSpecifiers: { serverOnly: Set; clientOnly: Set } envTypeMap: Map - onViolation?: (info: ViolationInfo) => boolean | void + onViolation?: ( + info: ViolationInfo, + ) => boolean | void | Promise } /** @@ -192,6 +195,13 @@ interface EnvState { * determine whether the importer is still reachable from an entry point. */ pendingViolations: Map> + + /** + * Violations deferred in build mode (both mock and error). Each gets a + * unique mock module ID so we can check which ones survived tree-shaking + * in `generateBundle`. + */ + deferredBuildViolations: Array } interface PendingViolation { @@ -200,6 +210,12 @@ interface PendingViolation { mockReturnValue: string } +interface DeferredBuildViolation { + info: ViolationInfo + /** Unique mock module ID assigned to this violation. */ + mockModuleId: string +} + /** * Intentionally cross-env shared mutable state. * @@ -569,6 +585,7 @@ export function importProtectionPlugin( postTransformImports: new Map(), serverFnLookupModules: new Set(), pendingViolations: new Map(), + deferredBuildViolations: [], } envStates.set(envName, envState) } @@ -772,7 +789,7 @@ export function importProtectionPlugin( } if (config.onViolation) { - const result = config.onViolation(pv.info) + const result = await config.onViolation(pv.info) if (result === false) continue } warnFn(formatViolation(pv.info, config.root)) @@ -812,12 +829,20 @@ export function importProtectionPlugin( }) } - function handleViolation( + /** Counter for generating unique per-violation mock module IDs in build mode. */ + let buildViolationCounter = 0 + + type HandleViolationResult = + | { id: string; syntheticNamedExports: boolean } + | string + | undefined + + async function handleViolation( ctx: ViolationReporter, env: EnvState, info: ViolationInfo, violationOpts?: { silent?: boolean }, - ): { id: string; syntheticNamedExports: boolean } | string | undefined { + ): Promise { const key = dedupeKey( info.type, info.importer, @@ -827,24 +852,35 @@ export function importProtectionPlugin( if (!violationOpts?.silent) { if (config.onViolation) { - const result = config.onViolation(info) + const result = await config.onViolation(info) if (result === false) { return undefined } } - const seen = hasSeen(env, key) - if (config.effectiveBehavior === 'error') { - if (!seen) ctx.error(formatViolation(info, config.root)) - return undefined + // Dev+error: throw immediately. + // Always throw on error — do NOT deduplicate via hasSeen(). + // Rollup may resolve the same specifier multiple times (e.g. + // commonjs--resolver's nested this.resolve() fires before + // getResolveStaticDependencyPromises). If we record the key + // on the first (nested) throw, the second (real) resolve + // silently returns undefined and the build succeeds — which + // is the bug this fixes. + // + // Build mode never reaches here — all build violations are + // deferred via shouldDefer and handled silently. + + return ctx.error(formatViolation(info, config.root)) } + const seen = hasSeen(env, key) + if (!seen) { ctx.warn(formatViolation(info, config.root)) } } else { - if (config.effectiveBehavior === 'error') { + if (config.effectiveBehavior === 'error' && config.command !== 'build') { return undefined } } @@ -868,17 +904,23 @@ export function importProtectionPlugin( ) } - // Build: Rollup uses syntheticNamedExports - return { id: RESOLVED_MOCK_MODULE_ID, syntheticNamedExports: true } + // Build: unique per-violation mock IDs so generateBundle can check + // which violations survived tree-shaking (both mock and error mode). + const mockId = `${RESOLVED_MOCK_BUILD_PREFIX}${buildViolationCounter++}` + return { id: mockId, syntheticNamedExports: true } } /** * Unified violation dispatch: either defers or reports immediately. * * When `shouldDefer` is true, calls `handleViolation` silently to obtain - * the mock module ID, stores the violation as pending, and triggers - * `processPendingViolations`. Otherwise reports (or silences for - * pre-transform resolves) immediately. + * the mock module ID, then stores the violation for later verification: + * - Dev mock mode: defers to `pendingViolations` for graph-reachability + * checking via `processPendingViolations`. + * - Build mode (mock + error): defers to `deferredBuildViolations` for + * tree-shaking verification in `generateBundle`. + * + * Otherwise reports (or silences for pre-transform resolves) immediately. * * Returns the mock module ID / resolve result from `handleViolation`. */ @@ -889,14 +931,24 @@ export function importProtectionPlugin( info: ViolationInfo, shouldDefer: boolean, isPreTransformResolve: boolean, - ): Promise> { + ): Promise { if (shouldDefer) { - const result = handleViolation(ctx, env, info, { silent: true }) - deferViolation(env, importerFile, info, result) - await processPendingViolations(env, ctx.warn.bind(ctx)) + const result = await handleViolation(ctx, env, info, { silent: true }) + + if (config.command === 'build') { + // Build mode: store for generateBundle tree-shaking check. + // The unique mock ID is inside `result`. + const mockId = typeof result === 'string' ? result : (result?.id ?? '') + env.deferredBuildViolations.push({ info, mockModuleId: mockId }) + } else { + // Dev mock: store for graph-reachability check. + deferViolation(env, importerFile, info, result) + await processPendingViolations(env, ctx.warn.bind(ctx)) + } + return result } - return handleViolation(ctx, env, info, { + return await handleViolation(ctx, env, info, { silent: isPreTransformResolve, }) } @@ -1027,6 +1079,8 @@ export function importProtectionPlugin( envState.transformResultKeysByFile.clear() envState.postTransformImports.clear() envState.serverFnLookupModules.clear() + envState.pendingViolations.clear() + envState.deferredBuildViolations.length = 0 envState.graph.clear() envState.deniedSources.clear() envState.deniedEdges.clear() @@ -1156,10 +1210,13 @@ export function importProtectionPlugin( // Dev mock mode: defer violations until post-transform data is // available, then confirm/discard via graph reachability. + // Build mode (both mock and error): defer violations until + // generateBundle so tree-shaking can eliminate false positives. const isDevMock = config.command === 'serve' && config.effectiveBehavior === 'mock' + const isBuild = config.command === 'build' - const shouldDefer = isDevMock && !isPreTransformResolve + const shouldDefer = (isDevMock && !isPreTransformResolve) || isBuild // Check if this is a marker import const markerKind = config.markerSpecifiers.serverOnly.has(source) @@ -1198,7 +1255,7 @@ export function importProtectionPlugin( : `Module "${getRelativePath(normalizedImporter)}" is marked client-only but is imported in the server environment`, }, ) - await reportOrDeferViolation( + const markerResult = await reportOrDeferViolation( this, env, normalizedImporter, @@ -1206,6 +1263,16 @@ export function importProtectionPlugin( shouldDefer, isPreTransformResolve, ) + + // In build mode, if the violation was deferred, return the unique + // build mock ID instead of the marker module. This lets + // generateBundle check whether the importer (and thus its marker + // import) survived tree-shaking. The mock is side-effect-free just + // like the marker module, and the bare import has no bindings, so + // replacing it is transparent. + if (isBuild && markerResult != null) { + return markerResult + } } return markerKind === 'server' @@ -1338,6 +1405,7 @@ export function importProtectionPlugin( id: new RegExp( [ RESOLVED_MOCK_MODULE_ID, + RESOLVED_MOCK_BUILD_PREFIX, RESOLVED_MARKER_PREFIX, RESOLVED_MOCK_EDGE_PREFIX, RESOLVED_MOCK_RUNTIME_PREFIX, @@ -1360,6 +1428,11 @@ export function importProtectionPlugin( return loadSilentMockModule() } + // Per-violation build mock modules — same silent mock code + if (id.startsWith(RESOLVED_MOCK_BUILD_PREFIX)) { + return loadSilentMockModule() + } + if (id.startsWith(RESOLVED_MOCK_EDGE_PREFIX)) { return loadMockEdgeModule( id.slice(RESOLVED_MOCK_EDGE_PREFIX.length), @@ -1379,6 +1452,58 @@ export function importProtectionPlugin( return undefined }, }, + + async generateBundle(_options, bundle) { + const envName = this.environment.name + const env = envStates.get(envName) + if (!env || env.deferredBuildViolations.length === 0) return + + // Collect all module IDs that survived tree-shaking in this bundle. + const survivingModules = new Set() + for (const chunk of Object.values(bundle)) { + if (chunk.type === 'chunk') { + for (const moduleId of Object.keys(chunk.modules)) { + survivingModules.add(moduleId) + } + } + } + + // Check each deferred violation: if its unique mock module survived + // in the bundle, the import was NOT tree-shaken — real leak. + const realViolations: Array = [] + for (const { info, mockModuleId } of env.deferredBuildViolations) { + if (!survivingModules.has(mockModuleId)) continue + + if (config.onViolation) { + const result = await config.onViolation(info) + if (result === false) continue + } + + realViolations.push(info) + } + + if (realViolations.length === 0) return + + if (config.effectiveBehavior === 'error') { + // Error mode: fail the build on the first real violation. + this.error(formatViolation(realViolations[0]!, config.root)) + } else { + // Mock mode: warn for each surviving violation. + const seen = new Set() + for (const info of realViolations) { + const key = dedupeKey( + info.type, + info.importer, + info.specifier, + info.resolved, + ) + if (!seen.has(key)) { + seen.add(key) + this.warn(formatViolation(info, config.root)) + } + } + } + }, }, { // Captures transformed code + composed sourcemap for location mapping. diff --git a/packages/start-plugin-core/src/import-protection-plugin/virtualModules.ts b/packages/start-plugin-core/src/import-protection-plugin/virtualModules.ts index 81dbf941647..b8a82ecd86f 100644 --- a/packages/start-plugin-core/src/import-protection-plugin/virtualModules.ts +++ b/packages/start-plugin-core/src/import-protection-plugin/virtualModules.ts @@ -8,6 +8,14 @@ import type { ViolationInfo } from './trace' export const MOCK_MODULE_ID = 'tanstack-start-import-protection:mock' export const RESOLVED_MOCK_MODULE_ID = resolveViteId(MOCK_MODULE_ID) +/** + * Per-violation mock prefix used in build+error mode. + * Each deferred violation gets a unique ID so we can check which ones + * survived tree-shaking in `generateBundle`. + */ +export const MOCK_BUILD_PREFIX = 'tanstack-start-import-protection:mock:build:' +export const RESOLVED_MOCK_BUILD_PREFIX = resolveViteId(MOCK_BUILD_PREFIX) + export const MOCK_EDGE_PREFIX = 'tanstack-start-import-protection:mock-edge:' export const RESOLVED_MOCK_EDGE_PREFIX = resolveViteId(MOCK_EDGE_PREFIX) diff --git a/packages/start-plugin-core/src/schema.ts b/packages/start-plugin-core/src/schema.ts index 05a5f4e6968..cf04cf378ec 100644 --- a/packages/start-plugin-core/src/schema.ts +++ b/packages/start-plugin-core/src/schema.ts @@ -42,7 +42,13 @@ const importProtectionOptionsSchema = z onViolation: z .function() .args(z.any()) - .returns(z.union([z.boolean(), z.void()])) + .returns( + z.union([ + z.boolean(), + z.void(), + z.promise(z.union([z.boolean(), z.void()])), + ]), + ) .optional(), include: z.array(patternSchema).optional(), exclude: z.array(patternSchema).optional(),