feat: add asset() for content-hashed public urls - #1197
Open
vivek7405 wants to merge 11 commits into
Open
Conversation
An app's own asset urls sat at stable paths, so a CDN kept serving the
previous bytes after a deploy. That caused two visible regressions on
webjs.dev in one day, and the purge workflow that cleans up after it
only exists for this repo: an app built with the framework has no such
workflow, so the framework owed its users an answer.
import { html, asset } from '@webjsdev/core';
html`<link rel="stylesheet" href=${asset('/public/app.css')}>`
`asset()` follows the existing `cspNonce()` seam: the server installs a
resolver at boot, the browser has none and the function returns the path
unchanged. So it is safe to call from a layout, which must load in the
browser to register its component imports.
Why opt-in. The first attempt (#1196) rewrote asset urls by matching the
assembled HTML. Two deep-review rounds found six major defects, five of
them one bug: at that layer framework output and author data are
indistinguishable, so the matcher kept editing things it did not own. It
read and published a content hash for any file under the project root
(`<img src="/.env">` became `/.env?v=<hash>`, leaking existence and a
fingerprint of the bytes), rewrote hyphenated CUSTOM ELEMENTS' reactive
props, mangled `#fragment` urls, and desynced `rel=preload` font hints
from the `@font-face url()` that actually fetches them. Each fix
narrowed one case, but the space is every tag times every attribute
times every rel times every custom element times every data-driven
value, so zero regressions was not provable at any amount of review.
Marking the url where the author writes it makes the meaning
unambiguous: the blast radius is exactly the marked set, and nothing
else in the document is ever read or rewritten.
The `public/` gate is kept anyway, because an app may pass user-derived
data (`asset(user.avatarPath)`), and a fragment is split before hashing
so `/x.svg#icon` cannot lose its query or its fragment.
Adoption shows the point: the website marks tailwind.css, the two brand
lockups, and the highlight script, and deliberately does NOT mark the
font preloads (they must match the unversioned CSS `url()`) or the
favicons (SEO repo-health tests parse those hrefs literally). The
framework cannot get that choice wrong because it no longer makes it.
Verified end to end: a prod handler renders the marked url with `?v=`
and serves it `max-age=31536000, immutable` while a bare url gets 3600,
a dev handler leaves urls untouched, and the full suite is regression-
free against a clean-tree baseline under a real `npm ci`.
Closes #1194
AGENTS.md gains a core-API row, the skill's built-ins reference gains the usage and the opt-in rationale, and framework-dev.md's CDN section no longer claims these urls are unfingerprinted (it now records which website urls are marked, which are deliberately not, and why the purge workflow stays as the safety net for the rest). The preload caveat is documented everywhere the helper is, because it is the one way a reader can misuse this and make things slower: marking a `rel=preload` hint whose asset is fetched by CSS `url()` guarantees the preload never matches and the file is fetched twice. Refs #1194
Adding a public core export carries obligations this PR had not met, and my earlier "zero regressions" claim was measured against a contaminated baseline so it did not surface them. The baseline was wrong because `packages/core/dist/` is GITIGNORED. My work was already committed, so `git stash` had nothing to stash, the "clean" run still used MY built bundle, and four export-governance tests appeared to be failing on main when they were failing because of this change. Re-measured by checking out origin/main and rebuilding dist from clean source: main fails 2 (both blog fixtures), not 7. The four real regressions, now fixed: - `.d.ts declares every runtime named export (#388)`: added the sibling `src/asset-url.d.ts` and the overlay re-export, mirroring csp-nonce. - The browser-phantom test then caught a genuine hazard: the overlay declared `setAssetUrlProvider` while the browser bundle drops it, so a browser `import { setAssetUrlProvider }` would type-check and crash at load. Allowlisted as intentionally server-only, exactly like `setCspNonceProvider`, and the positive control proves it stays stripped. - Gallery coverage: `asset` is demoed on the caching card (the right home, since asset fingerprinting IS a caching feature, and that card's header comment previously told readers to reach for Cache-Control instead); `setAssetUrlProvider` is exempt as internal wiring. - API coverage followed once both were classified. The scaffold's generated layout now marks its own stylesheet, so a new app is correct by default rather than only after reading a doc, and its comment teaches the one way to misuse this (marking a `rel=preload` whose asset is really fetched by a CSS `url()`). One scaffold test asserted the literal `href="/public/tailwind.css"`. Its intent (the layout links the static compiled stylesheet, JS-off safe) is unchanged, so the assertion now matches the `asset()` form and additionally pins the import. Refs #1194
Deep review caught service-worker.md asserting that `public/` asset urls carry a `?v=` fingerprint in production "so the cache can never serve stale bytes". That was already shaky before this PR and is flatly wrong after it: a `public/` url is fingerprinted only when the author marks it with asset(), and an un-marked one is stable across deploys. The claim is most dangerous exactly where it appeared. A service worker serves these stale-while-revalidate, so the first load after a deploy renders the OLD bytes, and the worker's cache version does not necessarily rescue it: that version derives from the deploy build id, which dev.js documents as a deploy fingerprint independent of per-file content hashes, so a deploy changing only a `public/` file need not change it. A reader trusting the old sentence would skip both asset() and a purge and ship stale CSS. Running the doc-sync surface grep rather than fixing only the reported line found a SECOND copy of the same sentence in packages/cli/templates/public/sw.js, which ships into every scaffolded app. Both are corrected, and both now distinguish framework-emitted urls (fingerprinted automatically) from `public/` ones (marked or stale). The surface map also calls for a docs-site topic page on a new public export, which was missing, so website/app/docs/cache gains a "Static Assets: asset()" section next to the other caching layers, carrying the same preload caveat. Verified: no surface still asserts the old guarantee. Refs #1194
…clear
Deep review round 2 confirmed two more defects.
CONTRACT. I documented asset() as generally isomorphic, which is true
for a page or layout but wrong for a component that ships. Hydration is
a full client RE-RENDER (render-client.js drops the hydrate marker and
createInstance replaces the SSR'd children), and the browser bundle
installs no resolver, so `<img src=${asset('/public/logo.svg')}>` inside
a shipping component paints the hashed url at SSR and then swaps it for
the bare path on upgrade: the same bytes fetched twice, with the
short-lived url left in the DOM. Verified in the source (no provider in
index-browser.js, replaceChildren in createInstance).
It still produces a VALID url, so by AGENTS.md's own dividing line ("could
a sensible app legitimately want this to pass?") this is a convention,
not a `webjs check` rule, and the sibling cspNonce() carries the same
asymmetry with the same prose-only scoping. Corrected the JSDoc, the
AGENTS.md row, built-ins.md, and the docs-site section to say: use it in
a page, layout, or metadata route. Our own adoption already obeys this
(brandLockup is called from the layout and a layout-rendered fragment),
so nothing shipping regresses.
SCAFFOLD. create.js emits the marked stylesheet, but clear-gallery.mjs
writes its OWN minimal layout and hardcoded the un-marked url with no
asset import, so the documented reset path silently undid it. Every app
that ran gallery:clear before building would ship a stable stylesheet
url and serve the previous CSS after a deploy, which is the exact
incident class this feature exists to end. Running the scaffold-sync
surface grep rather than patching the reported line confirmed this was
the only such copy (the api template has no layout).
Verified per that skill's mandatory step: generated a full-stack app, ran
gallery:clear, and confirmed the post-clear layout keeps both the
asset() call and its import, with `webjs check` clean and `webjs
typecheck` exit 0 (which also proves the new export resolves from the
type overlay in a real generated app).
Added the post-clear assertion to scaffold-gallery.test.js, with a
verified counterfactual: reverting the clear-script fix fails it. Nothing
covered this before, which is why it slipped.
Refs #1194
Review round 3 found five defects. The first is a genuine correctness bug the earlier rounds missed. ELISION FINGERPRINT LEAKED INTO PUBLIC HASHES. assetHashFor folds the elision-verdict fingerprint into any file under appDir, and public/ is under appDir, so the guard admitted it despite the comment two lines above claiming public files "hash over their bytes alone". That was inert while only module specifiers were hashed; asset() routes public paths through it, which activated it. Reproduced: one unchanged file hashes to three different values across three verdicts. A deploy that merely flips an unrelated component between elidable and interactive would therefore change every marked asset url and re-download every byte-identical stylesheet, image, and script, the exact cost the hash exists to avoid. Excluded public/ explicitly; module hashing is unchanged, and a regression test pins the url to bytes alone. MODULE-SCOPE CALLS SHIP THE MODULE. A depth-0 call is a module-scope side effect, and component-elision's allowlist covers only register / define / extends WebComponent / new, so hoisting `const CSS = asset(...)` pins the whole page or layout into the browser bundle. My brand.ts comment had warned against hoisting for the WRONG reason (resolver ordering, which is not a hazard: the resolver installs at boot before any app module loads). Corrected there and stated in every doc surface. MEMOIZED HASH VS RUNTIME-MUTABLE FILES. The hash is memoized for the process lifetime and prod never rebuilds, so a public/ file rewritten in place keeps its old url while being served immutable for a year. Marking is for deploy-time artifacts; documented rather than changed, since dropping the memo would re-read and re-hash a 99KB stylesheet on every render. Also: asset() was missing from SKILL.md's export index (the surface an agent reads before the on-demand references, so it was undiscoverable after a gallery:clear), setAssetUrlProvider was missing from AGENTS.md's browser-drop list, and one element of the security test asserted a path the fixture never created, so it passed via the missing-file fail-safe rather than the gate. The file is now created, making the assertion load-bearing. Zero regressions against the true clean-main baseline. Refs #1194
Review round 4 found that the previous commit's fix stopped one directory short, and that the repo's own Bun gate had been bypassed. CORE HASHES TOO. The elision-verdict fold excludes `public/` but not the core install, and the comment on that very line claims "Core / public files are never transformed, so they hash over their bytes alone". That was false for core, and had been all along. In a REAL install `locateCoreDir` resolves to `<appDir>/node_modules/@webjsdev/core`, which IS under appDir, so the containment test admits it; it escapes only in this monorepo, where the workspace symlink lands outside appDir, which is exactly why no in-repo test ever caught it. Reproduced with a real-install layout: one byte-identical core bundle hashes to three values across three verdicts. Consequence in production: any deploy that merely flips an unrelated component between elidable and interactive moves the core bundle url, so every returning visitor re-downloads the whole runtime (and in src mode every core module) for nothing. Core is served verbatim by fileResponse, so the fold never bought anything there. Excluded it, made the comment state what the code enforces, and pinned it with a test that builds the real-install layout the monorepo hides. BUN PARITY. The diff stages packages/server/src/dev.js, which the require-bun-parity hook treats as runtime-sensitive, with no test/bun/** file, so the gate was bypassed rather than satisfied. asset() resolution is squarely runtime-sensitive: a sync readFileSync, a node:crypto digest, node:path containment comparisons, and decodeURIComponent, where a divergence would either hand two runtimes different urls for one file (thrashing immutable caches across a mixed fleet) or weaken the gate that keeps a private file unhashed. Added test/bun/asset-url.mjs plus its runner and a CI matrix step; verified passing under both node and bun. Zero regressions against the true clean-main baseline. Refs #1194
…rder Review round 5 found the implementation clean and two things outside it. DEPLOYMENT PAGE WAS MADE FALSE. /docs/deployment is the canonical "what headers do I get in prod" page and stated flatly that static files get max-age=3600, with vendor bundles as the one immutable case. That was universally true before this change, because nothing ever emitted a ?v= for a public asset. Verified against the booted prod site that it is now false: /public/tailwind.css?v=... returns max-age=31536000, immutable. Corrected both places and pointed at /docs/cache. Also widened framework-dev.md's list of marked urls, which omitted the two brand marks that brandMark() also marks. RELEASE ORDERING IS NOW LOad-BEARING AND WAS UNRECORDED. dev.js imports setAssetUrlProvider from core statically, and the generated app imports asset, but packages/server declares "@webjsdev/core": "^0.7.1", a range every published core satisfies and none of them carries the export. So a server published before core dies at module load, and a cli published first makes every scaffolded app 500. Checked the workflow rather than assuming: the publish loop sorts by the changelog `date:` ASC and breaks ties on filename DESC, which would publish `server` BEFORE `core`. The loop is `set -e` sequential, so core-first is also the fail-safe order. Recorded both the timestamp rule and the range bump in framework-dev.md's release section, where it will be read at the next release rather than living only in a PR body, since this recurs every time server or the scaffold consumes a new core export. Deliberately NOT making the import defensive (a namespace import plus an optional call). That would trade a loud, self-describing load error for a silent permanent loss of fingerprinting, and it only half-helps, since the scaffold would still break. It also deviates from the house style that context.js already sets. Zero regressions against the true clean-main baseline. Refs #1194
Review round 6 found one real defect plus three smaller gaps. BRAND PAGE SERVED THE SAME FILES TWICE. brandLockup() marks the two lockups for the header and footer, but /brand renders those exact files three more times with literal hrefs, so loading the page fetched each lockup under two cache keys: the marked url (immutable for a year) and the bare one (max-age=3600 at origin, 14400 at the edge). Worse, the page whose whole purpose is displaying the marks would keep showing a stale logo after a deploy that changed one, which is the #1185 incident this feature exists to end. framework-dev.md's claim that the lockups and marks are marked was therefore false for the surface that matters most. Marked all five img urls; the download anchors stay bare, since those are links to fetch a file, not references the page renders. While marking them, avoided a nested backtick inside the html template (invariant 9) by composing the grid url with concatenation rather than a nested template literal. Verified by rendering the page: all ten img srcs now carry a hash, and the lockup hashes match the header's, so it is one cache key rather than two. PER-PACKAGE INVENTORIES. packages/server/AGENTS.md's asset-hash row enumerates that module's exports and was missing resolveAssetUrl, which matters because it carries a STRICTER contract than its neighbours (the public/-only gate exists because callers may pass user-derived data). packages/core/AGENTS.md had a row for the sibling csp-nonce.js but none for asset-url.js. Both added. A TEST COMMENT WAS BACKWARDS. asset-helper-serve.test.js claimed "a dev server never installs the resolver"; dev.js installs it unconditionally and what makes dev a no-op is `_enabled` two modules away. Corrected, in the one test whose stated job is proving the wiring. BASEPATH. asset() strips a base path to resolve the file but never adds one, so under webjs.basePath the author must write the prefix. That is pre-existing (the framework only prefixes urls it emits) but the `bp` parameter made it look supported. Documented in the JSDoc and built-ins. Zero regressions against the true clean-main baseline. Refs #1194
Review round 7 found two gaps, both on surfaces the feature already claimed to cover. BRAND DOWNLOADS. The previous commit marked /brand's images but left the download anchors bare, reasoning they are links to fetch a file rather than references the page renders. That distinction does not exist in HTTP: an <a href> GET and an <img src> GET share one cache key. So after a deploy that changed a mark, the thumbnail showed the new drawing while the link beneath it served the previous one for up to the edge TTL, on the page whose entire purpose is distributing current marks. Marked them. The usual objection does not apply: both anchors carry a valueless `download`, whose saved filename derives from the url PATH, so a `?v=` query cannot change what lands on disk, and the visible link text is the filename string, independent of the href. BLOG EXAMPLE. examples/blog/app/layout.ts links its compiled stylesheet at a stable url, and example-blog.webjs.dev is one of the four hostnames in the same Cloudflare zone the purge workflow covers. It is the same shape as the stylesheet this PR marked on the website, so the rewritten framework-dev.md claim that these assets no longer depend on the purge was not true for it. Marked, and the doc now names both surfaces. Verified by rendering: /brand emits a hash on every image and both download links, with the lockup hashes byte-equal to the header's, and the blog layout emits /public/tailwind.css?v=48d66c579fc4. Full suite shows zero regressions against the clean-main baseline. It now reports FEWER failures than that baseline, because the two remaining entries were the blog-fixture tests, which pass once examples/blog has a seeded database, confirming they were environmental all along. Refs #1194
Every sibling in test/bun pairs mkdtempSync with an rmSync; this one did not, so both `npm test` and the Bun CI step stranded a webjs-asset-bun-* directory on every run. Verified: a full suite run now leaves none.
vivek7405
marked this pull request as ready for review
July 31, 2026 00:41
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces #1196 (closed unmerged after two deep-review rounds found six major defects). Rationale on #1194.
The API
Emits
/public/app.css?v=<hash>in production, servedmax-age=31536000, immutable. Un-marked, that url getsmax-age=3600and a CDN can serve stale bytes after a deploy until something purges it.Follows the existing
cspNonce()seam: the server installs a resolver at boot, the browser has none and returns the path unchanged.Why opt-in, not automatic
The predecessor rewrote asset urls by matching assembled HTML. Five of its six defects were one bug: at that layer framework output and author data are indistinguishable. It hashed any file under the project root (
<img src="/.env">leaked existence plus a content fingerprint), rewrote hyphenated custom elements' reactive props, mangled#fragmenturls, and desyncedrel=preloadfont hints. The space is every tag times attribute times rel times custom element times data-driven value, so zero regressions was not provable. Marking the url where the author writes it makes the blast radius exactly the marked set.Constraints (documented on every surface)
rel=preloadwhose asset is fetched by CSSurl()must stay un-marked.Release requirements (important)
@webjsdev/coremust publish before@webjsdev/serverand@webjsdev/cli.dev.jsimportssetAssetUrlProviderstatically and the generated app importsasset, whilepackages/serverdeclares"@webjsdev/core": "^0.7.1", a range every published core satisfies and none of them carries the export. In the release PR:changelog/core/<version>.mdthe earliestdate:of the batch. The publish loop sorts by that ASC and breaks ties on filename DESC, which would otherwise publishserverbeforecore. The loop isset -esequential, so core-first is also the fail-safe order.packages/server's core range to the version carrying the export, so npm enforces the coupling permanently.Both are now recorded in
framework-dev.md's release section.Test plan
/.env,/db/app.db,.server.ts,package.json), traversal (raw and encoded), base path, dev no-op, isomorphic fallback, throwing provider, and hash-stability against the elision verdict for BOTHpublic/and core?v=and serves itimmutablewhile a bare url gets 3600; an unmarked preload is untouched; dev leaves urls alonetest/bun/asset-url.mjs) plus a CI matrix step; verified passing under node and bunwebjs checkandwebjs typecheckclean, and the marking survivesgallery:clear(asserted, with counterfactual)npm ci: zero regressions against a clean-origin/mainbaseline (name-for-name diff)Review
Five rounds. Rounds 1 and 2 were deep multi-agent; 3 to 5 were single broad reviewers. Findings fixed: the security gate, fragments, frame and streaming coverage, export governance, hydration scoping, the
gallery:clearreset path, the elision fingerprint leaking intopublic/AND core hashes, the missing Bun parity, and doc drift acrossservice-worker.md,/docs/deployment, andSKILL.md.Closes #1194