diff --git a/runner/e2e/preview-head-assets.spec.ts b/runner/e2e/preview-head-assets.spec.ts index 94f55693..310b5b53 100644 --- a/runner/e2e/preview-head-assets.spec.ts +++ b/runner/e2e/preview-head-assets.spec.ts @@ -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, @@ -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 ``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
renders themed", async ({ page }) => { await page.route("**/api/versions", (route) => @@ -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 @@ -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 }) => { @@ -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 diff --git a/runner/packages/runtime/src/head-assets.ts b/runner/packages/runtime/src/head-assets.ts index a3bde191..93ff53da 100644 --- a/runner/packages/runtime/src/head-assets.ts +++ b/runner/packages/runtime/src/head-assets.ts @@ -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 @@ -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); @@ -243,7 +251,7 @@ const RECEIVER_HEAD = `(function (assets) { element.onerror = nudge; } element.setAttribute(MARK, ''); - head.appendChild(element); + head.insertBefore(element, anchor); } nudge(); })(`; @@ -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 `` 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 `