Skip to content

feat(server)!: version-in-URL for vendor + scanner tightening + lint rule + optional warm#88

Open
vivek7405 wants to merge 6 commits into
mainfrom
feat/option-4-vendor
Open

feat(server)!: version-in-URL for vendor + scanner tightening + lint rule + optional warm#88
vivek7405 wants to merge 6 commits into
mainfrom
feat/option-4-vendor

Conversation

@vivek7405
Copy link
Copy Markdown
Collaborator

Summary

The minimal architectural change to webjs's no-build vendor pipeline, scoped after the design exploration in closed PR #87 surfaced that the larger esm.sh + disk-cache rewrite was over-engineering for the problems actually present on main.

Three real changes plus housekeeping:

  1. Version-in-URL for vendor bundles fixes the stale browser cache bug where /__webjs/vendor/<pkg>.js URLs stayed stable across version bumps. Combined with the production Cache-Control: immutable headers this meant browsers held the old bundle forever after npm install pkg@new. Now URLs are /__webjs/vendor/<pkg>@<version>.js so version bumps invalidate browser caches automatically.

  2. Scanner tightening removes four false-positive sources from the bare-import scan: route.{ts,js,mjs,mts} and middleware.{ts,js,mjs,mts} (server-only by file-router convention), test/ and tests/ directories, import type statements (TypeScript erases them), and import strings inside JSDoc / line comments. Less noise in the vendor pipeline, fewer spurious bundle attempts on server-only packages.

  3. New no-non-erasable-typescript lint rule scans .ts source for the four constructs that trip the native type-stripper (enum, value-carrying namespace, constructor parameter properties, import = require). Companion to the existing erasable-typescript-only tsconfig-flag check; catches violations even if the flag is missing or disabled.

  4. Optional webjs vendor warm command pre-bundles every bare-import dep into the in-memory vendor cache before the server accepts connections. Add to prestart to eliminate first-user latency on production cold starts. Off by default.

What this PR is NOT

Lots of the architectural discussion in closed PR #87 explored options that this PR explicitly does NOT take on:

  • No esm.sh or other remote bundler. Keeps esbuild local; no runtime third-party dep.
  • No disk cache. In-memory only, like main. Matches Rails 7 + importmap-rails posture where vendoring via bin/importmap pin --download is opt-in, not default.
  • No .webjs/vendor/ directory. Nothing committed to source control.
  • No vendor pin/unpin/list CLI. Only warm is added, and it's an in-memory pre-bake, not a persistent cache management tool.
  • No predev/prestart auto-pin in scaffolds. Users opt in to warm per-project when they want it.
  • No framework-package bundling change. @webjsdev/core and friends continue to be served per-file from node_modules (see "Why" below).

Why framework-package bundling was deferred

Initially planned. On implementation review the subpath problem made it complex: importmap.js hardcodes 8 entries for @webjsdev/core/{directives,context,task,testing,lazy-loader,...} subpaths. Bundling preserves the package-name entry but breaks subpath imports unless we either (a) serve trailing-slash entries (defeats bundling) or (b) emit a one-bundle with all subpath exports flattened (breaks import { repeat } from '@webjsdev/core/directives'). The current per-file approach works correctly for BOTH workspace dev AND npm-installed scenarios; marginal HTTP-request-count savings on HTTP/2 multiplex didn't justify the complexity. Documented for future revisit if a real perf issue surfaces.

Concrete impact

Bug on main Status
Stale browser cache after version bump Fixed by version-in-URL
Spurious vendor pipeline attempts on server-only imports in route.ts Fixed by scanner tightening
Spurious vendor attempts on ws from type-only imports Fixed by (?!type\s) lookahead in IMPORT_RE
Spurious vendor attempts on clsx / tailwind-merge mentioned in JSDoc comments Fixed by stripComments() pre-pass
First-user latency on production cold start (was implicit cost) Mitigable via opt-in webjs vendor warm in prestart
Non-erasable TypeScript caught only via tsconfig flag (skip-able) Caught by source-level scan in new lint rule

Files changed (8 files, +656 / -75)

  • packages/server/src/vendor.js: scanner tightening, resolvePackageDir + getPackageVersion helpers, parseVendorId, version-keyed cache in bundlePackage, version-in-URL in vendorImportMapEntries, URL-id parsing in serveVendorBundle
  • packages/server/src/dev.js: vendor URL handler passes the full id, two call sites of vendorImportMapEntries updated to pass appDir
  • packages/server/src/check.js: new no-non-erasable-typescript rule + scanner with 4 patterns
  • packages/server/index.js: re-export getPackageVersion and parseVendorId for CLI consumption
  • packages/cli/bin/webjs.js: new vendor warm subcommand, USAGE updated
  • packages/server/test/vendor/vendor.test.js: 4 new scanner tests + 4 new parseVendorId tests + updated tests for new bundlePackage/serveVendorBundle/vendorImportMapEntries signatures + 1 cache-independence test
  • packages/server/test/check/check.test.js: 6 new tests for no-non-erasable-typescript rule
  • packages/server/test/dev/dev-handler.test.js: vendor URL test uses versioned form

Test plan

  • npm test: 1168/1168 pass
  • Verified webjs vendor warm runs end-to-end in examples/blog
  • Verified version-in-URL: bundlePackage's version-keyed cache populates independently for different version strings
  • Scanner tightening verified by 4 new positive tests + 1 negative
  • Manual production deploy validation (Docker build + Railway redeploy) pending

Design history

The full design exploration that led to this scope lives in closed PR #87 (feat/no-build-vendor-drops-esbuild). That PR explored esm.sh + committed disk cache + auto-prune + pin/unpin/list CLI + predev/prestart automation; was closed 2026-05-23 as over-engineering. The scanner improvements and lint rule were salvaged from that branch (re-applied here directly on top of main since the closed branch had drifted too far for a clean cherry-pick).

Why now

The version-in-URL fix is the real production bug. The other changes amortize accumulated tech debt around the vendor pipeline. Together they're ~80-100 substantive lines added vs main, no new user-facing conventions, no new directories, no new mental models. Forward-compatible: if real production users later need disk cache or other features, they're additive to this architecture, not replacements.

Follow-ups

  • Write blog post on the no-build journey (tracked as a pending task in agent memory)
  • Audit further dep-graph reductions now that the scanner is cleaner (chokidar, ws as candidates for Node 24+ native replacement; tracked as a pending task)

vivek7405 added 6 commits May 24, 2026 00:09
…s and false positives

The vendor scanner picked up server-only imports from contexts that
never reach the browser, generating spurious vendor pipeline work
on packages that can't be browser-bundled.

Four tightenings:

  1. route.{ts,js,mjs,mts} and middleware.{ts,js,mjs,mts}: server-
     only by file-router convention. New isServerOnlyFile() helper
     joins these to the existing .server.{ts,js,mjs,mts} suffix
     check. Imports of @prisma/client, ws, etc. in these files no
     longer enter the vendor pipeline.

  2. test/ and tests/ directories: tests are server-only by webjs
     convention. Their imports of test frameworks and DB clients
     shouldn't generate browser vendor entries.

  3. `import type X from 'pkg'` statements: TypeScript type-only
     imports are erased at compile time, never reach the runtime.
     The IMPORT_RE has a (?!type\s) negative lookahead. Catches
     real-world false positive in api/chat/route.ts which imports
     'ws' as type-only.

  4. Imports inside /* … */ block comments and // line comments:
     JSDoc examples (cn.ts in @webjsdev/ui, etc.) frequently show
     'import x from clsx' in a code block. The scanner now strips
     comments before pattern-matching.

Four new tests:

  scanBareImports: skips route.ts and middleware.ts
  scanBareImports: skips test/ and tests/ directories
  scanBareImports: skips import type statements
  scanBareImports: skips import strings inside comments

These bug fixes were salvaged from the closed PR #87
(feat/no-build-vendor-drops-esbuild). Re-applied directly on top
of main's esbuild-on-demand vendor.js instead of cherry-picking,
because the closed branch had drifted too far for a clean apply.

1155 tests pass.
Source-level companion to the existing erasable-typescript-only
tsconfig-flag rule. Scans every .ts / .mts file under the app for
the four constructs the framework's type-stripper rejects at
request time:

  - enum declarations (any of enum / const enum / declare enum,
    with uppercase first letter to avoid matching variables
    literally named 'enum')
  - namespace blocks containing value statements (let/const/var/
    function/class). Type-only namespaces, which ARE erasable,
    are intentionally allowed
  - constructor parameter properties (public/private/protected/
    readonly modifier directly before a parameter name)
  - import = require (TypeScript-style CommonJS import)

Each violation reports file, line number, the construct name, and
a concrete fix. Skips node_modules, dist, build, .next, .git, and
any folder starting with underscore (the framework's _private
convention).

Why both rules ship enabled by default:

  erasable-typescript-only catches the tsconfig case (flag missing
  or off). It's the early-warning path; if the flag is set, the
  TypeScript compiler flags violations in your editor before they
  reach the runtime.

  no-non-erasable-typescript catches the source case (offending
  syntax that slipped past tsconfig, or files written before the
  flag was added, or third-party packages that publish raw .ts).
  It's the late-warning path; runs at commit time via
  webjs check.

Together, an app gets two independent defenses for the same class
of violations. Either alone is incomplete: the tsconfig flag does
nothing if the user disables it; a source scan alone doesn't help
during editing.

Six new tests: one positive case per construct (asserts the rule
flags it), one negative case (clean erasable .ts file passes),
and one scope test (node_modules and _private folders are
correctly skipped).

All 1164 tests pass.
…ache

Vendor URLs were /__webjs/vendor/<pkg>.js with no version segment.
Combined with the production cache header Cache-Control: public,
max-age=31536000, immutable, this meant browsers held the cached
bundle forever after a version bump because the URL never changed.
A user who ran npm install dayjs@2.0.0 would still get dayjs@1.11.13
in the browser until they cleared their cache manually.

Fix: include the installed version in the URL.

  Before: /__webjs/vendor/dayjs.js
  After:  /__webjs/vendor/dayjs@1.11.13.js

After a version bump, vendorImportMapEntries regenerates with the
new version, the importmap emits a different URL, the browser sees
it as a cache miss and fetches fresh. immutable cache headers stay
in place; they're correct under content-addressed URLs.

Implementation details:

  - new resolvePackageDir(pkgName, appDir): walks createRequire's
    resolved path back to the package root, handling npm-workspace
    hoisting (a leaf app's node_modules is often empty; deps live
    at the workspace root)
  - new getPackageVersion(pkgName, appDir): reads
    node_modules/<pkg>/package.json's version field. Returns null
    for unresolvable packages
  - vendorImportMapEntries(bareImports, appDir): now takes appDir,
    reads version per package, skips packages whose version can't
    be resolved (browser surfaces 'unresolved bare specifier' which
    is the right signal)
  - new parseVendorId(id): parses URL-segment 'dayjs@1.11.13.js'
    back to { pkgName, version }. Scoped packages use '--' as the
    '/' separator (URL and filesystem safe). Inverse of the URL
    generation in vendorImportMapEntries
  - bundlePackage(pkgName, version, appDir, dev): now takes version,
    cache is keyed by '<pkg>@<version>' so different versions of
    the same package cache independently and old versions naturally
    evict via LRU
  - serveVendorBundle(id, appDir, dev): now takes the URL id
    instead of pkgName, parses it, dispatches to bundlePackage.
    Malformed ids (no @Version segment) return a clear 404
  - dev.js's vendor URL handler updated to pass the full id
  - all callers of vendorImportMapEntries pass appDir

Test updates:

  - vendorImportMapEntries tests assert the new versioned URL shape
  - parseVendorId tests cover plain, scoped, malformed inputs
  - bundlePackage + serveVendorBundle tests use the new signatures
  - dev-handler test for vendor URL uses versioned form
  - new test: different version segments cache independently
  - 1161 to 1168 tests pass

Doc comment on vendorImportMapEntries references Rails 7 +
importmap-rails as the precedent for URL-with-version addressing.
The double-dash separator for scoped packages mirrors Rails'
filesystem convention.
…n cold-start mitigation

Without warming, the first request after a server cold-start pays
esbuild's per-package bundling cost (50-300ms per package). On a
typical app with 5-8 client-side npm deps that's 300-800ms added
to the first user's first request. For serverless deployments with
frequent cold starts the cost compounds; for long-running containers
it's a one-time hit per deploy.

`webjs vendor warm` scans the app for bare-import packages and
calls bundlePackage on each, populating the in-memory vendor cache
before the server accepts connections. After warming completes the
first user request hits cache and returns in ~1ms.

  Server-only packages that the scanner mis-picks (rare after the
  earlier scanner tightening) fail bundling silently and report
  'FAILED (probably server-only deps; safe to ignore).' Doesn't
  block the warm; doesn't block the boot when chained to prestart.

Designed to be added to scaffold's prestart hook by users who want
the perf characteristic, off by default. Usage:

  // package.json
  "prestart": "prisma migrate deploy && webjs vendor warm"

Or used standalone for ad-hoc warming:

  webjs vendor warm

No disk artifacts (unlike Rails 7 + importmap-rails's
`bin/importmap pin --download` which writes to vendor/javascript/).
Matches webjs's no-build philosophy of keeping bundling ephemeral
and in-memory by default; persistent disk cache stays a forward-
compatible future addition gated on real user demand.

Also re-exports getPackageVersion and parseVendorId from
@webjsdev/server's main entry so the CLI can use them without
reaching into ./src/vendor.js (a non-exported subpath).

Verified end-to-end against examples/blog: warm runs, reports 0
packages (workspace-internal only), exits cleanly. 1168 tests
still pass.
The CLI's `webjs vendor warm` command needs these helpers but
importing @webjsdev/server/src/vendor.js (the subpath) breaks at
runtime because the server package's exports field doesn't expose
internal src/* paths. Re-export from the main entry so the CLI
can `import { ... } from '@webjsdev/server'` cleanly.
…ehavior, not the closed PR's

The lint rule's description and violation message described
'rejected at request time with a 500' from the closed-PR esm.sh
exploration where the esbuild fallback was removed. On main +
Option 4 the esbuild fallback stays. Updated message to accurately
describe the real cost: 3x wire bytes per request and lost
position preservation, not a hard rejection.

Cherry-picked the rule from the closed branch but didn't notice
the description was tailored for that branch's removed-fallback
behavior. Same rule, same patterns, same enforcement logic;
correct message now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant