Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions runner/e2e/preview-head-assets.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,14 @@ const CDN_THEME = "https://cdn.jsdelivr.net/npm/handsontable@18/styles/ht-theme-

const DEMO_JS = `import Handsontable from 'handsontable';

// What a styled demo actually does, and the DEV-2581 regression in one line: this
// runs *before* the injected head assets are re-created, and it overrides the theme
// on the same selector the theme itself uses. Equal specificity, so whichever lands
// later wins — appending the re-created stylesheet erased this.
const override = document.createElement('style');
override.textContent = '.ht-theme-main { --ht-cell-vertical-padding: 11px }';
document.head.appendChild(override);

new Handsontable(document.getElementById('grid'), {
data: [['a', 1], ['b', 2], ['c', 3]],
colHeaders: true,
Expand Down Expand Up @@ -90,8 +98,42 @@ function readHead(page: Page) {
});
}

/**
* Wait until the re-created head has finished loading before reading it.
*
* The stylesheets are cross-origin `<link>`s, so they land some time after the grid
* becomes visible — reading immediately made this spec flake (measured: one run in
* three saw `links=0`). Polling the observable end state is the fix; a fixed sleep
* would either flake again or slow every run to the worst case.
*/
async function headSettled(page: Page) {
await expect
.poll(
async () => {
const head = await readHead(page);
// Non-zero cell padding is the honest "the theme stylesheet applied" signal.
// `themeVar !== ""` is not: the demo's own override sets that variable
// synchronously, so it is already non-empty while the cross-origin sheet is
// still in flight — a readiness check that passes before the thing under test
// has happened, which then fails a later assertion for the wrong reason.
const themed = head.stylesheets.length > 0 && !head.cellPadding.startsWith("0px");
return themed
? "ready"
: `pending: links=${head.stylesheets.length} padding=${head.cellPadding}`;
},
{ timeout: 30_000, message: "the re-created head never finished loading" },
)
.toBe("ready");
}

test.describe("head assets reach the live preview", () => {
test.skip(process.env.E2E_LIVE !== "1", "set E2E_LIVE=1 to run live-render checks");
// The config default is 60s, and the waits inside one test already allow more than
// that on a cold bundler: `previewReady` 120s, the grid 120s, `headSettled` 30s. At
// 60s the test aborts with Playwright's generic "Test timeout exceeded" *before*
// those waits can report what they measured — the least informative failure wins.
// Same budget as `preview-scheme.spec.ts`, the closest sibling.
test.describe.configure({ timeout: 300_000 });

test("a demo styled only from its <head> renders themed", async ({ page }) => {
await page.route("**/api/versions", (route) =>
Expand All @@ -109,6 +151,7 @@ test.describe("head assets reach the live preview", () => {
await previewReady(page);
await expect(grid(page)).toBeVisible({ timeout: 120_000 });

await headSettled(page);
const head = await readHead(page);

// Each of these is zero/empty on `master`; the message carries the measurement so
Expand All @@ -129,6 +172,12 @@ test.describe("head assets reach the live preview", () => {
expect(head.themeVar, `measured --ht-cell-vertical-padding: ${JSON.stringify(head.themeVar)}`).not.toBe("");
expect(head.cellPadding, `measured td padding: ${head.cellPadding}`).not.toBe("0px");
expect(head.cellPadding).toMatch(/^[1-9]/);

// DEV-2581: the demo's own override, applied at runtime before the head was
// re-created, still wins. `4px` here means the re-created theme stylesheet landed
// after it and took the cascade — the state measured on prod for 6z5k1q2bd4,
// where the demo's blue/white palette was replaced by the theme's defaults.
expect(head.themeVar, "the demo's own theme override survives the re-created head").toBe("11px");
});

test("what the bundler already handles is not touched, and the workspace stays clean", async ({ page }) => {
Expand All @@ -142,6 +191,7 @@ test.describe("head assets reach the live preview", () => {
await previewReady(page);
await expect(grid(page)).toBeVisible({ timeout: 120_000 });

await headSettled(page);
const head = await readHead(page);

// A local stylesheet already applies today — the bundler resolves the local URL
Expand Down
18 changes: 16 additions & 2 deletions runner/packages/runtime/src/head-assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,14 @@ const RECEIVER_HEAD = `(function (assets) {
if (typeof document === 'undefined' || !assets || !assets.length) { return; }
var MARK = '${MARK_ATTRIBUTE}';
var head = document.head || document.getElementsByTagName('head')[0] || document.documentElement;
/* Everything goes in at the head's start, before whatever is already there
(DEV-2581). This line runs at the end of the demo's module, so by now the demo
has appended its own style, and a demo overrides the theme on the same selector
the theme itself uses: equal specificity, document order decides. Appending put
the theme after the override and erased it. One anchor, captured once, so the
assets keep their authored order among themselves; insertBefore(node, null)
appends, which is what an empty head wants. */
var anchor = head.firstChild;

/* The demo has already built its grid against an unstyled DOM by the time this
runs, and a cross-origin stylesheet lands later still. A generic bubbling
Expand Down Expand Up @@ -228,7 +236,7 @@ const RECEIVER_HEAD = `(function (assets) {
style.setAttribute(MARK, '');
if (asset.media) { style.setAttribute('media', asset.media); }
style.appendChild(document.createTextNode(asset.css));
head.appendChild(style);
head.insertBefore(style, anchor);
continue;
}
var element = document.createElement(asset.tag);
Expand All @@ -243,7 +251,7 @@ const RECEIVER_HEAD = `(function (assets) {
element.onerror = nudge;
}
element.setAttribute(MARK, '');
head.appendChild(element);
head.insertBefore(element, anchor);
}
nudge();
})(`;
Expand Down Expand Up @@ -296,6 +304,12 @@ export const HEAD_ASSETS_LINE_SUFFIX = `${jsonInner(RECEIVER_TAIL)}")}catch(e){}
* Prepending would not buy the ordering it looks like it buys, either: a dynamically
* inserted cross-origin `<link>` never blocks script execution, so the stylesheet
* lands after the demo's module body either way. That is what `nudge` is for.
*
* Two different axes, easy to conflate: *this* decision is where the injected line
* sits in the JS entry, and it stays last. Where the DOM nodes go is a separate
* question with the opposite answer — they are inserted at the head's start, because
* the demo's own `<style>` is already there by then and would otherwise win the
* cascade (DEV-2581).
*/
export function headAssetsModuleLine(assets: HeadAsset[]): string {
return HEAD_ASSETS_LINE_PREFIX + jsonInner(JSON.stringify(assets)) + HEAD_ASSETS_LINE_SUFFIX;
Expand Down
65 changes: 64 additions & 1 deletion runner/pipeline/head-assets.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,19 @@ test("injecting twice is a no-op, even when the head changed underneath", () =>

/** A fake document, enough for the payload to build nodes against. */
function fakeDocument() {
const head = { children: [], appendChild(node) { this.children.push(node); } };
const head = {
children: [],
appendChild(node) { this.children.push(node); return node; },
// Real insertion semantics: the whole point of the DEV-2581 cases is *where* a
// node lands relative to what the demo's own module already put here.
insertBefore(node, anchor) {
const at = anchor === null || anchor === undefined ? -1 : this.children.indexOf(anchor);
if (at === -1) this.children.push(node);
else this.children.splice(at, 0, node);
return node;
},
get firstChild() { return this.children.length > 0 ? this.children[0] : null; },
};
const created = [];
const doc = {
title: "Sandbox - CodeSandbox",
Expand Down Expand Up @@ -605,3 +617,54 @@ test("a title's character references are decoded too", () => {
const { doc } = runPayload("<head><title>Sales &amp; Ops &#8212; Q3</title></head>");
assert.equal(doc.title, "Sales & Ops — Q3");
});

// ------------------------------------- cascade order (DEV-2581, regression on prod)

test("re-created assets land before styles the demo's own module already added", () => {
// The demo's module body runs first and appends its own <style>; our line runs at
// the end of that module. Appending our nodes therefore put the authored theme
// stylesheet *after* the demo's overrides, and a demo overrides the theme on the
// same selector the theme itself uses — equal specificity, so document order is
// the only tiebreak, and the theme won. Measured on prod for 6z5k1q2bd4:
// --ht-accent-color resolved to the theme's #1a42e8 instead of the demo's #2f8fe0.
//
// `/d` serves the authored HTML, where the head links parse before the module ever
// runs. Reproducing that means inserting at the head's start, not the end.
const demoStyle = { tagName: "STYLE", attrs: {}, children: [], textContent: ".ht-theme-main { --ht-accent-color: #2f8fe0 }" };
const { appended } = runPayload(FIXTURE, [demoStyle]);

const ours = appended.filter((node) => node.attrs["data-hot-runner-head"] === "");
const demoAt = appended.indexOf(demoStyle);
assert.equal(demoAt, ours.length, "every re-created node sits before the demo's own style");
assert.ok(appended.slice(0, ours.length).every((node) => node.attrs["data-hot-runner-head"] === ""));
});

test("inserting at the start keeps the authored order among the assets themselves", () => {
// Inserting each node before the same anchor in source order is what preserves it;
// inserting each before the *previous* one would silently reverse the cascade
// between two stylesheets that set the same variable.
const demoStyle = { tagName: "STYLE", attrs: {}, children: [], textContent: "/* demo */" };
const { appended } = runPayload(FIXTURE, [demoStyle]);
const ours = appended.filter((node) => node.attrs["data-hot-runner-head"] === "");
assert.deepEqual(
ours.map((node) => node.attrs.href ?? node.attrs.name ?? node.textContent),
[
CDN_CORE,
CDN_THEME,
":root { --e2e-head-sentinel: 7px }",
"viewport",
CDN_ICONS,
"data:text/css,%3Aroot%7B--e2e-data-sentinel%3A%209px%7D",
],
);
});

test("an empty head still receives every asset, in order", () => {
// The anchor is null in this case, and `insertBefore(node, null)` appends — the
// shape every other case in this file exercises, kept explicit so a change to the
// anchor logic cannot quietly break the common path.
const { appended } = runPayload(FIXTURE);
assert.equal(appended.length, 6);
assert.equal(appended[0].attrs.href, CDN_CORE);
assert.equal(appended.at(-1).attrs.href, "data:text/css,%3Aroot%7B--e2e-data-sentinel%3A%209px%7D");
});
Loading