Skip to content

dogfood: check false-positives on import type from a .server.ts file #805

Description

@vivek7405

Goal context

North-star: a minimalist or locally-run AI model (MiniMax, Kimi, a small local model) must produce correct webjs code on the FIRST iteration, not after N check-fail/rewrite cycles. webjs's information model is "grep the no-build source + agent-docs + MCP, not training data." A false-positive check actively defeats that model: it flags code the greppable source itself confirms is correct, so even an agent that did everything right is sent into a rewrite loop.

Problem

webjs check's no-server-import-in-browser-module fires on a TYPE-ONLY import of a .server.ts module from a browser-shipped page/component, for example:

import type { Todo } from '#db/schema.server.ts';

Three independent agents (Opus, MiniMax-M3, and a third) building the same todo app hit this. A type-only import is fully erased by the TS stripper (module.stripTypeScriptTypes), so it never reaches the browser and cannot crash it. The check is a false positive, and its "fix" text pushes agents to invent a browser-safe types.ts duplicate of the schema type, which is pure ceremony that the framework's own thesis (trust the greppable source) says is unnecessary.

Root cause (verified in source)

packages/server/src/module-graph.js:81:

const IMPORT_RE = /\bimport\s+(?:(?:[\w*{}\s,]+)\s+from\s+)?['"]([^'"]+)['"]/g;

The [\w*{}\s,]+ clause matches type { Todo }, so import type { ... } from '...' is recorded as a real graph edge (the comment at :89 even shows export type { T } from './bar' as a matched form). checkServerImportInBrowserModule (check.js:1190) then walks that edge via transitiveDeps and flags the module.

Design / approach

Drop FULLY type-only import/export statements from the module-graph edge set: they are guaranteed-erased and never fetched by the browser. Keep a MIXED statement (import { type A, b }) as an edge, because b is a runtime binding. This is safe across every consumer of the graph:

  • servability/auth gate (reachableFromEntries): a type-only import never needs its target servable.
  • preload hints (transitiveDeps): never preload an erased dep.
  • elision: a type-only import is not a client effect.
  • this check: the whole point.

Do it in the scanner so all consumers benefit uniformly.

Implementation notes (for the implementing agent)

  • Where: packages/server/src/module-graph.js, the scan loop around IMPORT_RE / EXPORT_FROM_RE (L80-102) and parseFile (~L442). Detect a statement-leading import type / export type (the type keyword immediately after import/export, not an inline specifier) and skip recording its specifier as an edge. Do NOT skip a mixed import { type A, b }.
  • Landmine: the scanner runs off a string/template REDACTION mask (redactStringsAndTemplates); match against the masked source, not raw. Keep the existing #-alias expansion (expandImportAlias) behavior so a type-only #db/... import is still dropped.
  • Landmine: this same graph backs the auth gate and elision. A differential elision test exists (test/elision/differential-elision.test.js); dropping type-only edges must not change any verdict for value imports.
  • Landmine: this is a lexical fix; note it under the IMPORTANT: prove the hand-rolled lexer matches an AST (auth gate / elision / check rest on it) #753 lexer-vs-AST umbrella.
  • Invariant: AGENTS.md invariant Corrected backtick errors #1 must still hold for VALUE imports (a real import { db } from a .server.ts must still fire).
  • Tests: add a rule test under packages/server/test/check/ asserting import type { X } from './x.server.ts' in a shipped page/component yields ZERO violations, plus a counterfactual that a VALUE import { X } from the same file still fires, plus a module-graph test that a type-only import is not an edge while a mixed one is.
  • Docs: correct any agent-docs text recommending the types.ts workaround (grep agent-docs for "no-server-import"); re-scope dogfood: collapse the agent iteration loop on first-time webjs scaffolds #804, which frames the workaround as the fix.

Acceptance criteria

  • import type { Todo } from '#db/schema.server.ts' in a browser-shipped page/component produces no no-server-import-in-browser-module violation.
  • A VALUE import of a no-'use server' .server.ts utility from the same module STILL fires (counterfactual).
  • A mixed import { type A, b } from a .server.ts still fires.
  • Differential elision + auth-gate tests unchanged (no verdict drift for value imports).
  • agent-docs no longer recommends the types.ts duplication workaround for this case.

Additional context (re-checked 2026-07-06, anchors re-verified against main)

Shipped together in ONE PR that Closes #805 #806 #807 #808 #809 (owner's request). Shared-file coordination:

Everything ships on one draft PR; each commit is one logical unit (one issue or one sub-part). The PR is not marked ready until all five are done and the self-review loop is clean.

Re-verified specifics (#805). Anchors on main: module-graph.js:81 IMPORT_RE, :102 EXPORT_FROM_RE, parse loop for (const re of [IMPORT_RE, EXPORT_FROM_RE]) around L444; consumer checkServerImportInBrowserModule at check.js:1103 (reachable-server-dep loop ~L1205). Reproduce: scaffold, add export type Todo to db/schema.server.ts, write a browser-shipped component with import type { Todo } from '#db/schema.server.ts', run webjs check -> false positive. Test command: node --test packages/server/test/check/*.test.js and the module-graph suite. This is the first commit of the combined PR (land it before #809's check work).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

  • Status
    Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions