diff --git a/docs/TOOLS.md b/docs/TOOLS.md index 49e3fac..7607131 100644 --- a/docs/TOOLS.md +++ b/docs/TOOLS.md @@ -126,7 +126,7 @@ Run from the repository root. | --- | --- | | `npm install` | dependencies | | `npm run dev` | Vite and the Express API together, via `concurrently`. Frontend on 5173, API on 3001 | -| `npm run build` | **three steps**: the client bundle, then an SSR bundle from `src/entry-server.jsx`, then `scripts/prerender.js`, which writes static HTML for every route. This is the only real gate this project has | +| `npm run build` | **three steps**: the client bundle, then an SSR bundle from `src/entry-server.jsx`, then `scripts/prerender.js`, which writes static HTML for every route. This is the only real gate this project has, and it refuses rather than emit a wrong page: a render that throws, a Suspense fallback, or a template that already carries a canonical each fail the build, naming the route | | `npm run build:client` | the client bundle alone. Does **not** prerender — do not use it to produce a release | | `npm run preview` | serve the built client | | `npm start` / `npm run server` | the Express server alone, serving `dist/` | diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 3b182f4..7b83bac 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -54,6 +54,25 @@ policy text live there as plain data, imported by both the client pages and to static HTML at build time using `src/entry-server.jsx`. Nothing at request time renders React on the server. +It refuses to produce a page rather than produce a wrong one, and each refusal +names its route: + +- a render that throws, so the build does not report a stack trace with no clue + which of the nineteen pages produced it +- a Suspense fallback (``), because `renderToString` does not wait: a + lazy import anywhere above a route would silently ship an empty page to every + crawler +- a template that already carries a canonical, which means the script is being + run over its own output, since `dist/index.html` is both template and home page + +Two rules about what it moves. Metadata React leaves inline is hoisted into +``, **except** inside an inline `` (an SVG `` is a picture's +label, not the page's) and except `` microdata, which belongs +beside the thing it describes. And the hero preload is keyed on the image React +marked with a high fetch priority: keying it on `loading="eager"` matched the +header logo, so for months every page preloaded the logo and no page preloaded +its own hero. + ### The three boundaries worth knowing about 1. **The privacy policy has two renderers, one source.** `src/data/privacyPolicy.js` diff --git a/scripts/prerender.js b/scripts/prerender.js index 3179070..fc18b6f 100644 --- a/scripts/prerender.js +++ b/scripts/prerender.js @@ -59,24 +59,68 @@ const TEMPLATE_TAGS_TO_STRIP = [ const HOISTABLE_TAGS = /]*>[\s\S]*?<\/title>|]*?\/?>|]*?\/?>|]*type="application\/ld\+json"[^>]*>[\s\S]*?<\/script>/g +// An inline may carry its own , and microdata rides in +// tags. Neither belongs in : hoisting an SVG title gives +// the page two titles, which is the exact shape of defect this hoist exists to +// prevent. So SVG blocks are parked before the hoist and put back after. +const SVG_BLOCK = //gi +const SVG_TOKEN = 'svg' + +// renderToString does not wait for Suspense: it emits the fallback and marks it. +// A page carrying one of these lost its content silently, and crawlers would +// receive the fallback as the page. +const SUSPENSE_MARKERS = ['', ''] + +// Substitute a marker that must appear exactly once, without regex replacement +// semantics. String.replace interprets `$&`, `$'` and `$$` inside the +// REPLACEMENT, and the replacement here is page copy, which is not ours to +// trust: one `$` before an escaped entity would inject markup into the page. +const replaceOnce = (text, marker, replacement, url) => { + const parts = text.split(marker) + if (parts.length !== 2) { + throw new Error( + `prerender: ${url}: expected exactly one ${marker} in the template, found ${parts.length - 1}.`, + ) + } + return `${parts[0]}${replacement}${parts[1]}` +} + const buildPage = (template, url) => { - const { html } = render(url) + let html + try { + ;({ html } = render(url)) + } catch (error) { + // Without the route, the build fails with a stack trace and no clue which + // of the nineteen pages produced it. + throw new Error(`prerender: ${url} could not be rendered: ${error.message}`, { cause: error }) + } - const hoisted = html.match(HOISTABLE_TAGS) || [] - const body = html.replace(HOISTABLE_TAGS, '') + const svgs = [] + const parked = html.replace(SVG_BLOCK, (svg) => `${SVG_TOKEN}${svgs.push(svg) - 1}${SVG_TOKEN}`) - // Preload this route's own LCP image. The hero is the one marked eager during - // render, so the hint always matches what the page actually paints first. + const hoisted = [] + const body = parked + .replace(HOISTABLE_TAGS, (tag) => { + if (/\bitemprop=/i.test(tag)) return tag + hoisted.push(tag) + return '' + }) + .replace(new RegExp(`${SVG_TOKEN}(\\d+)${SVG_TOKEN}`, 'g'), (_, index) => svgs[Number(index)]) + + // Preload this route's own LCP image: the one React marked with a high fetch + // priority. Keying on `loading="eager"` matched the header logo, which is on + // every page, so every page preloaded the logo and no page preloaded its own + // hero. React writes the attribute camelCase in HTML, hence the /i. const heroSrc = html - .match(/]*loading="eager"[^>]*>/)?.[0] - .match(/src="([^"]+)"/)?.[1] + .match(/]*\bfetchpriority="high"[^>]*>/i)?.[0] + ?.match(/\bsrc="([^"]+)"/i)?.[1] const head = [] if (heroSrc) { head.push(``) } - // React emits its own image preload for the eager hero; drop it so the hint - // above isn't duplicated. + // React emits its own preload for that hero; drop it so the hint above is not + // duplicated. for (const tag of hoisted) { if (/rel="preload"[^>]*as="image"/.test(tag)) continue head.push(tag) @@ -87,8 +131,17 @@ const buildPage = (template, url) => { page = page.replace(pattern, '') } - page = page.replace('', ` ${head.join('\n ')}\n `) - page = page.replace('
', `
${body}
`) + page = replaceOnce(page, '', ` ${head.join('\n ')}\n `, url) + page = replaceOnce(page, '
', `
${body}
`, url) + + const marker = SUSPENSE_MARKERS.find((m) => page.includes(m)) + if (marker) { + throw new Error( + `prerender: ${url} shipped a Suspense fallback (${marker}) instead of its content. ` + + 'renderToString does not wait, so a lazy import or a Suspense boundary above this route ' + + 'silently empties the page for every crawler.', + ) + } return page } @@ -100,6 +153,16 @@ const outputPathFor = (url) => const template = readFileSync(path.join(distDir, 'index.html'), 'utf8') +// dist/index.html is both the template and the output for `/`, so running this +// script twice without a rebuild would treat a finished page as the template and +// give every page two canonicals and two of every head tag. +if (/rel="canonical"/.test(template)) { + throw new Error( + 'prerender: dist/index.html already carries a canonical, so it is a rendered page rather than the ' + + 'template. Run `vite build` before prerendering.', + ) +} + const written = [] for (const url of routes) { const page = buildPage(template, url)