fix(runner): insert re-created head assets before the demo's own styles (DEV-2581) - #244
Conversation
…es (DEV-2581) Follow-up to DEV-2576, found on prod. The re-created nodes were appended to document.head, but the injected line runs at the end of the demo's module — by then the demo has already appended its own <style>. A demo overrides the theme on the same selector the theme itself uses, so specificity is equal and document order decides, and appending put the theme stylesheet last: the demo's palette was erased. Measured on 6z5k1q2bd4 at /share, against /d as the control: --ht-accent-color #1a42e8 vs #2f8fe0 --ht-foreground-color #222 vs #123653 header background #f7f7f9 vs #eaf4ff Before DEV-2576 the demo had no theme; after it, the wrong one. Everything now goes in at the head's start, against one anchor captured before the loop so the assets keep their authored order among themselves. That reproduces the document order /d has and already proves correct: the head's assets first, then the demo's runtime <style>, then Handsontable's auto-injected core CSS. Note the two axes, which are easy to conflate. Where the injected *line* sits in the JS entry is unchanged — still last, so compile positions and babel code frames are untouched (DEV-2557). Where the *DOM nodes* go is the opposite answer, and this is that fix. The test gap that let it through: every unit case seeded an empty head, so nothing measured our nodes against content the demo adds at runtime. The fake document now implements insertBefore/firstChild, three unit cases cover position, relative order and the empty-head path, and the live spec's demo now applies a --ht-* override at runtime that must survive. Also fixed a flake the new assertion exposed: the spec read the head as soon as the grid was visible, but the stylesheets are cross-origin and land later (one run in three saw links=0). It now polls until the theme has actually applied — non-zero cell padding, not a non-empty variable, because the demo's own override makes that variable non-empty synchronously while the sheet is still in flight. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… (DEV-2581) Review of #244: the spec inherited playwright.config.ts's 60s per-test default while the waits inside one test already allow more — previewReady 120s, the grid 120s, headSettled 30s. On a cold bundler the test would abort with Playwright's generic "Test timeout of 60000ms exceeded" before any of those could report what they measured, so the least informative failure is the one a reviewer sees. 300s, matching preview-scheme.spec.ts, which is the closest sibling and already does this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Review finding addressed.
Worth noting the squeeze predates the poll — Re-verified: Thanks for running both A/B arms independently, including the sequential-anchor variant — that second arm (5 tests red) is the one I hadn't proved myself, and it's the exact mistake that would have silently reversed two stylesheets setting the same variable. |
1 participant
Fixes DEV-2581, a follow-up to DEV-2576 (#240) found on prod.
Problem
#240 re-creates the authored
<head>from the module entry, but it appended the nodes. The injected line runs at the end of the demo's module, so by then the demo has already appended its own<style>— and a demo overrides the theme on the same selector the theme itself uses:Equal specificity, so document order is the only tiebreak, and appending put the theme stylesheet last. Measured on
6z5k1q2bd4,/shareagainst/das the control:/d(correct)/share(before this PR)<style>, auto-injected core<style>, auto-core, link core (ours), link theme (ours)--ht-accent-color#2f8fe0#1a42e8--ht-foreground-color#123653#222rgb(234,244,255)rgb(247,247,249)Before #240 the demo had no theme; after it, the wrong one. Strictly better, still not what the demo declares.
Fix
Insert at the head's start, against one anchor captured before the loop so the assets keep their authored order among themselves.
insertBefore(node, null)appends, which is what an empty head wants, so the common path is unchanged. This reproduces the document order/dhas and already proves correct: head assets first, the demo's runtime<style>after them, Handsontable's auto-injected core CSS last.Two axes worth separating, because #240's own comments discuss the other one: where the injected line sits in the JS entry is unchanged — still last, so compile positions and babel code frames are untouched (DEV-2557). Where the DOM nodes go is the opposite answer, and that is this change.
Test gap this exposes
Every #240 unit case seeded an empty head, so nothing measured our nodes against content the demo adds at runtime — which is exactly the shape that broke.
insertBefore/firstChildinstead of only appending.<style>; their relative order among themselves is preserved (inserting each before the previous one would silently reverse two stylesheets that set the same variable); the empty-head path still receives everything in order.e2e/preview-head-assets.spec.ts: the payload demo now applies.ht-theme-main { --ht-cell-vertical-padding: 11px }at runtime, and the spec asserts it survives the re-created head.A/B, both arms measured. Reverting only the insertion point (
insertBefore→appendChild) fails on the new assertion withExpected: "11px"/Received: "4px"— the theme overriding the demo, i.e. the prod symptom. Restored: 6 passed across three repeats.One flake fixed along the way
The first A/B run failed on padding rather than on the cascade assertion, which exposed a weak readiness check: the spec read the head as soon as the grid was visible, but the stylesheets are cross-origin and land later (one run in three saw
links=0). It now polls until the theme has actually applied — non-zero cell padding, not a non-empty--ht-cell-vertical-padding, because the demo's own override makes that variable non-empty synchronously while the sheet is still in flight. A readiness signal that can be satisfied by the thing under test is worse than none.Verification
After deploy,
6z5k1q2bd4should read--ht-accent-color: #2f8fe0and header backgroundrgb(234,244,255)on/shareand/edit, matching/d. It stays version-skewed regardless — CSS from jsDelivrlatestover JS from PR 13201 — because of DEV-2578.🤖 Generated with Claude Code
Note
Cursor Bugbot is generating a summary for commit 27565d2. Configure here.