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 57738b2952c..37d03593fd3 100644 --- a/e2e/react-start/import-protection/tests/import-protection.spec.ts +++ b/e2e/react-start/import-protection/tests/import-protection.spec.ts @@ -226,7 +226,7 @@ for (const mode of ['build', 'dev'] as const) { }) } -for (const mode of ['build', 'dev'] as const) { +for (const mode of ['build', 'dev', 'dev.warm'] as const) { test(`no false positive for boundary-safe pattern in ${mode}`, async () => { const violations = await readViolations(mode) @@ -340,7 +340,7 @@ test('build has violations in both client and SSR environments', async () => { expect(ssrViolations.length).toBeGreaterThanOrEqual(2) }) -for (const mode of ['build', 'dev'] as const) { +for (const mode of ['build', 'dev', 'dev.warm'] as const) { test(`no false positive for factory-safe middleware pattern in ${mode}`, async () => { const violations = await readViolations(mode) @@ -363,7 +363,7 @@ for (const mode of ['build', 'dev'] as const) { }) } -for (const mode of ['build', 'dev'] as const) { +for (const mode of ['build', 'dev', 'dev.warm'] as const) { test(`no false positive for cross-boundary-safe pattern in ${mode}`, async () => { const violations = await readViolations(mode) @@ -387,7 +387,7 @@ for (const mode of ['build', 'dev'] as const) { }) } -for (const mode of ['build', 'dev'] as const) { +for (const mode of ['build', 'dev', 'dev.warm'] as const) { test(`cross-boundary-leak: leaky consumer still produces violation in ${mode}`, async () => { const violations = await readViolations(mode) @@ -407,7 +407,7 @@ for (const mode of ['build', 'dev'] as const) { }) } -for (const mode of ['build', 'dev'] as const) { +for (const mode of ['build', 'dev', 'dev.warm'] as const) { test(`beforeload-leak: server import via beforeLoad triggers client violation in ${mode}`, async () => { const violations = await readViolations(mode) @@ -489,7 +489,7 @@ test('warm run traces include line numbers', async () => { if (step.specifier?.includes('?tsr-split=')) continue if (step.file.includes('routeTree.gen')) continue const prev = v!.trace[i - 1] - if (prev?.specifier?.includes('?tsr-split=')) continue + if (prev.specifier?.includes('?tsr-split=')) continue expect( step.line, @@ -498,3 +498,45 @@ test('warm run traces include line numbers', async () => { expect(step.line).toBeGreaterThan(0) } }) + +// Regression: SERVER_FN_LOOKUP variant pollution + hasSeenEntry bug. +// +// The Start compiler excludes ?server-fn-module-lookup variants from its +// transform, so they retain the original (untransformed) imports. If the +// reachability check considers those untransformed imports, it would see +// edges the compiler has actually pruned, causing a false positive. +// +// Additionally, in Vite dev mode the client entry resolves through virtual +// modules, so a naïve resolveId-based entry detection may never fire, +// causing all deferred violations to be confirmed immediately. +// +// The cross-boundary-safe pattern exercises both bugs: +// auth-wrapper.ts exports createAuthServerFn (factory with middleware) +// → session-util.ts wraps @tanstack/react-start/server +// → usage.ts calls createAuthServerFn().handler() +// All server imports are inside compiler boundaries and must be pruned. + +for (const mode of ['dev', 'dev.warm'] as const) { + test(`regression: server-fn-lookup variant does not cause false positive in ${mode}`, async () => { + const violations = await readViolations(mode) + + // Any violation touching the cross-boundary-safe chain where the importer + // or trace includes the ?server-fn-module-lookup query (or its normalized + // key) would indicate the lookup variant polluted the reachability check. + const lookupHits = violations.filter( + (v) => + v.envType === 'client' && + (v.importer.includes('auth-wrapper') || + v.importer.includes('session-util') || + v.importer.includes('cross-boundary-safe') || + v.trace.some( + (s) => + s.file.includes('auth-wrapper') || + s.file.includes('session-util') || + s.file.includes('cross-boundary-safe'), + )), + ) + + expect(lookupHits).toEqual([]) + }) +} 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 65812207f97..73e90830675 100644 --- a/packages/start-plugin-core/src/import-protection-plugin/plugin.ts +++ b/packages/start-plugin-core/src/import-protection-plugin/plugin.ts @@ -185,18 +185,6 @@ interface EnvState { */ postTransformImports: Map> - /** - * Whether a `resolveId` call without an importer has been observed for this - * environment since `buildStart`. Vite calls `resolveId(source, undefined)` - * for true entry modules during a cold start. On warm start (`.vite` cache - * exists), Vite reuses its module graph and does NOT call `resolveId` for - * entries, so this stays `false`. - * - * When `false`, the import graph is considered unreliable (edges may be - * missing) and violations are reported immediately instead of deferred. - */ - hasSeenEntry: boolean - /** * Violations deferred in dev mock mode. Keyed by the violating importer's * normalized file path. Violations are confirmed or discarded by the @@ -579,7 +567,6 @@ export function importProtectionPlugin( }, }, postTransformImports: new Map(), - hasSeenEntry: false, serverFnLookupModules: new Set(), pendingViolations: new Map(), } @@ -682,12 +669,16 @@ export function importProtectionPlugin( // Check all code-split variants for this parent. The edge is // live if ANY variant's resolved imports include `current`. + // Skip SERVER_FN_LOOKUP variants — they contain untransformed + // code (the compiler excludes them), so their import lists + // include imports that the compiler would normally strip. const keySet = env.transformResultKeysByFile.get(parent) let anyVariantCached = false let edgeLive = false if (keySet) { for (const k of keySet) { + if (k.includes(SERVER_FN_LOOKUP_QUERY)) continue const resolvedImports = env.postTransformImports.get(k) if (resolvedImports) { anyVariantCached = true @@ -752,10 +743,12 @@ export function importProtectionPlugin( const toDelete: Array = [] for (const [file, violations] of env.pendingViolations) { - // On warm start, skip graph reachability — confirm immediately. - const status = env.hasSeenEntry - ? checkPostTransformReachability(env, file) - : 'reachable' + // Wait for entries before running reachability. registerEntries() + // populates entries at buildStart; resolveId(!importer) may add more. + const status = + env.graph.entries.size > 0 + ? checkPostTransformReachability(env, file) + : 'unknown' if (status === 'reachable') { for (const pv of violations) { @@ -1033,7 +1026,6 @@ export function importProtectionPlugin( envState.transformResultCache.clear() envState.transformResultKeysByFile.clear() envState.postTransformImports.clear() - envState.hasSeenEntry = false envState.serverFnLookupModules.clear() envState.graph.clear() envState.deniedSources.clear() @@ -1118,7 +1110,6 @@ export function importProtectionPlugin( source, importer: importerPath, isEntryResolve, - hasSeenEntry: env.hasSeenEntry, command: config.command, behavior: config.effectiveBehavior, }) @@ -1141,7 +1132,9 @@ export function importProtectionPlugin( if (!importer) { env.graph.addEntry(source) - env.hasSeenEntry = true + // Flush pending violations now that an additional entry is known + // and reachability analysis may have new roots. + await processPendingViolations(env, this.warn.bind(this)) return undefined }