diff --git a/.agents/skills/webjs/references/built-ins.md b/.agents/skills/webjs/references/built-ins.md index b3824055a..6e1a8a9e5 100644 --- a/.agents/skills/webjs/references/built-ins.md +++ b/.agents/skills/webjs/references/built-ins.md @@ -84,6 +84,8 @@ export const revalidate = 60; // cache this page's HTML for 60s Both are automatic, prod-focused, and need no config. In production every served module and `public/` asset gets a per-file `?v=` and `Cache-Control: public, max-age=31536000, immutable`, so a returning client fetches a changed file only when its bytes change. Every cacheable response also carries a weak `ETag`, and a repeat request with a matching `If-None-Match` gets a `304 Not Modified` with no body. Private (`no-store` / `private`) and streamed responses are excluded from the ETag path (no cross-session 304). Dev is byte-faithful (no hashing). +**A page's ETag is only useful if the page renders the same bytes twice.** The ETag is a hash of the response body, so any per-render-varying value anywhere in the document defeats it: a `Date.now()`, a `Math.random()`, an id from a module-scope counter (which never resets in a long-lived server), or a CSP nonce. The failure is silent and total. The page renders correctly, every content assertion still passes, the header is still present, and the only symptom is that no `If-None-Match` ever matches, so every revalidation ships the whole document instead of an empty 304. A page under CSP is excluded from the server HTML cache for exactly this reason (the nonce must differ per response). If a page opts into a public `Cache-Control`, guard it with a test that renders the page twice, through its layout, and asserts the two outputs are byte-identical. + ## Rate limiting `rateLimit()` is middleware backed by the pluggable cache store (memory by default, Redis when the global store is switched). Fixed-window. diff --git a/website/app/docs/cache/page.ts b/website/app/docs/cache/page.ts index f3bcc5297..6e300800a 100644 --- a/website/app/docs/cache/page.ts +++ b/website/app/docs/cache/page.ts @@ -94,6 +94,12 @@ export const metadata = {

This sets the standard Cache-Control header on the HTTP response. Browsers and CDNs cache the rendered page without any server-side state.

+

Setting cacheControl to anything other than no-store or private also opts the page into conditional GET: WebJs attaches a weak ETag and answers a matching If-None-Match with a 304, so a revalidation costs a few hundred bytes instead of the whole document. A bare max-age=60 is enough; the public keyword controls shared-cache storage, not whether you get an ETag.

+ +

That path only works if the page renders the same bytes twice. The ETag is a hash of the response body, so any per-render-varying value anywhere in the document defeats it: a Date.now(), a Math.random(), an id from a module-scope counter (which never resets in a long-lived server), or a CSP nonce, which is why a page under CSP is excluded from the server HTML cache. Nothing errors when this happens. The page renders correctly, every content assertion still passes, and the only symptom is a caching layer that silently never engages, so it is worth a test that renders the page twice, through its layout, and asserts the two outputs are identical.

+ +

Note that max-age=0 is a common default worth thinking about. It keeps the browser revalidating on every view, which is right when deploys must be visible immediately, but it means a stored copy is never reused directly. A small non-zero value is what produces real browser cache hits on back/forward and repeat visits.

+

Server HTML Response Cache (export const revalidate)

For a page that renders identical HTML for every visitor, opt into the server HTML response cache so the SSR pipeline runs once per window instead of once per request (webjs's no-build equivalent of Next.js's Full Route Cache and ISR). Declare a revalidation window on the page module:

diff --git a/website/app/layout.ts b/website/app/layout.ts index 3e23620dd..2e6c15340 100644 --- a/website/app/layout.ts +++ b/website/app/layout.ts @@ -46,9 +46,20 @@ export function generateMetadata(ctx: { url: string }) { // The marketing site is identical for every visitor (no per-user / session // reads), so it is safe to cache at the CDN. Set on the root layout so it // applies to every page (a per-user page could override with no-store). - // `s-maxage` is the edge cache; `max-age=0` keeps the browser revalidating; - // `stale-while-revalidate` serves instantly while refreshing. - cacheControl: 'public, max-age=0, s-maxage=600, stale-while-revalidate=86400', + // `s-maxage` is the edge cache; `stale-while-revalidate` serves instantly + // while refreshing. `max-age=60` is the browser copy: the previous `0` meant + // a page was NEVER reusable from disk, so every view paid a round trip even + // when nothing had changed. 60s is the smallest value that yields real + // browser cache hits (back/forward, repeat visits, a second tab) while + // bounding how long a reader can hold pre-deploy HTML to one minute. + // Known tradeoff (#1131): the client router fetches with the default HTTP + // cache mode, so within that minute a prefetch or soft nav can be served + // wholly from cache, handing the router pre-deploy `x-webjs-build` / + // `x-webjs-src` headers. Its deploy check then compares two equally stale + // ids and skips the snapshot eviction. `max-age=0` avoided that by forcing + // a revalidation every time. Accepted here because the blast radius is one + // minute of slightly stale marketing copy, and a hard nav still corrects it. + cacheControl: 'public, max-age=60, s-maxage=600, stale-while-revalidate=86400', title: TITLE, description: DESCRIPTION, openGraph: { diff --git a/website/components/copy-cmd.ts b/website/components/copy-cmd.ts index 3de7a3e14..75ab4f732 100644 --- a/website/components/copy-cmd.ts +++ b/website/components/copy-cmd.ts @@ -5,10 +5,19 @@ import { WebComponent, html, signal } from '@webjsdev/core'; * affordance. Light DOM, Tailwind utilities throughout. The whole * inner wrapper is the click target (text or icon both trigger copy); * the icon is an always-visible visual hint, not a separate focusable - * element. The command text is the button's accessible NAME, and an - * sr-only aria-describedby hint adds "Copy command to clipboard" as its - * description, so a screen reader announces both the payload and the - * action without the label hiding the command. + * element. The command text is the click target's accessible NAME (no + * aria-label hides it), and `title="Copy command to clipboard"` supplies + * the accessible DESCRIPTION, so a screen reader announces the payload and + * the action. The title doubles as a native hover tooltip. + * + * The description is a title attribute rather than an aria-describedby + * reference on purpose. A reference needs a document-unique id, the only + * way to mint one during SSR is a module-scope counter, and a counter + * never resets in a long-lived server, so consecutive renders of the same + * page emit different bytes. That changes the page's ETag on every request + * and silently kills the 304 path site-wide (#1127). A static attribute + * needs no id, adds no text content (so selections and _copy see only the + * command), and keeps the output byte-stable. * * Usage: * npm create webjs@latest my-app @@ -22,8 +31,6 @@ import { WebComponent, html, signal } from '@webjsdev/core'; * addEventListener in lifecycle hooks. Cleanup of the auto-reset * timer happens in disconnectedCallback. */ -let HINT_SEQ = 0; - export class CopyCmd extends WebComponent { copied = signal(false); // Increments on every successful copy. The live-region text is keyed off its @@ -32,9 +39,6 @@ export class CopyCmd extends WebComponent { // "Copied" even though `copied` is already true. private _copies = signal(0); private _resetTimer: number | undefined; - // Per-instance id so aria-describedby points at this button's own hint - // (multiple copy-cmd can share a page; the value is document-unique). - private _hintId = `copy-cmd-hint-${HINT_SEQ++}`; disconnectedCallback() { if (this._resetTimer) clearTimeout(this._resetTimer); @@ -90,7 +94,7 @@ export class CopyCmd extends WebComponent { data-copy-text role="button" tabindex="0" - aria-describedby=${this._hintId} + title="Copy command to clipboard" @click=${this._copy} @keydown=${this._onKey} > @@ -101,7 +105,6 @@ export class CopyCmd extends WebComponent { tabindex="-1" @click=${this._copy} >${isCopied ? CHECK_ICON : COPY_ICON} - Copy command to clipboard ${announce} `; diff --git a/website/test/components/browser/copy-cmd.test.js b/website/test/components/browser/copy-cmd.test.js index 7f47d345a..34c8e879a 100644 --- a/website/test/components/browser/copy-cmd.test.js +++ b/website/test/components/browser/copy-cmd.test.js @@ -87,30 +87,38 @@ suite('copy-cmd', () => { document.body.removeChild(el); }); - test('describes the copy action via aria-describedby without hiding the command', async () => { + test('describes the copy action via a title without hiding the command', async () => { const el = await mount('npm create webjs@latest my-app'); const target = el.querySelector('[data-copy-text]'); // The command stays the accessible NAME (slotted text, no aria-label)... assert.equal(target.getAttribute('aria-label'), null, 'no aria-label overrides the command name'); assert.ok(target.textContent.includes('npm create webjs@latest my-app'), 'the command is the accessible name'); - // ...and an sr-only describedby hint adds the copy ACTION as the description. - const hintId = target.getAttribute('aria-describedby'); - assert.ok(hintId, 'the button references a description via aria-describedby'); - const hint = el.querySelector('#' + hintId); - assert.ok(hint, 'the referenced hint element exists in the same subtree'); - assert.ok(/copy/i.test(hint.textContent) && /clipboard/i.test(hint.textContent), - 'the hint describes the copy-to-clipboard action'); - assert.ok(hint.className.includes('sr-only'), 'the hint is visually hidden (screen-reader only)'); + // ...and the title supplies the copy ACTION as the accessible description + // (plus a native hover tooltip). A title needs no id, which is the point: + // the old aria-describedby needed a document-unique id, the only SSR-safe + // source of one is a module-scope counter, and the counter made every + // render emit different bytes, killing the page ETag (#1127). + assert.equal(target.getAttribute('title'), 'Copy command to clipboard', + 'the title describes the copy-to-clipboard action'); + assert.equal(target.getAttribute('aria-describedby'), null, + 'no id-based description reference remains'); + // The description is an attribute, not text, so the target's text is + // EXACTLY the command: selections and _copy cannot pick up anything else. + assert.equal(target.textContent.trim(), 'npm create webjs@latest my-app', + 'the click target contains only the command text'); document.body.removeChild(el); }); - test('two copy-cmd on a page get distinct describedby hint ids', async () => { - const a = await mount('npm create webjs@latest one'); - const b = await mount('npm create webjs@latest two'); - const idA = a.querySelector('[data-copy-text]').getAttribute('aria-describedby'); - const idB = b.querySelector('[data-copy-text]').getAttribute('aria-describedby'); - assert.ok(idA && idB, 'both buttons carry a describedby id'); - assert.ok(idA !== idB, 'the two hint ids are unique so neither shadows the other'); + test('renders no generated ids, so repeated renders are byte-identical', async () => { + // The regression guard for #1127. Any per-render-varying value in the + // component's output (a counter, a timestamp, a random id) changes the + // page's bytes between renders, which changes its ETag, which means an + // If-None-Match can never match and a 304 is impossible. Assert the + // rendered markup of two independent instances of the SAME command is + // identical, which no monotonic id scheme can satisfy. + const a = await mount('npm create webjs@latest my-app'); + const b = await mount('npm create webjs@latest my-app'); + assert.equal(a.innerHTML, b.innerHTML, 'two renders of the same command emit identical markup'); a.remove(); b.remove(); }); diff --git a/website/test/ssr/conditional-get.test.ts b/website/test/ssr/conditional-get.test.ts new file mode 100644 index 000000000..505e5fdc8 --- /dev/null +++ b/website/test/ssr/conditional-get.test.ts @@ -0,0 +1,57 @@ +/** + * The caching contract end to end (#1127): the served page must carry a + * browser-reusable Cache-Control, an ETag, and answer a matching + * If-None-Match with an empty 304. + * + * This is the test of record for the whole fix, through the real request + * pipeline rather than renderToString. It fails on either regression path: + * + * - Revert the header to `max-age=0` (or any no-store / private value) and + * the max-age assertion fails. Nothing else in the suite reads the header, + * so without this a one-line revert of the fix ships green. + * - Reintroduce any per-render nondeterminism (the copy-cmd counter class) + * and the replay fails: the second render hashes to a different ETag, the + * recorded validator no longer matches, and the expected 304 comes back + * 200 with a full body, which is exactly how the bug presented in + * production. render-determinism.test.ts diagnoses that failure precisely; + * this test proves its consequence at the HTTP layer. + */ +import test, { before } from 'node:test'; +import assert from 'node:assert/strict'; +import { resolve, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { createRequestHandler } from '@webjsdev/server'; + +const WEBSITE_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..', '..'); + +let handle: (path: string, headers?: Record) => Promise; + +before(async () => { + const app = await createRequestHandler({ appDir: WEBSITE_ROOT, dev: false }); + await app.warmup?.(); + handle = (path, headers = {}) => app.handle(new Request('http://localhost' + path, { headers })); +}); + +for (const route of ['/', '/docs/getting-started']) { + test(`${route} is browser-cacheable and revalidates to an empty 304`, async () => { + const first = await handle(route); + assert.equal(first.status, 200); + + const cc = first.headers.get('cache-control') || ''; + const maxAge = Number(/(?:^|,)\s*max-age=(\d+)/.exec(cc)?.[1] ?? NaN); + assert.ok(maxAge > 0, + `max-age must be positive for the browser to ever reuse a stored copy, got: ${cc}`); + assert.ok(!/no-store|private/.test(cc), + `a no-store / private value opts the page out of the ETag path entirely, got: ${cc}`); + + const etag = first.headers.get('etag'); + assert.ok(etag, 'a cacheable 200 carries a validator'); + + const replay = await handle(route, { 'if-none-match': etag! }); + assert.equal(replay.status, 304, + 'replaying the ETag must revalidate; a 200 here means consecutive renders ' + + 'hash differently (see render-determinism.test.ts for the exact divergence)'); + assert.equal((await replay.text()).length, 0, 'a 304 has no body'); + assert.equal(replay.headers.get('etag'), etag, 'the validator survives the 304'); + }); +} diff --git a/website/test/ssr/render-determinism.test.ts b/website/test/ssr/render-determinism.test.ts new file mode 100644 index 000000000..86be243a5 --- /dev/null +++ b/website/test/ssr/render-determinism.test.ts @@ -0,0 +1,63 @@ +/** + * Render determinism, the precondition for conditional GET (#1127). + * + * The site opts every page into a public `Cache-Control` via the root layout, + * which also makes the framework attach a weak ETag and answer `If-None-Match` + * with a 304. That whole path is silently dead if a page does not render the + * same bytes twice: a different body hashes to a different ETag, the validator + * the browser holds never matches, and every revalidation ships the full + * document instead of an empty 304. + * + * Nothing else in the suite catches this. The page still renders, every + * assertion about its content still passes, and the only symptom is a caching + * layer that quietly never engages. The original offender was a module-scope + * counter minting `copy-cmd-hint-` ids that never reset in a long-lived + * server, so consecutive renders of the home page differed by a handful of + * digits. + * + * This test is deliberately generic rather than a check for that one counter, + * because the failure mode is a CLASS: any `Date.now()`, `Math.random()`, + * incrementing id, or iteration-order wobble in any component a page renders + * reintroduces it. + */ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { renderToString } from '@webjsdev/core/server'; +import RootLayout from '#app/layout.ts'; +import LandingPage from '#app/page.ts'; +import WhatIsWebJs from '#app/what-is-webjs/page.ts'; +import WhyWebJs from '#app/why-webjs/page.ts'; +import GettingStarted from '#app/docs/getting-started/page.ts'; + +// Render each page THROUGH the root layout. The ETag is computed over the whole +// served document, so a page-only render would miss anything the layout +// contributes, which is most of the risk surface: it is the layout that stamps +// the CSP nonce into four script tags and wraps every page's chrome. Guarding +// the page subtree alone would leave the exact file this test exists to protect +// (app/layout.ts) with no coverage at all. +const PAGES: Array<[string, () => unknown]> = [ + ['/', () => LandingPage()], + ['/what-is-webjs', () => WhatIsWebJs()], + ['/why-webjs', () => WhyWebJs()], + ['/docs/getting-started', () => GettingStarted()], +]; + +for (const [route, render] of PAGES) { + test(`${route} renders identical bytes twice (ETag stability)`, async () => { + const first = await renderToString(RootLayout({ children: render() }) as any); + const second = await renderToString(RootLayout({ children: render() }) as any); + assert.ok(first.length > 5000, 'renders a substantial full document, not a fragment'); + if (first !== second) { + // Surface the first divergence rather than dumping two large documents, + // so the failure names the offending markup directly. + const a = first.split('\n'); + const b = second.split('\n'); + const i = a.findIndex((line, n) => line !== b[n]); + assert.fail( + `${route} rendered different bytes on a second render, so its ETag changes every request ` + + `and a 304 is impossible. First divergence at line ${i + 1}:\n` + + ` render 1: ${a[i]?.trim()}\n render 2: ${b[i]?.trim()}`, + ); + } + }); +}