fix(build): the prerender preloaded the logo, hoisted any tag, and hid its errors
Four faults in the one build step that decides what a crawler receives. Closes #233 and #232. 1. Every page preloaded /logo.png. The hero hint was keyed on the first `loading="eager"` image, and that is the header logo, on every page. So the image each page actually paints first was never preloaded, and React's own preload for it was being discarded as a duplicate. It now keys on the image React marked with a high fetch priority, matched case-insensitively, because React writes the attribute camelCase in HTML and a case-sensitive match would have quietly removed every preload instead. 2. The hoist moved ANY title, meta or link out of the body into <head>. An inline <svg><title> is a picture's label, and microdata rides in <meta itemprop>: both would have become page-level head tags the moment the long-form copy carried an icon with a title. SVG blocks are now parked before the hoist, and itemprop tags stay where they are. 3. `page.replace('</head>', body)` interprets `$&`, `$'` and `$$` INSIDE the replacement, and the replacement is page copy. React escapes & into &, so any `$` immediately before an escaped character injected markup. Copy carries no `$` today; the next page with a price would have. Replacement is now a split and join, and each marker must appear exactly once. 4. A render error named no route, and a Suspense fallback shipped silently as an empty page. Both now fail the build and say which route. Also refuses to run over its own output: dist/index.html is both the template and the home page, so a second run without a rebuild gave every page two canonicals. Proven by mutation, each restored afterwards: a probe <svg><title> in the footer stays in the body on all 19 pages and no page gains a second title; a Suspense boundary fails the build naming the route; a second prerender run refuses; copy reading "Save $10 & more" reaches the page literally with one root div; and every page now preloads its own hero, with none preloading the logo. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
null
parent
fef8b0718c
commit
28b07abc28
|
|
@ -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/` |
|
||||
|
|
|
|||
|
|
@ -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
|
||||
`<head>`, **except** inside an inline `<svg>` (an SVG `<title>` is a picture's
|
||||
label, not the page's) and except `<meta itemprop>` 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`
|
||||
|
|
|
|||
|
|
@ -59,24 +59,68 @@ const TEMPLATE_TAGS_TO_STRIP = [
|
|||
const HOISTABLE_TAGS =
|
||||
/<title[^>]*>[\s\S]*?<\/title>|<meta\b[^>]*?\/?>|<link\b[^>]*?\/?>|<script[^>]*type="application\/ld\+json"[^>]*>[\s\S]*?<\/script>/g
|
||||
|
||||
// An inline <svg> may carry its own <title>, and microdata rides in
|
||||
// <meta itemprop> tags. Neither belongs in <head>: 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 = /<svg\b[\s\S]*?<\/svg>/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(/<img[^>]*loading="eager"[^>]*>/)?.[0]
|
||||
.match(/src="([^"]+)"/)?.[1]
|
||||
.match(/<img\b[^>]*\bfetchpriority="high"[^>]*>/i)?.[0]
|
||||
?.match(/\bsrc="([^"]+)"/i)?.[1]
|
||||
|
||||
const head = []
|
||||
if (heroSrc) {
|
||||
head.push(`<link rel="preload" as="image" href="${heroSrc}" fetchpriority="high" />`)
|
||||
}
|
||||
// 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>', ` ${head.join('\n ')}\n </head>`)
|
||||
page = page.replace('<div id="root"></div>', `<div id="root">${body}</div>`)
|
||||
page = replaceOnce(page, '</head>', ` ${head.join('\n ')}\n </head>`, url)
|
||||
page = replaceOnce(page, '<div id="root"></div>', `<div id="root">${body}</div>`, 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)
|
||||
|
|
|
|||
Loading…
Reference in New Issue