From 75643abd7d0ddd7aadf61c44b7ce8fab7e4c6f4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Artur=20M=C4=99dryga=C5=82?= Date: Thu, 20 Aug 2026 11:49:40 +0200 Subject: [PATCH 1/3] fix(runner): carry the demo's head assets into the Tier-1 preview (DEV-2576) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The classic bundler renders a demo's inside its own document shell and discards the authored . Measured inside a live preview: document.title is "Sandbox - CodeSandbox" and there are no stylesheet links, while the bundler's own input FS still holds the authored /index.html complete with its tags. A demo whose theme CSS is a CDN therefore rendered core-only on /share and /edit — --ht-* undefined, cell padding 0px, borders falling back to the demo's own --ht-foreground-color — while /d, which serves the real HTML, looked right. withInjections already distrusts the head for a + + +
+ + + diff --git a/runner/pipeline/head-assets.test.mjs b/runner/pipeline/head-assets.test.mjs new file mode 100644 index 000000000..01791584b --- /dev/null +++ b/runner/pipeline/head-assets.test.mjs @@ -0,0 +1,472 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import vm from "node:vm"; +import fs from "node:fs"; +import path from "node:path"; +import { Parser } from "acorn"; +import { SandpackRuntime } from "../packages/runtime/dist/sandpack.js"; +import { SCHEME_MESSAGE_TYPE } from "../packages/runtime/dist/scheme.js"; +import { MONITOR_MESSAGE_TYPE } from "../packages/runtime/dist/monitor.js"; +import { + HEAD_ASSETS_MARKER, + HEAD_ASSETS_LINE_PREFIX, + HEAD_ASSETS_LINE_SUFFIX, + extractHeadAssets, + headAssetsSource, + headAssetsModuleLine, + injectHeadAssets, + stripInjectedHeadAssets, +} from "../packages/runtime/dist/head-assets.js"; + +// DEV-2576. The classic bundler renders the demo's inside its own document +// shell and throws the authored away — measured inside a live preview: +// `document.title` is "Sandbox - CodeSandbox" and there are no stylesheet links, +// while the bundler's own input FS still holds the authored HTML intact. So the +// head has to be re-created from the module entry, which is the one file the +// bundler is guaranteed to evaluate. +// +// What is *not* re-created is a local stylesheet: probing a payload on the deployed +// runner showed `./styles.css` already applying (the bundler resolves the local URL +// through the module graph) while the inline '); + assert.deepEqual(assets, [{ kind: "style", css: "body { color: #000 }", media: "print" }]); +}); + +test("no head, or a head with nothing to carry, extracts to nothing", () => { + assert.deepEqual(extractHeadAssets("hi"), []); + assert.deepEqual(extractHeadAssets(""), []); + assert.deepEqual(extractHeadAssets(''), []); +}); + +// ------------------------------------------------------------ emitted payload + +test("the payload parses as ES5", () => { + // The classic bundler runs its own 2018-era babel over the module entry, and a + // parse failure there presents as a blank preview with no error card. `new + // Function` in modern node accepts plenty that babel refuses, so the check names + // the syntax level rather than just executing the string. + Parser.parse(headAssetsSource(extractHeadAssets(FIXTURE)), { ecmaVersion: 5 }); +}); + +test("the payload is one physical line, appended after the demo's own source", () => { + // Appended, like the scheme receiver and unlike the monitor: this line carries + // the demo's own bytes, so it can be long, and babel's code frame prints the two + // lines *above* a fault verbatim. Prepending it would bury a syntax error on + // authored line 1 the way DEV-2557's reporter did. + const out = injectHeadAssets(filesWith(), HTML_ENTRY, MODULE_ENTRY); + const code = out[MODULE_ENTRY]; + assert.ok(code.startsWith("grid();\n"), "the authored source stays first"); + const last = code.trimEnd().split("\n").at(-1); + assert.equal(last, lineFrom(FIXTURE)); + assert.equal(last.split("\n").length, 1); +}); + +test("the strip constants compose the line exactly, so the strip cannot rot", () => { + // Matched against the exported constants rather than a pattern, for the reason + // `stripInjectedReporter` states: a regex over the injected *shape* keeps passing + // its own tests while silently ceasing to match a reworded injection. + const assets = extractHeadAssets(FIXTURE); + const line = headAssetsModuleLine(assets); + assert.equal(line, `try{(0,eval)(${JSON.stringify(headAssetsSource(assets))})}catch(e){}`); + assert.ok(line.startsWith(HEAD_ASSETS_LINE_PREFIX)); + assert.ok(line.endsWith(HEAD_ASSETS_LINE_SUFFIX)); + + const message = `SyntaxError: Unexpected token\n${line}\n at eval`; + const stripped = stripInjectedHeadAssets(message); + assert.ok(!stripped.includes(CDN_THEME), "the demo's head is out of the message"); + assert.ok(stripped.includes("SyntaxError: Unexpected token"), "the diagnostic survives"); +}); + +test("the payload carries no marker of the other two injectors", () => { + // Both siblings decide idempotency with indexOf over the whole entry source, so a + // payload that happened to contain their marker would make the colour-scheme + // bridge or the monitor silently inert. + const line = lineFrom(FIXTURE); + assert.ok(!line.includes(SCHEME_MESSAGE_TYPE)); + assert.ok(!line.includes(MONITOR_MESSAGE_TYPE)); + assert.ok(line.includes(HEAD_ASSETS_MARKER)); +}); + +test("no timestamp, counter or random value reaches the payload", () => { + // `SandpackRuntime.pushUpdate` skips the push when the derived sandbox is + // unchanged, and that skip is a correctness fix: the bundler's no-change path + // resets the document without re-evaluating any module. A fresh value per call + // would make every keystroke a real compile. + const source = headAssetsSource(extractHeadAssets(FIXTURE)); + assert.doesNotMatch(source, /Date\.now|Math\.random|new Date/); + const once = lineFrom(FIXTURE); + const twice = lineFrom(FIXTURE); + assert.equal(once, twice); +}); + +test("editing the head does change the payload", () => { + // The inverse of the scheme receiver's contract, and deliberately so: that one is + // a constant because it learns the scheme over postMessage, while this one *is* + // the head. A head edit has to produce a real diff, or the preview keeps the old + // title and the old stylesheet. + const edited = FIXTURE.replace("Head assets demo", "Head assets demo!"); + assert.notEqual(lineFrom(edited), lineFrom(FIXTURE)); +}); + +// --------------------------------------------------------------- the injector + +test("the authored map is untouched and only the module entry gains the line", () => { + // `/d/:id` is correct today *because* the static build reads the authored map, + // and Download-zip, fork and the CodeSandbox/StackBlitz exports read it too. A + // prelude leaking in there would ship runner plumbing inside every copy of a demo. + const authored = filesWith(); + const before = JSON.stringify(authored); + + const out = injectHeadAssets(authored, HTML_ENTRY, MODULE_ENTRY); + + assert.equal(JSON.stringify(authored), before, "authored map untouched"); + assert.ok(out[MODULE_ENTRY].includes(HEAD_ASSETS_MARKER), "module entry carries the payload"); + assert.equal(out[HTML_ENTRY], authored[HTML_ENTRY], "the HTML entry is read, never written"); + assert.equal(out["/styles.css"], authored["/styles.css"], "non-entry files untouched"); +}); + +test("nothing to do returns the very same object", () => { + // `withInjections` reduces over this result and `sameFiles` diffs it key by key, + // so a fresh object with equal contents is fine for correctness but wasteful — and + // returning the same reference is what the two existing injectors promise. + const files = filesWith(); + assert.equal(injectHeadAssets(files, null, MODULE_ENTRY), files, "no htmlEntry"); + assert.equal(injectHeadAssets(files, undefined, MODULE_ENTRY), files, "htmlEntry undefined"); + assert.equal(injectHeadAssets(files, "/nope.html", MODULE_ENTRY), files, "html file absent"); + assert.equal(injectHeadAssets(files, HTML_ENTRY, "/nope.js"), files, "module file absent"); + assert.equal(injectHeadAssets(files, HTML_ENTRY, HTML_ENTRY), files, "module entry is the document"); + + const bareHead = { [HTML_ENTRY]: "", [MODULE_ENTRY]: "grid();" }; + assert.equal(injectHeadAssets(bareHead, HTML_ENTRY, MODULE_ENTRY), bareHead, "nothing to carry"); +}); + +test("injecting twice is a no-op, even when the head changed underneath", () => { + const once = injectHeadAssets(filesWith(), HTML_ENTRY, MODULE_ENTRY); + const twice = injectHeadAssets(once, HTML_ENTRY, MODULE_ENTRY); + assert.equal(twice, once, "same object back"); + + const withNewHead = { ...once, [HTML_ENTRY]: FIXTURE.replace("Head assets demo", "Changed") }; + const thrice = injectHeadAssets(withNewHead, HTML_ENTRY, MODULE_ENTRY); + assert.equal(thrice, withNewHead, "marker wins over a changed head"); + assert.equal( + thrice[MODULE_ENTRY].split(HEAD_ASSETS_MARKER).length - 1, + once[MODULE_ENTRY].split(HEAD_ASSETS_MARKER).length - 1, + "not double-prefixed", + ); +}); + +// ------------------------------------------------------------------ execution + +/** A fake document, enough for the payload to build nodes against. */ +function fakeDocument() { + const head = { children: [], appendChild(node) { this.children.push(node); } }; + const created = []; + const doc = { + title: "Sandbox - CodeSandbox", + head, + createElement(tag) { + const node = { + tagName: tag.toUpperCase(), + attrs: {}, + children: [], + textContent: "", + setAttribute(name, value) { this.attrs[name] = value; }, + appendChild(child) { this.children.push(child); this.textContent += child.text ?? ""; }, + }; + if (tag === "link") { + Object.defineProperty(node, "href", { + get() { return this.attrs.href ?? ""; }, + set(value) { this.attrs.href = value; }, + }); + } + if (tag === "textarea") { + Object.defineProperty(node, "innerHTML", { + set(value) { this.value = value.replace(/&/g, "&").replace(/—/g, "—"); }, + }); + } + created.push(node); + return node; + }, + createTextNode: (text) => ({ text }), + createEvent: () => ({ initEvent() {} }), + getElementsByTagName(tag) { + const want = tag.toUpperCase(); + return head.children.filter((node) => node.tagName === want); + }, + }; + return { doc, created }; +} + +/** Run an injected module entry the way the bundler would, and report what it built. */ +function runPayload(html, seed = []) { + const { doc } = fakeDocument(); + seed.forEach((node) => doc.head.appendChild(node)); + const dispatched = []; + const context = vm.createContext({ + document: doc, + window: { dispatchEvent: (event) => dispatched.push(event) }, + demoRan: false, + }); + context.window.document = doc; + const out = injectHeadAssets({ [HTML_ENTRY]: html, [MODULE_ENTRY]: "demoRan = true;\n" }, HTML_ENTRY, MODULE_ENTRY); + vm.runInContext(out[MODULE_ENTRY], context); + return { doc, dispatched, context, appended: doc.head.children }; +} + +test("running the payload rebuilds the authored head", () => { + // The one case that proves the feature: everything above asserts strings, and the + // payload is a hand-written ES5 blob no typechecker reads. + const { doc, appended, context } = runPayload(FIXTURE); + + assert.equal(context.demoRan, true, "the demo's own source still evaluates"); + assert.equal(doc.title, "Head assets demo", "the title reaches the document"); + assert.deepEqual( + appended.map((node) => `${node.tagName} ${node.attrs.href ?? node.attrs.name ?? node.textContent}`), + [ + `LINK ${CDN_CORE}`, + `LINK ${CDN_THEME}`, + "STYLE :root { --e2e-head-sentinel: 7px }", + "META viewport", + `LINK ${CDN_ICONS}`, + "LINK data:text/css,%3Aroot%7B--e2e-data-sentinel%3A%209px%7D", + ], + "every asset, in authored order", + ); + assert.ok( + appended.every((node) => node.attrs["data-hot-runner-head"] === ""), + "every generated node is tagged, so a second evaluation can recognise its own work", + ); +}); + +test("a stylesheet the document already has is not added twice", () => { + // The head-dropping is measured on the parcel path; `vue-cli` is not, and a + // template that *did* keep the head would otherwise fetch every stylesheet twice + // and stack duplicate rules. `href` on the seed, not just the attribute: that is + // what a real link reports, and it is what the guard compares. + const existing = { tagName: "LINK", attrs: { href: CDN_THEME }, href: CDN_THEME, children: [], textContent: "" }; + const { appended } = runPayload(FIXTURE, [existing]); + const themeLinks = appended.filter((node) => node.href === CDN_THEME || node.attrs.href === CDN_THEME); + assert.equal(themeLinks.length, 1, "only the pre-existing link, no second copy"); + assert.equal(themeLinks[0], existing, "and it is the one that was already there"); + // The rest of the head still lands — the guard is per-asset, not a bail-out. + assert.ok(appended.some((node) => node.attrs.href === CDN_CORE), "the other stylesheet still lands"); +}); + +test("a style block the document already has is not added twice", () => { + const css = ":root { --e2e-head-sentinel: 7px }"; + const existing = { tagName: "STYLE", attrs: {}, children: [], textContent: css }; + const { appended } = runPayload(FIXTURE, [existing]); + const matching = appended.filter((node) => node.textContent === css); + assert.equal(matching.length, 1, "only the pre-existing style"); + assert.equal(matching[0], existing); +}); + +test("the grid is nudged to re-measure once, plus once per stylesheet", () => { + // Appending means the demo has already built its grid against an unstyled DOM, + // and a cross-origin stylesheet arrives later still. A generic bubbling `resize` + // is the demo-agnostic lever — Handsontable's own listener re-measures — and it + // must be bounded, not a timer or a poll. + const { dispatched, appended } = runPayload(FIXTURE); + assert.equal(dispatched.length, 1, "one nudge after the loop"); + + const links = appended.filter((node) => node.tagName === "LINK"); + links.forEach((node) => node.onload?.()); + assert.equal(dispatched.length, 1 + links.length, "one more per stylesheet that loads"); +}); + +// -------------------------------------------------------------- the runtime + +const entryFor = (over) => ({ + framework: "javascript", + displayName: "JavaScript", + tier: 1, + engine: "sandpack", + sandpackTemplate: "parcel", + sandpackEnvironment: "parcel", + container: null, + htWrappers: [], + entry: "/index.js", + htmlEntry: "/index.html", + devCommand: null, + buildCommand: "build", + outputDir: "dist", + outputGlob: null, + ...over, +}); + +/** Record what the runtime hands the bundler, without mounting one. */ +function published(entry, files) { + const runtime = new SandpackRuntime(entry, { iframe: {} }); + const pushes = []; + runtime.client = { + updateSandbox: (setup) => pushes.push(setup), + destroy() {}, + listen: () => () => {}, + }; + runtime.files = { ...files }; + return { runtime, pushes }; +} + +test("the runtime injects the head into the module entry, never into the HTML entry", async () => { + // The existing injectors are reduced over *both* targets on purpose. This one is + // cross-file, so folding it into that reduce would inject into the HTML too — + // where, by the premise of the bug, it would be thrown away. + const { runtime } = published(entryFor(), filesWith()); + const derived = await runtime.sandboxFiles(); + + assert.ok(derived["/index.js"].includes(HEAD_ASSETS_MARKER), "module entry carries it"); + assert.ok(!derived["/index.html"].includes(HEAD_ASSETS_MARKER), "HTML entry does not"); + assert.ok(derived["/index.js"].includes(CDN_THEME), "and it carries the theme stylesheet"); +}); + +test("a vue-cli entry is covered too, even though its sandbox entry is the module", async () => { + // `HTML_ENTRY_ENVS` is {parcel, static}, so on vue-cli `resolveSandboxEntry` + // answers /src/main.js and the head would never be seen. The gate is the catalog + // entry's own `htmlEntry`, which vue does declare. + const files = { + "/index.html": FIXTURE, + "/src/main.js": "grid();\n", + "/package.json": JSON.stringify({ dependencies: { handsontable: "18.0.0" } }), + }; + const { runtime } = published( + entryFor({ framework: "vue", sandpackEnvironment: "vue-cli", entry: "/src/main.js" }), + files, + ); + const derived = await runtime.sandboxFiles(); + assert.ok(derived["/src/main.js"].includes(HEAD_ASSETS_MARKER)); +}); + +test("an entry with no htmlEntry is left exactly as it is today", async () => { + const files = { + "/src/main.js": "grid();\n", + "/package.json": JSON.stringify({ dependencies: { handsontable: "18.0.0" } }), + }; + const { runtime } = published( + entryFor({ framework: "vue", sandpackEnvironment: "vue-cli", entry: "/src/main.js", htmlEntry: null }), + files, + ); + const derived = await runtime.sandboxFiles(); + assert.ok(!derived["/src/main.js"].includes(HEAD_ASSETS_MARKER)); +}); + +test("a demo whose title names another injector still gets that injector", async () => { + // Ordering, pinned: head assets are injected last, so the scheme and monitor + // guards — plain indexOf over the whole entry — are decided on demo bytes only. + const html = `${SCHEME_MESSAGE_TYPE}`; + const { runtime } = published(entryFor(), filesWith(html)); + const derived = await runtime.sandboxFiles(); + const line = derived["/index.js"]; + assert.ok(line.includes(HEAD_ASSETS_MARKER), "head payload present"); + assert.ok( + line.indexOf(SCHEME_MESSAGE_TYPE) !== line.lastIndexOf(SCHEME_MESSAGE_TYPE), + "the scheme receiver is present as well as the demo's title text", + ); +}); + +test("the runtime's authored map never sees the payload", async () => { + const { runtime } = published(entryFor(), filesWith()); + await runtime.sandboxFiles(); + assert.ok(!JSON.stringify(runtime.files).includes(HEAD_ASSETS_MARKER)); +}); diff --git a/runner/pipeline/sandpack-reload.test.mjs b/runner/pipeline/sandpack-reload.test.mjs index 1ac161bf9..013f70413 100644 --- a/runner/pipeline/sandpack-reload.test.mjs +++ b/runner/pipeline/sandpack-reload.test.mjs @@ -172,6 +172,25 @@ test("reload() stamps the entry, so the bundler always has a diff to act on", as } }); +test("an entry with no htmlEntry gets no head-assets payload", async () => { + // What every hand-built baseline in this file (and in theme-live-patch.test.mjs) + // silently depends on. `ENTRY.htmlEntry` is null and `FILES` holds no HTML at all, + // so the DEV-2576 head injection is inert here and `injectSchemeReceiver` alone is + // still the whole derived map. If that injection ever becomes unconditional, this + // case goes red and names the reason, instead of four unrelated assertions failing + // for a reason none of them is about. + const { runtime, client } = mounted(); + + await runtime.reload(); + + const code = client.pushes[0].setup.files["/src/main.js"].code; + assert.ok(!code.includes("hot-runner-head"), "no head payload without an htmlEntry"); + assert.ok( + code.startsWith(injectSchemeReceiver({ ...FILES }, ENTRY.entry)["/src/main.js"]), + "the scheme receiver is still the only thing appended before the stamp", + ); +}); + test("an edit that changes nothing is not pushed at all", async () => { const { runtime, client } = mounted(); diff --git a/runner/workers/api/src/import-url.ts b/runner/workers/api/src/import-url.ts index 57ac898eb..61caab0ea 100644 --- a/runner/workers/api/src/import-url.ts +++ b/runner/workers/api/src/import-url.ts @@ -553,8 +553,12 @@ export function normalizeCdnGlobals(html: string, js: string): NormalizedImport cssImports.push(`import 'handsontable/styles/${hotCss[2]}';`); return ""; } - // Any other stylesheet is left alone: a CDN loads fine and pins - // nothing that the version picker cares about. + // Any other stylesheet is left alone: it pins nothing the version picker cares + // about. It does *not* "load fine" on its own, which is what this comment used to + // claim: the Tier-1 bundler discards the authored , so a CDN reaches + // the live preview only because `head-assets.ts` re-creates it from the module + // entry (DEV-2576). The rewrite above predates that and exists for the version + // pin, not for the loading. if (!isScript) return tag; const found = packageFromCdnUrl(url); From 8be686f586159d43f037b0f78e121afd8adefd0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Artur=20M=C4=99dryga=C5=82?= Date: Thu, 20 Aug 2026 12:19:05 +0200 Subject: [PATCH 2/3] fix(runner): key the head-asset dedupe on rel, drop the window latch (DEV-2576, review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five findings from a high-effort review of #240, all reproduced against the built dist before changing anything. - The duplicate guard compared resolved hrefs only, so `` followed by `` — the canonical CDN idiom — appended the preload and then skipped the stylesheet as a duplicate. A preload styles nothing, so the module no-opped for the commonest shape it exists to fix. Now keyed on rel + resolved href, read off the reflected properties a real element exposes. - The `window.__hotRunnerHeadAssets` latch could only do harm: the per-asset guards already make a second evaluation inert, while the bundler resets the preview document on a recompile, so a flag on the surviving `window` left every later compile unstyled. Verified: same window, fresh document, zero nodes appended and the title back to the bundler's. Dropped. - `jsonInner` inherited `JSON.stringify`'s treatment of U+2028/U+2029, which are legal in ES2019+ string literals but LineTerminators in ES5, so one of them in a title or style block made the emitted line unparseable for the bundler's babel — acorn at ecmaVersion 5 said "Unterminated string constant". The module's own ES5 gate could not catch it: that covers the constant receiver, not the payload. - `extractHeadAssets` required an explicit ``. It is optional in HTML, and `/d/:id` renders such a demo themed, so the preview stayed divergent for that shape. Everything before `` is now the implicit head; a link in the body is still left alone, and that scope is now stated in a test. - The tag matcher's `[^>]*` truncated at a `>` inside a quoted attribute value, and the remainder was then read as attribute names — the authored value dropped, unrelated attributes attached, and on a a corrupted href. Quoted runs are matched as units now. Six regression tests, and the fake-document harness now reflects href/rel and answers getAttribute — without that it could not see the first finding at all. One of the new tests just asserts the receiver carries no backtick: it lives in a TS template literal, and a backtick in one of its comments silently closes it. Co-Authored-By: Claude Opus 5 --- runner/packages/runtime/src/head-assets.ts | 73 ++++++++++++--- runner/pipeline/head-assets.test.mjs | 104 ++++++++++++++++++++- 2 files changed, 163 insertions(+), 14 deletions(-) diff --git a/runner/packages/runtime/src/head-assets.ts b/runner/packages/runtime/src/head-assets.ts index df1aaac27..5fd06d9d7 100644 --- a/runner/packages/runtime/src/head-assets.ts +++ b/runner/packages/runtime/src/head-assets.ts @@ -43,13 +43,24 @@ export type HeadAsset = | { kind: "element"; tag: "link" | "meta"; attrs: [string, string][] }; const HEAD_RE = /]*>([\s\S]*?)<\/head>/i; +/** Where the implicit head ends when the document declares none. */ +const BODY_OPEN_RE = /` receivers, and a commented-out tag is not an asset. */ const SCRIPT_BLOCK_RE = /]*>[\s\S]*?<\/script>/gi; const COMMENT_RE = //g; -const TOKEN_RE = - /]*>([\s\S]*?)<\/title>|]*)>([\s\S]*?)<\/style>|<(link|meta)\b([^>]*?)\/?>/gi; +// `>` is legal inside a quoted attribute value (`content="x > y"`), and a `[^>]*` +// tag matcher truncates there — `attributesOf` then reads the rest of the value as +// attribute *names*, so the authored value is lost and unrelated attributes ride +// along. Quoted runs are therefore matched as units. +const TAG_BODY = String.raw`(?:"[^"]*"|'[^']*'|[^>])`; +const TOKEN_RE = new RegExp( + `([\\s\\S]*?)<\\/title>` + + `|([\\s\\S]*?)<\\/style>` + + `|<(link|meta)\\b(${TAG_BODY}*?)\\/?>`, + "gi", +); const ATTR_RE = /([a-zA-Z_:][-a-zA-Z0-9_:.]*)(?:\s*=\s*(?:"([^"]*)"|'([^']*)'|([^\s"'=<>`]+)))?/g; function attributesOf(raw: string): [string, string][] { @@ -75,9 +86,18 @@ const valueOf = (attrs: [string, string][], name: string): string | undefined => * the HTML (see `headAssetsModuleLine`). */ export function extractHeadAssets(html: string): HeadAsset[] { + // `` is optional in HTML — a document that omits it still has an implicit + // one, and `/d/:id` renders such a demo themed because the browser parses it that + // way. Falling back to "everything before " keeps the two paths in step. const head = HEAD_RE.exec(html); - if (head === null) return []; - const inner = (head[1] ?? "").replace(SCRIPT_BLOCK_RE, "").replace(COMMENT_RE, ""); + let scope: string; + if (head !== null) { + scope = head[1] ?? ""; + } else { + const body = BODY_OPEN_RE.exec(html); + scope = body === null ? html : html.slice(0, body.index); + } + const inner = scope.replace(SCRIPT_BLOCK_RE, "").replace(COMMENT_RE, ""); const assets: HeadAsset[] = []; TOKEN_RE.lastIndex = 0; @@ -125,15 +145,21 @@ export function extractHeadAssets(html: string): HeadAsset[] { * error card. `pipeline/head-assets.test.mjs` gates it with acorn at `ecmaVersion: 5`, * because `new Function` in modern node would accept plenty that babel refuses. * - * The three guards make the payload inert when the head *was* preserved — the + * The per-asset guards make the payload inert when the head *was* preserved — the * head-dropping is measured on the parcel path, and `vue-cli` is not — so it can be * applied to every entry that declares an `htmlEntry` without risking a second copy * of every stylesheet. + * + * Deliberately *no* `window`-level "already ran" latch, unlike `scheme.ts` and + * `monitor.ts`. Those two register listeners, which must be hooked once; this one + * creates DOM nodes, and the bundler resets the preview document on a recompile + * (`pushUpdate`'s own comment: "the bundler's no-change path resets the document + * without re-evaluating any module"). A latch on `window` — which survives that + * reset — would make every compile after the first leave the head empty, while + * buying nothing the per-asset guards do not already provide. */ const RECEIVER_HEAD = `(function (assets) { if (typeof document === 'undefined' || !assets || !assets.length) { return; } - if (window.__hotRunnerHeadAssets) { return; } - window.__hotRunnerHeadAssets = true; var MARK = '${MARK_ATTRIBUTE}'; var head = document.head || document.getElementsByTagName('head')[0] || document.documentElement; @@ -148,10 +174,23 @@ const RECEIVER_HEAD = `(function (assets) { window.dispatchEvent(event); } catch (e) {} } - function hasHref(url) { + function relOf(node) { + /* node.rel is the reflected property every real link has; getAttribute is the + fallback for anything that only carries the attribute. */ + var value = node.rel; + if (value === undefined || value === null) { + value = node.getAttribute ? node.getAttribute('rel') : null; + } + return (value || '').toLowerCase(); + } + /* Keyed on rel *and* href, not href alone: a preload link and a stylesheet link + for the same URL are the canonical CDN idiom, and an href-only guard appends the + preload and then skips the stylesheet as a duplicate — leaving the demo + unstyled, which is the bug this file exists to fix. */ + function hasLink(url, rel) { var links = document.getElementsByTagName('link'); for (var i = 0; i < links.length; i += 1) { - if (links[i].href === url) { return true; } + if (links[i].href === url && relOf(links[i]) === rel) { return true; } } return false; } @@ -189,7 +228,7 @@ const RECEIVER_HEAD = `(function (assets) { if (asset.tag === 'link') { /* element.href is the resolved absolute URL, which is what an existing link reports too — comparing the authored strings would miss a match. */ - if (hasHref(element.href)) { continue; } + if (hasLink(element.href, relOf(element))) { continue; } element.onload = nudge; element.onerror = nudge; } @@ -205,10 +244,20 @@ export function headAssetsSource(assets: HeadAsset[]): string { return RECEIVER_HEAD + JSON.stringify(assets) + RECEIVER_TAIL; } -/** `JSON.stringify` of a string, without its surrounding quotes. */ +/** + * `JSON.stringify` of a string, without its surrounding quotes. + * + * U+2028 and U+2029 are escaped by hand because `JSON.stringify` leaves them raw: + * they are legal inside an ES2019+ string literal but are LineTerminators in ES5, so + * one of them anywhere in a demo's `` or `<style>` (a paste out of a word + * processor is enough) makes the emitted line unparseable for the 2018-era babel the + * classic bundler runs — presenting as the blank preview with no error card that this + * module's ES5 discipline exists to avoid. The module's own acorn gate cannot catch + * it: that covers the constant receiver, not the demo-derived payload. + */ const jsonInner = (value: string): string => { const quoted = JSON.stringify(value); - return quoted.slice(1, quoted.length - 1); + return quoted.slice(1, quoted.length - 1).replace(/\u2028/g, "\\u2028").replace(/\u2029/g, "\\u2029"); }; /** diff --git a/runner/pipeline/head-assets.test.mjs b/runner/pipeline/head-assets.test.mjs index 01791584b..7b21de934 100644 --- a/runner/pipeline/head-assets.test.mjs +++ b/runner/pipeline/head-assets.test.mjs @@ -272,7 +272,14 @@ function fakeDocument() { attrs: {}, children: [], textContent: "", - setAttribute(name, value) { this.attrs[name] = value; }, + // Reflected properties and getAttribute both, because the payload reads `rel` + // and `href` off the node rather than out of its own descriptor — a harness + // that only stored attributes could not see a dedupe keyed on them. + setAttribute(name, value) { + this.attrs[name] = value; + if (name === "rel") this.rel = value; + }, + getAttribute(name) { return name in this.attrs ? this.attrs[name] : null; }, appendChild(child) { this.children.push(child); this.textContent += child.text ?? ""; }, }; if (tag === "link") { @@ -345,7 +352,17 @@ test("a stylesheet the document already has is not added twice", () => { // template that *did* keep the head would otherwise fetch every stylesheet twice // and stack duplicate rules. `href` on the seed, not just the attribute: that is // what a real link reports, and it is what the guard compares. - const existing = { tagName: "LINK", attrs: { href: CDN_THEME }, href: CDN_THEME, children: [], textContent: "" }; + // A real pre-existing stylesheet carries its rel, and the guard is keyed on rel + + // resolved href — a bare `<link href>` with no rel is a different link, not this one. + const existing = { + tagName: "LINK", + attrs: { rel: "stylesheet", href: CDN_THEME }, + rel: "stylesheet", + href: CDN_THEME, + children: [], + textContent: "", + getAttribute(name) { return name in this.attrs ? this.attrs[name] : null; }, + }; const { appended } = runPayload(FIXTURE, [existing]); const themeLinks = appended.filter((node) => node.href === CDN_THEME || node.attrs.href === CDN_THEME); assert.equal(themeLinks.length, 1, "only the pre-existing link, no second copy"); @@ -470,3 +487,86 @@ test("the runtime's authored map never sees the payload", async () => { await runtime.sandboxFiles(); assert.ok(!JSON.stringify(runtime.files).includes(HEAD_ASSETS_MARKER)); }); + +// ------------------------------------------------ regressions (review of PR #240) + +test("a preload and a stylesheet for the same URL both land", () => { + // `<link rel="preload" as="style" href="X">` followed by `<link rel="stylesheet" + // href="X">` is the canonical CDN idiom. Deduping on href alone appended the + // preload and then skipped the stylesheet as a duplicate — a preload styles + // nothing, so the demo stayed unthemed and this whole module no-opped for the + // commonest shape it exists to fix. The guard is keyed on rel *and* href. + const html = + '<head><link rel="preload" as="style" href="https://cdn.example.com/t.css">' + + '<link rel="stylesheet" href="https://cdn.example.com/t.css"></head>'; + const { appended } = runPayload(html); + assert.deepEqual( + appended.map((node) => `${node.attrs.rel} ${node.attrs.href}`), + ["preload https://cdn.example.com/t.css", "stylesheet https://cdn.example.com/t.css"], + ); +}); + +test("a second evaluation against a fresh document re-applies the head", () => { + // The bundler resets the preview document on a recompile without re-evaluating + // every module (`pushUpdate`'s comment). A `window`-level "already ran" latch — + // which `scheme.ts` and `monitor.ts` can afford because they register listeners, + // not nodes — would survive that reset and leave every later compile unstyled. + const html = '<head><title>T'; + const first = runPayload(html); + assert.equal(first.appended.length, 1); + + // Same window object, brand-new document: what a reset looks like from in here. + const { doc } = fakeDocument(); + const context = vm.createContext({ document: doc, window: first.context.window, demoRan: false }); + context.window.document = doc; + const out = injectHeadAssets({ [HTML_ENTRY]: html, [MODULE_ENTRY]: "demoRan = true;\n" }, HTML_ENTRY, MODULE_ENTRY); + vm.runInContext(out[MODULE_ENTRY], context); + + assert.equal(doc.head.children.length, 1, "the head is rebuilt, not skipped"); + assert.equal(doc.title, "T"); +}); + +test("a U+2028 in the head still emits an ES5-parseable line", () => { + // JSON.stringify leaves U+2028/U+2029 raw. They are legal in an ES2019+ string + // literal and LineTerminators in ES5, so one pasted into a title (a word processor + // is enough) made the emitted line unparseable for the bundler's babel — the blank + // preview with no error card. The module's own acorn gate covers the constant + // receiver, not the demo-derived payload, so it could not catch this. + const line = lineFrom(`a${"\u2028"}b`); + Parser.parse(line, { ecmaVersion: 5 }); + assert.ok(!stripInjectedHeadAssets(`x ${line} y`).includes("\u2028"), "and the strip still matches"); +}); + +test("a > inside a quoted attribute value does not truncate the tag", () => { + // `[^>]*` stopped at the first `>`, and the remainder of the value was then read as + // attribute *names*: the authored content was dropped and unrelated attributes rode + // along. On a that corrupts the href that gets loaded. + const assets = extractHeadAssets(''); + assert.deepEqual(assets, [ + { kind: "element", tag: "meta", attrs: [["name", "og:d"], ["content", "x > y"]] }, + ]); +}); + +test("a document with no element still hands over its implicit one", () => { + // is optional in HTML; the browser makes one, and `/d/:id` renders such a + // demo themed. Everything before is that implicit head. + const assets = extractHeadAssets( + 'hi', + ); + assert.deepEqual(assets, [ + { kind: "element", tag: "link", attrs: [["rel", "stylesheet"], ["href", "https://cdn.example.com/t.css"]] }, + ]); + + // Scope, stated: a link in the *body* is left alone. The bundler renders the body, + // so whatever it does with one there it does with or without this module. + assert.deepEqual( + extractHeadAssets('t'), + [{ kind: "title", text: "t" }], + ); +}); + +test("the receiver carries no backtick", () => { + // It lives inside a TS template literal. A backtick in one of its comments closes + // that literal, and the file stops compiling — twice while writing this module. + assert.ok(!headAssetsSource([]).includes("`")); +}); From 7f7199449494e342315425e243fda26564974ddd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Artur=20M=C4=99dryga=C5=82?= Date: Thu, 20 Aug 2026 12:22:26 +0200 Subject: [PATCH 3/3] fix(runner): decode character references in re-created head attributes (DEV-2576) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bugbot, on #240: attribute values travelled as raw source text, so an authored href="...?family=Inter&display=swap" — the idiomatic Google Fonts form — was re-created with the literal `&` and requested a URL the CDN does not have. The parser resolves references before the DOM sees them, so the stylesheet or font never arrived: the exact failure this module exists to end. The title was already decoded; attributes were not. Decoded through the same detached textarea, so it is the browser's own table rather than a hand-rolled one. A '; + + // Extraction keeps the source text; decoding belongs to the payload, where the + // browser's own table is available. + const assets = extractHeadAssets(html); + assert.equal(assets[0].attrs[1][1], "https://fonts.example.com/css2?family=Inter&display=swap"); + + const { appended } = runPayload(html); + assert.equal(appended[0].attrs.href, "https://fonts.example.com/css2?family=Inter&display=swap"); + assert.equal(appended[1].attrs.content, "a & b"); + assert.equal(appended[2].textContent, 'a[href*="&"] { color: #000 }', "style text stays raw"); +}); + +test("a title's character references are decoded too", () => { + const { doc } = runPayload("Sales & Ops — Q3"); + assert.equal(doc.title, "Sales & Ops — Q3"); +});