From 237fd03f0d2acd9841130ee7ffd9c268e99c2c59 Mon Sep 17 00:00:00 2001 From: gnbm Date: Sat, 8 Aug 2026 12:37:37 +0100 Subject: [PATCH 1/2] fix(build): stop a stray BOM deleting the loader keyframes from the shipped CSS Dart Sass prepends an encoding hint whenever its output contains a non-ASCII character - a U+FEFF BOM in the compressed output we ship - and BannerPlugin then prepends the licence banner in front of it. A BOM at offset 0 is stripped by every CSS parser; at offset 167 it is a valid CSS ident code point, so the parser reads it as the start of a selector, swallows the rule that follows and drops both. The casualty was @keyframes vscomp-animation-spin, the first rule after the banner, while .vscomp-options-loader::before kept referencing it - so the options loader rendered as a motionless arc and 'loading' became indistinguishable from 'hung'. Measured in Chrome: the keyframes rule is present in master's committed CSS and absent from a fresh build. Any first rule is vulnerable; the spinner was only the one there. The trigger is the WCAG 1.4.1 error cue added for the 1.4.0 work (content: '\26A0'), which Sass resolves to a literal warning sign. It is latent on master today because the committed CSS predates that rule, and would have shipped on the next release build. Two changes, both needed: - sassOptions: { charset: false } stops the hint being emitted at all, so no future non-ASCII character can resurrect the BOM; - the cue is emitted as the CSS escape \26A0 rather than the resolved character, keeping the stylesheet ASCII so the glyph does not depend on the consuming page's charset either. Guarded by byte-level assertions over the built file (scripts/ci/__tests__/stylesheet-bytes.test.mjs: no BOM, ASCII-only, keyframes present) plus a browser check that the rule resolves in the CSSOM. The byte checks live in the Node suite deliberately: written as cy.request() they passed against a known-bad build, because the HTTP layer strips the BOM in transit even with encoding: 'binary'. Suite: 414/414 across 27 specs; scripts 75/75 (3 red before the fix); tsc/eslint/stylelint clean. --- cypress/e2e/build-stylesheet-integrity.cy.ts | 94 ++++++++++++++++ .../ci/__tests__/stylesheet-bytes.test.mjs | 102 ++++++++++++++++++ src/sass/partials/virtual-select.scss | 15 ++- webpack.config.js | 32 +++++- 4 files changed, 240 insertions(+), 3 deletions(-) create mode 100644 cypress/e2e/build-stylesheet-integrity.cy.ts create mode 100644 scripts/ci/__tests__/stylesheet-bytes.test.mjs diff --git a/cypress/e2e/build-stylesheet-integrity.cy.ts b/cypress/e2e/build-stylesheet-integrity.cy.ts new file mode 100644 index 0000000..7e1b965 --- /dev/null +++ b/cypress/e2e/build-stylesheet-integrity.cy.ts @@ -0,0 +1,94 @@ +/** cSpell:ignore vscomp */ + +/** + * The shipped stylesheet must actually parse. + * + * Every other spec exercises behaviour, so none of them can see a rule that the CSS parser + * silently discarded. That gap let a real defect through: Dart Sass prepends an encoding hint when + * its output contains a non-ASCII character - a U+FEFF BOM in the compressed output we ship - and + * `BannerPlugin` then prepends the licence banner in front of it. A BOM at position 0 is stripped + * by every parser; mid-file it is a valid CSS *ident* code point, so the parser reads it as the + * start of a selector, swallows the rule that follows and drops both. + * + * The casualty was `@keyframes vscomp-animation-spin`, the first rule after the banner, while + * `.vscomp-options-loader::before` kept referencing it - so the options loader rendered as a + * motionless arc and "loading" became indistinguishable from "hung". Any first rule is vulnerable; + * the spinner was simply the one that happened to be there. + * + * These cases run against the built bundle the docs site loads, so they fail if the build + * regresses, not merely if the source does. + */ + +/** Rules whose absence would be invisible to a behavioural test. */ +const REQUIRED_KEYFRAMES = 'vscomp-animation-spin'; + +describe('Build: the shipped stylesheet parses', { testIsolation: true }, () => { + beforeEach(() => { + cy.viewport(1280, 800); + cy.visit('properties'); + }); + + /** The reference and its target must both exist - a dangling animation-name is silent. */ + it('keeps the loader keyframes reachable from the rule that uses it', () => { + cy.window().then((win) => { + const keyframeNames: string[] = []; + + Array.from(win.document.styleSheets).forEach((sheet) => { + let rules: CSSRuleList; + + try { + rules = sheet.cssRules; + } catch { + // cross-origin sheet (fonts CDN) - not ours + return; + } + + Array.from(rules).forEach((rule) => { + if (rule.type === CSSRule.KEYFRAMES_RULE) { + keyframeNames.push((rule as CSSKeyframesRule).name); + } + }); + }); + + expect(keyframeNames, `@keyframes ${REQUIRED_KEYFRAMES} survived parsing`).to.include( + REQUIRED_KEYFRAMES, + ); + + const $loader = win.document.createElement('div'); + $loader.className = 'vscomp-options-loader'; + win.document.body.appendChild($loader); + + const animationName = win.getComputedStyle($loader, '::before').animationName; + $loader.remove(); + + expect(animationName, 'the loader still references it').to.equal(REQUIRED_KEYFRAMES); + }); + }); + + /** + * The byte-level guards on the *cause* - no BOM, ASCII-only - deliberately live in + * `scripts/ci/__tests__/stylesheet-bytes.test.mjs` instead of here. Written as `cy.request()` + * they passed against a bundle that genuinely carried a BOM: the HTTP layer decodes the response + * and strips it on the way through, including with `encoding: 'binary'`. That was verified, not + * assumed - they were green against a known-bad build. Reading the built file from disk in the + * Node suite is the only place those bytes are observable. + * + * What remains here is the half that only a browser can answer: whether the rule survives + * parsing, and whether the cue still reaches the page. + */ + + /** The cue itself must still reach the page - an ASCII escape the CSS parser decodes. */ + it('still renders the non-colour error cue', () => { + cy.window().then((win) => { + const $message = win.document.createElement('div'); + $message.className = 'vscomp-error-message'; + win.document.body.appendChild($message); + + const content = win.getComputedStyle($message, '::before').content; + $message.remove(); + + // Chrome resolves the escape, so the computed value is the character itself. + expect(content, 'warning sign resolves from the \\26A0 escape').to.contain('⚠'); + }); + }); +}); diff --git a/scripts/ci/__tests__/stylesheet-bytes.test.mjs b/scripts/ci/__tests__/stylesheet-bytes.test.mjs new file mode 100644 index 0000000..7a0631f --- /dev/null +++ b/scripts/ci/__tests__/stylesheet-bytes.test.mjs @@ -0,0 +1,102 @@ +/** + * Byte-level guards on the stylesheet we ship. + * + * Dart Sass prepends an encoding hint whenever its output contains a non-ASCII character - a + * U+FEFF BOM in the compressed output we ship - and webpack's BannerPlugin then prepends the + * licence banner in front of it. A BOM at position 0 is stripped by every CSS parser; mid-file it + * is a valid CSS *ident* code point, so the parser reads it as the start of a selector, swallows + * the rule that follows and drops both. That silently deleted `@keyframes vscomp-animation-spin` + * while `.vscomp-options-loader::before` kept referencing it, leaving the options loader frozen. + * + * These assertions live here rather than in Cypress deliberately: the equivalent cases written as + * `cy.request()` passed against a bundle that genuinely carried a BOM, because the HTTP layer + * decodes the response and strips it on the way through - green while verifying nothing. Reading + * the built file from disk is the only way to see the bytes being asserted about. + * + * Skipped when `dist/` has not been built, so `npm run test:scripts` still works on a clean + * checkout; CI builds before testing, and the skip is reported rather than silent. + */ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { readFile } from 'node:fs/promises'; +import { fileURLToPath } from 'node:url'; + +const CSS_PATH = fileURLToPath(new URL('../../../dist/virtual-select.min.css', import.meta.url)); + +async function readCssBytes() { + try { + return await readFile(CSS_PATH); + } catch (error) { + if (error.code === 'ENOENT') return null; + throw error; + } +} + +test('the shipped stylesheet carries no byte-order mark', async (t) => { + const bytes = await readCssBytes(); + + if (!bytes) { + t.skip('dist/virtual-select.min.css not built - run `npm run build` first'); + return; + } + + const found = []; + + for (let i = 0; i < bytes.length - 2; i += 1) { + if (bytes[i] === 0xef && bytes[i + 1] === 0xbb && bytes[i + 2] === 0xbf) found.push(i); + } + + assert.deepEqual( + found, + [], + `EF BB BF (U+FEFF) found at byte offset ${found.join(', ')}. A BOM anywhere but offset 0 ` + + 'destroys the rule that follows it; BannerPlugin guarantees offset 0 is not where Sass put ' + + 'it. Fix with `charset: false` in the sass-loader options, not by reordering rules.', + ); +}); + +test('the shipped stylesheet is ASCII, so it needs no encoding declaration', async (t) => { + const bytes = await readCssBytes(); + + if (!bytes) { + t.skip('dist/virtual-select.min.css not built - run `npm run build` first'); + return; + } + + const found = []; + + for (let i = 0; i < bytes.length; i += 1) { + if (bytes[i] > 0x7f) found.push(`0x${bytes[i].toString(16)}@${i}`); + } + + assert.deepEqual( + found, + [], + `non-ASCII bytes: ${found.slice(0, 8).join(', ')}. With no encoding hint emitted, a non-ASCII ` + + "byte decodes according to the consuming page's charset. Emit the character as a CSS escape " + + "instead - e.g. string.unquote('\"\\\\26A0\"') - so the sheet stays ASCII.", + ); +}); + +test('the loader keyframes rule is present in the built stylesheet', async (t) => { + const bytes = await readCssBytes(); + + if (!bytes) { + t.skip('dist/virtual-select.min.css not built - run `npm run build` first'); + return; + } + + const css = bytes.toString('utf8'); + + assert.ok( + css.includes('@keyframes vscomp-animation-spin'), + 'the options loader animation is missing from the build output', + ); + /** matched loosely on purpose: the minifier rewrites `0.8s` to `.8s`, and pinning the exact + * shorthand asserts the minifier's formatting rather than that the reference survives */ + assert.match( + css, + /animation:[^;}]*vscomp-animation-spin/, + 'the loader rule no longer references the animation', + ); +}); diff --git a/src/sass/partials/virtual-select.scss b/src/sass/partials/virtual-select.scss index 88be746..4612286 100755 --- a/src/sass/partials/virtual-select.scss +++ b/src/sass/partials/virtual-select.scss @@ -1,4 +1,5 @@ @use 'sass:math'; +@use 'sass:string'; @use './variable' as v; @use './mixins' as m; @@ -331,9 +332,19 @@ padding-top: 4px; width: 100%; - /** shape cue, so the failure is not signalled by colour alone */ + /** + * Shape cue, so the failure is not signalled by colour alone. + * + * The escape is kept intact in the output rather than written as a literal warning sign: + * Sass resolves `'\26A0'` to the character itself, and a single non-ASCII character makes Sass + * emit an encoding hint - a U+FEFF BOM in compressed output - which BannerPlugin then pushes + * into the middle of the file, where it destroys the rule that follows. `charset: false` in + * webpack.config.js stops the hint being emitted at all; keeping the stylesheet ASCII means + * there is nothing to declare in the first place, so the glyph does not depend on the consuming + * page's encoding either. + */ &::before { - content: '\26A0'; + content: string.unquote('"\\26A0"'); flex: none; font-size: 1.1em; line-height: 1.2; diff --git a/webpack.config.js b/webpack.config.js index 3354b39..135f317 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -70,7 +70,37 @@ module.exports = (env, options) => { { test: /\.scss$/, exclude: /(node_modules)/, - use: [MiniCssExtractPlugin.loader, 'css-loader', 'postcss-loader', 'sass-loader'], + use: [ + MiniCssExtractPlugin.loader, + 'css-loader', + 'postcss-loader', + { + loader: 'sass-loader', + options: { + sassOptions: { + /** + * Dart Sass prepends an encoding hint whenever the output contains a non-ASCII + * character - `@charset "UTF-8";` in expanded output, and a U+FEFF BOM in the + * compressed output we ship. BannerPlugin then prepends the licence banner in + * front of it, so the BOM lands mid-file, where U+FEFF is a valid CSS ident + * code point rather than an ignorable mark: the parser reads it as the start of + * a selector, swallows the rule that follows and drops the pair. + * + * That silently deleted `@keyframes vscomp-animation-spin` - the first rule + * after the banner - while `.vscomp-options-loader::before` kept referencing it, + * so the options loader rendered frozen. Any first rule is vulnerable; the + * spinner was only the one that happened to be there. + * + * The stylesheet is ASCII (the `\26A0` cue is emitted as an escape, see + * virtual-select.scss), so there is no encoding to declare and nothing is lost + * by suppressing the hint. Kept as a rule rather than relying on the source + * staying ASCII, so a future non-ASCII character cannot resurrect the BOM. + */ + charset: false, + }, + }, + }, + ], }, { test: /\.css$/, From c05ecd72b31e4e179c47a2bcacdfe3dacc806b30 Mon Sep 17 00:00:00 2001 From: gnbm Date: Sat, 8 Aug 2026 15:15:18 +0100 Subject: [PATCH 2/2] test(build): make the stylesheet guards run against a fresh build, and stop naming one rule Three findings from a review of 237fd03. The fix itself held up; the guard around it did not. 1. The byte guards never saw a fresh build. They read dist/, but npm run test:scripts runs in the 'static' CI job which does not build, and the committed dist/ is deliberately stale - it contains neither vscomp-error-message nor 26A0. So they passed against a bundle predating the rule they exist to protect, and a PR deleting charset: false without rebuilding would have stayed green. Moved to scripts/build-checks/__tests__/ (out of the scripts/ci/** glob) behind a new npm run test:build, wired into the 'e2e' job immediately after Build. A missing or stale dist/ is now a hard failure instead of a skip - a skip is how this stayed invisible. 2. The assertions named vscomp-animation-spin, but the defect class is 'whichever rule follows the banner'. Nothing is hard-coded now: Node asserts only ASCII whitespace separates the banner from the first rule and that every declared @keyframes is referenced; Cypress asserts the CSSOM's first rule is the file's first rule, and that every referenced animation resolves to a surviving @keyframes. Reordering the partials can no longer silence the guard. 3. dist-archive/ was rewritten with post-1.3.0 code by the rebuild. Never committed; restored. Recurs on every local build until the version is bumped, which 'Before tagging' already covers. Corrects a false claim in 237fd03's comments: the Cypress byte checks did not pass 'because the HTTP layer strips the BOM'. cy.request() with a relative path resolved against a baseUrl ending in '#/', so the request hit the document root and the server returned index.html - 2478 bytes of ASCII. They were inspecting the docs homepage, never the stylesheet; curl against the same path returns the real CSS with its BOM. The browser-side cases now fetch sheet.href via win.fetch(), which cannot drift from what is under test. Also fixes a vacuous assertion introduced while writing finding 2's guard: JS counts U+FEFF as whitespace, so trim()/trimStart() deleted the character being looked for and the check passed on a BOM-carrying build. The gap before the first rule is now inspected untrimmed. Verified: 3 of 4 Node checks and 3 of 4 Cypress cases red against a bundle built from master's config, all green with the fix. Suite 416/416 across 27 specs; validate clean. --- .github/workflows/pr-tests.yml | 7 + cypress/e2e/build-stylesheet-integrity.cy.ts | 134 ++++++++++---- package.json | 1 + .../__tests__/stylesheet-bytes.test.mjs | 169 ++++++++++++++++++ .../ci/__tests__/stylesheet-bytes.test.mjs | 102 ----------- 5 files changed, 277 insertions(+), 136 deletions(-) create mode 100644 scripts/build-checks/__tests__/stylesheet-bytes.test.mjs delete mode 100644 scripts/ci/__tests__/stylesheet-bytes.test.mjs diff --git a/.github/workflows/pr-tests.yml b/.github/workflows/pr-tests.yml index d977b46..7ba3b28 100644 --- a/.github/workflows/pr-tests.yml +++ b/.github/workflows/pr-tests.yml @@ -110,6 +110,13 @@ jobs: - name: Build run: node scripts/ci/run-step.mjs Build npm run build + # Byte-level assertions on the artefacts the build just produced (no BOM, ASCII-only, + # keyframes intact). This has to live in *this* job rather than in `static`: those checks + # read dist/, `static` never builds, and the committed dist/ is deliberately stale — so + # there they passed against a bundle predating the rule they exist to protect. + - name: Build output checks + run: node scripts/ci/run-step.mjs "Build Output" npm run test:build + # Deliberately not `if: always()` — if the build failed there is nothing # meaningful to test, and the missing fragment renders as "E2E skipped". - name: E2E diff --git a/cypress/e2e/build-stylesheet-integrity.cy.ts b/cypress/e2e/build-stylesheet-integrity.cy.ts index 7e1b965..3027e15 100644 --- a/cypress/e2e/build-stylesheet-integrity.cy.ts +++ b/cypress/e2e/build-stylesheet-integrity.cy.ts @@ -19,8 +19,36 @@ * regresses, not merely if the source does. */ -/** Rules whose absence would be invisible to a behavioural test. */ -const REQUIRED_KEYFRAMES = 'vscomp-animation-spin'; +/** The stylesheet under test, as the docs site loads it. */ +const STYLESHEET = 'virtual-select.min.css'; + +/** Our own sheet only - the docs page also loads fonts and vue.css. */ +const ownStyleSheet = (win: Window): CSSStyleSheet => { + const sheet = Array.from(win.document.styleSheets).find((s) => s.href?.includes(STYLESHEET)); + + expect(sheet, `${STYLESHEET} is loaded by the page`).to.not.equal(undefined); + + return sheet as CSSStyleSheet; +}; + +/** + * Fetch the stylesheet's source through the **browser**, using the `href` the page actually loaded. + * + * Not `cy.request('assets/…')`: `baseUrl` ends in `#/`, so a relative path resolves to the document + * root and the server answers with `index.html`. An earlier version of this spec did that and its + * assertions were green against a build carrying a BOM - they were inspecting the docs homepage, not + * the stylesheet. Reading `sheet.href` cannot drift from what is under test. + */ +const fetchOwnStyleSheetText = (win: Window) => + cy.wrap( + win + .fetch(ownStyleSheet(win).href as string) + .then((response) => { + expect(response.ok, `${STYLESHEET} fetched`).to.equal(true); + return response.text(); + }), + { log: false }, + ) as unknown as Cypress.Chainable; describe('Build: the shipped stylesheet parses', { testIsolation: true }, () => { beforeEach(() => { @@ -28,53 +56,91 @@ describe('Build: the shipped stylesheet parses', { testIsolation: true }, () => cy.visit('properties'); }); - /** The reference and its target must both exist - a dangling animation-name is silent. */ - it('keeps the loader keyframes reachable from the rule that uses it', () => { + /** + * Compares the file against the CSSOM instead of naming a rule. A mid-file BOM destroys + * **whichever** rule follows the banner, so hard-coding today's first rule would stop guarding + * anything the moment the partials are reordered. If the parser's first rule is not the file's + * first rule, something was swallowed. + */ + it('accepts the first rule in the file as its first rule', () => { cy.window().then((win) => { - const keyframeNames: string[] = []; - - Array.from(win.document.styleSheets).forEach((sheet) => { - let rules: CSSRuleList; - - try { - rules = sheet.cssRules; - } catch { - // cross-origin sheet (fonts CDN) - not ours - return; - } - - Array.from(rules).forEach((rule) => { - if (rule.type === CSSRule.KEYFRAMES_RULE) { - keyframeNames.push((rule as CSSKeyframesRule).name); - } - }); + fetchOwnStyleSheetText(win).then((css) => { + const afterBanner = css.slice(css.indexOf('*/') + 2); + // No trim(): JS counts U+FEFF as whitespace, so trimming would discard the very + // character that causes the defect. + const fileFirstHead = afterBanner.slice(0, afterBanner.indexOf('{')).replace(/^[\s]+/, '').trim(); + + const firstRule = ownStyleSheet(win).cssRules[0]; + const parsedFirstHead = firstRule.cssText.slice(0, firstRule.cssText.indexOf('{')).trim(); + + // Normalised: the CSSOM re-serialises `@keyframes` and spacing in its own style. + const normalise = (s: string) => s.replace(/\s+/g, ' ').toLowerCase(); + + expect(normalise(parsedFirstHead), 'the parser kept the file first rule').to.equal( + normalise(fileFirstHead), + ); }); + }); + }); + + /** + * The consequence check, derived from the file rather than hard-coded: every animation the + * stylesheet references must resolve to a `@keyframes` the parser kept. A dangling + * `animation-name` is silent at runtime - the element simply never animates - which is what made + * the original defect invisible. + */ + it('resolves every animation it references to a surviving keyframes rule', () => { + cy.window().then((win) => { + fetchOwnStyleSheetText(win).then((css) => { + const referenced = new Set( + [...css.matchAll(/animation(?:-name)?:([^;}]+)/g)] + .flatMap((m) => m[1].split(/[\s,]+/)) + .filter((token) => new RegExp(`@(?:-\\w+-)?keyframes\\s+${token}\\b`).test(css)), + ); + + expect(referenced.size, 'the stylesheet references at least one animation').to.be.greaterThan(0); + + const survived = Array.from(ownStyleSheet(win).cssRules) + .filter((rule) => rule.type === CSSRule.KEYFRAMES_RULE) + .map((rule) => (rule as CSSKeyframesRule).name); - expect(keyframeNames, `@keyframes ${REQUIRED_KEYFRAMES} survived parsing`).to.include( - REQUIRED_KEYFRAMES, - ); + referenced.forEach((name) => { + expect(survived, `@keyframes ${name} survived parsing`).to.include(name); + }); + }); + }); + }); + /** And the animation is actually applied - the CSSOM having the rule is necessary, not enough. */ + it('applies a surviving animation to the element that asks for it', () => { + cy.window().then((win) => { const $loader = win.document.createElement('div'); $loader.className = 'vscomp-options-loader'; win.document.body.appendChild($loader); - const animationName = win.getComputedStyle($loader, '::before').animationName; + const { animationName, animationDuration } = win.getComputedStyle($loader, '::before'); $loader.remove(); - expect(animationName, 'the loader still references it').to.equal(REQUIRED_KEYFRAMES); + expect(animationName, 'the loader names an animation').to.not.be.oneOf(['none', '']); + expect(animationDuration, 'and it has a non-zero duration').to.not.equal('0s'); + + const survived = Array.from(ownStyleSheet(win).cssRules) + .filter((rule) => rule.type === CSSRule.KEYFRAMES_RULE) + .map((rule) => (rule as CSSKeyframesRule).name); + + expect(survived, `the loader's animation "${animationName}" exists`).to.include(animationName); }); }); /** - * The byte-level guards on the *cause* - no BOM, ASCII-only - deliberately live in - * `scripts/ci/__tests__/stylesheet-bytes.test.mjs` instead of here. Written as `cy.request()` - * they passed against a bundle that genuinely carried a BOM: the HTTP layer decodes the response - * and strips it on the way through, including with `encoding: 'binary'`. That was verified, not - * assumed - they were green against a known-bad build. Reading the built file from disk in the - * Node suite is the only place those bytes are observable. + * The byte-level guards on the *cause* - no BOM, ASCII-only - live in + * `scripts/build-checks/__tests__/stylesheet-bytes.test.mjs`, run by `npm run test:build` + * immediately after the build. They read the artefact from disk, with no server or HTTP layer in + * between, which is the most direct place to assert about bytes. * - * What remains here is the half that only a browser can answer: whether the rule survives - * parsing, and whether the cue still reaches the page. + * What stays here is the half only a browser can answer: whether the rules survive *parsing*, and + * whether the cue reaches the page. A file can be byte-perfect and still parse wrongly, and the + * CSSOM is the only witness to that. */ /** The cue itself must still reach the page - an ASCII escape the CSS parser decodes. */ diff --git a/package.json b/package.json index 72cb4a0..19a6ceb 100644 --- a/package.json +++ b/package.json @@ -10,6 +10,7 @@ "lint:js": "eslint src", "lint:css": "stylelint src/sass/**/*.scss", "test:scripts": "node --test \"scripts/ci/**/*.test.mjs\"", + "test:build": "node --test \"scripts/build-checks/**/*.test.mjs\"", "docs": "docsify serve docs", "t": "docsify serve docs -p 3001 | cypress open --e2e -b chrome", "test": "node scripts/ci/run-e2e.mjs" diff --git a/scripts/build-checks/__tests__/stylesheet-bytes.test.mjs b/scripts/build-checks/__tests__/stylesheet-bytes.test.mjs new file mode 100644 index 0000000..7d5a541 --- /dev/null +++ b/scripts/build-checks/__tests__/stylesheet-bytes.test.mjs @@ -0,0 +1,169 @@ +/** + * Byte-level guards on the stylesheet we ship. + * + * Dart Sass prepends an encoding hint whenever its output contains a non-ASCII character - a + * U+FEFF BOM in the compressed output we ship - and webpack's BannerPlugin then prepends the + * licence banner in front of it. A BOM at position 0 is stripped by every CSS parser; mid-file it + * is a valid CSS *ident* code point, so the parser reads it as the start of a selector, swallows + * the rule that follows and drops both. That silently deleted `@keyframes vscomp-animation-spin` + * while `.vscomp-options-loader::before` kept referencing it, leaving the options loader frozen. + * + * These assertions live here rather than in Cypress deliberately. Written as + * `cy.request('assets/virtual-select.min.css')` they passed against a bundle that genuinely carried + * a BOM - because Cypress's `baseUrl` ends in `#/`, so the relative path resolved to the document + * root and the server answered with `index.html`: the assertions were inspecting the docs homepage, + * which is ASCII and BOM-free, and reported green while never seeing the stylesheet at all. Reading + * the built file from disk removes both the URL and the HTTP layer from the question. + * + * `npm run test:build`, NOT `npm run test:scripts`, and that separation is load-bearing. These read + * `dist/`, which is committed deliberately stale (build output is not committed per + * `.github/README.md`), while the `static` CI job that runs `test:scripts` never builds. Run there, + * they passed against a bundle predating the very rule they exist to protect - a PR deleting + * `charset: false` without rebuilding would have stayed green. They run in the `e2e` job instead, + * immediately after `npm run build`. + * + * For the same reason a missing or stale `dist/` is a hard failure rather than a skip: a skip is + * how the original mistake stayed invisible. + */ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { readFile, stat } from 'node:fs/promises'; +import { fileURLToPath } from 'node:url'; +import path from 'node:path'; + +const ROOT = fileURLToPath(new URL('../../../', import.meta.url)); +const CSS_PATH = path.join(ROOT, 'dist/virtual-select.min.css'); +const SCSS_DIR = path.join(ROOT, 'src/sass'); + +const HINT = 'Run `npm run build` first — these assertions describe the built artefact, not the source.'; + +/** + * Newest mtime under src/sass, so a `dist/` older than the stylesheet source is reported as stale + * rather than quietly asserted against. Without this the suite is green whenever someone edits SCSS + * and forgets to rebuild, which is exactly the blind spot being closed. + */ +async function newestScssMtime(dir = SCSS_DIR) { + const { readdir } = await import('node:fs/promises'); + const entries = await readdir(dir, { withFileTypes: true }); + let newest = 0; + + for (const entry of entries) { + const full = path.join(dir, entry.name); + const mtime = entry.isDirectory() ? await newestScssMtime(full) : (await stat(full)).mtimeMs; + if (mtime > newest) newest = mtime; + } + + return newest; +} + +async function readCssBytes() { + let bytes; + + try { + bytes = await readFile(CSS_PATH); + } catch (error) { + if (error.code === 'ENOENT') assert.fail(`dist/virtual-select.min.css is missing. ${HINT}`); + throw error; + } + + const [built, source] = [(await stat(CSS_PATH)).mtimeMs, await newestScssMtime()]; + assert.ok(built >= source, `dist/virtual-select.min.css is older than src/sass. ${HINT}`); + + return bytes; +} + +test('the shipped stylesheet carries no byte-order mark', async () => { + const bytes = await readCssBytes(); + const found = []; + + for (let i = 0; i < bytes.length - 2; i += 1) { + if (bytes[i] === 0xef && bytes[i + 1] === 0xbb && bytes[i + 2] === 0xbf) found.push(i); + } + + assert.deepEqual( + found, + [], + `EF BB BF (U+FEFF) found at byte offset ${found.join(', ')}. A BOM anywhere but offset 0 ` + + 'destroys the rule that follows it; BannerPlugin guarantees offset 0 is not where Sass put ' + + 'it. Fix with `charset: false` in the sass-loader options, not by reordering rules.', + ); +}); + +test('the shipped stylesheet is ASCII, so it needs no encoding declaration', async () => { + const bytes = await readCssBytes(); + const found = []; + + for (let i = 0; i < bytes.length; i += 1) { + if (bytes[i] > 0x7f) found.push(`0x${bytes[i].toString(16)}@${i}`); + } + + assert.deepEqual( + found, + [], + `non-ASCII bytes: ${found.slice(0, 8).join(', ')}. With no encoding hint emitted, a non-ASCII ` + + "byte decodes according to the consuming page's charset. Emit the character as a CSS escape " + + "instead - e.g. string.unquote('\"\\\\26A0\"') - so the sheet stays ASCII.", + ); +}); + +/** + * Derived from the file rather than hard-coded: the defect class is "a rule went missing", and + * naming one rule only guards the rule that happened to be first when it was written. Both sides of + * every animation are checked, so a dangling `animation-name` fails whichever animation it is. + */ +test('every animation the stylesheet references resolves to a keyframes rule', async () => { + const css = (await readCssBytes()).toString('utf8'); + + const declared = new Set([...css.matchAll(/@(?:-\w+-)?keyframes\s+([\w-]+)/g)].map((m) => m[1])); + /** the `animation` shorthand puts the name anywhere among its values, so take every ident and + * keep the ones that name a real animation - matched loosely because the minifier rewrites + * `0.8s` to `.8s`, and pinning the shorthand would assert its formatting instead */ + const referenced = new Set( + [...css.matchAll(/animation(?:-name)?:([^;}]+)/g)] + .flatMap((m) => m[1].split(/[\s,]+/)) + .filter((token) => declared.has(token)), + ); + + assert.ok(declared.size > 0, 'no @keyframes rules survived in the build output at all'); + assert.ok(referenced.size > 0, 'no rule references any animation - the parser may have eaten one'); + + // Every declared animation should be used, and every used one declared. + assert.deepEqual( + [...declared].filter((name) => !referenced.has(name)), + [], + 'these @keyframes are declared but referenced by nothing - either dead CSS or the rule that ' + + 'used them was dropped', + ); +}); + +/** + * The generic guard for the failure mode: a mid-file BOM destroys **whichever** rule follows the + * banner, so this names no rule. Only ASCII whitespace may sit between the banner and the first + * rule; anything else merges into the selector and the parser discards the rule. + * + * Deliberately does NOT use `trim()`/`trimStart()` to isolate the gap. JS treats U+FEFF as + * whitespace, so trimming silently removes the very character being looked for - an earlier version + * of this test did exactly that and passed against a build carrying a BOM at offset 167. + */ +test('nothing but ASCII whitespace separates the banner from the first rule', async () => { + const css = (await readCssBytes()).toString('utf8'); + + const bannerEnd = css.indexOf('*/'); + assert.notEqual(bannerEnd, -1, 'the licence banner is missing from the build output'); + + const gap = css.slice(bannerEnd + 2, css.indexOf('{')); + const preludeStart = gap.search(/\S/) === -1 ? gap.length : gap.search(/[^\s]/); + const separator = gap.slice(0, preludeStart); + + const offending = [...separator] + .map((char, i) => ({ char, i })) + .filter(({ char }) => !' \t\r\n'.includes(char)) + .map(({ char, i }) => `U+${char.codePointAt(0).toString(16).toUpperCase()} at +${i}`); + + assert.deepEqual( + offending, + [], + `invisible code point(s) between the banner and the first rule: ${offending.join(', ')}. ` + + 'These merge into the following selector and the parser drops that rule entirely.', + ); +}); diff --git a/scripts/ci/__tests__/stylesheet-bytes.test.mjs b/scripts/ci/__tests__/stylesheet-bytes.test.mjs deleted file mode 100644 index 7a0631f..0000000 --- a/scripts/ci/__tests__/stylesheet-bytes.test.mjs +++ /dev/null @@ -1,102 +0,0 @@ -/** - * Byte-level guards on the stylesheet we ship. - * - * Dart Sass prepends an encoding hint whenever its output contains a non-ASCII character - a - * U+FEFF BOM in the compressed output we ship - and webpack's BannerPlugin then prepends the - * licence banner in front of it. A BOM at position 0 is stripped by every CSS parser; mid-file it - * is a valid CSS *ident* code point, so the parser reads it as the start of a selector, swallows - * the rule that follows and drops both. That silently deleted `@keyframes vscomp-animation-spin` - * while `.vscomp-options-loader::before` kept referencing it, leaving the options loader frozen. - * - * These assertions live here rather than in Cypress deliberately: the equivalent cases written as - * `cy.request()` passed against a bundle that genuinely carried a BOM, because the HTTP layer - * decodes the response and strips it on the way through - green while verifying nothing. Reading - * the built file from disk is the only way to see the bytes being asserted about. - * - * Skipped when `dist/` has not been built, so `npm run test:scripts` still works on a clean - * checkout; CI builds before testing, and the skip is reported rather than silent. - */ -import test from 'node:test'; -import assert from 'node:assert/strict'; -import { readFile } from 'node:fs/promises'; -import { fileURLToPath } from 'node:url'; - -const CSS_PATH = fileURLToPath(new URL('../../../dist/virtual-select.min.css', import.meta.url)); - -async function readCssBytes() { - try { - return await readFile(CSS_PATH); - } catch (error) { - if (error.code === 'ENOENT') return null; - throw error; - } -} - -test('the shipped stylesheet carries no byte-order mark', async (t) => { - const bytes = await readCssBytes(); - - if (!bytes) { - t.skip('dist/virtual-select.min.css not built - run `npm run build` first'); - return; - } - - const found = []; - - for (let i = 0; i < bytes.length - 2; i += 1) { - if (bytes[i] === 0xef && bytes[i + 1] === 0xbb && bytes[i + 2] === 0xbf) found.push(i); - } - - assert.deepEqual( - found, - [], - `EF BB BF (U+FEFF) found at byte offset ${found.join(', ')}. A BOM anywhere but offset 0 ` + - 'destroys the rule that follows it; BannerPlugin guarantees offset 0 is not where Sass put ' + - 'it. Fix with `charset: false` in the sass-loader options, not by reordering rules.', - ); -}); - -test('the shipped stylesheet is ASCII, so it needs no encoding declaration', async (t) => { - const bytes = await readCssBytes(); - - if (!bytes) { - t.skip('dist/virtual-select.min.css not built - run `npm run build` first'); - return; - } - - const found = []; - - for (let i = 0; i < bytes.length; i += 1) { - if (bytes[i] > 0x7f) found.push(`0x${bytes[i].toString(16)}@${i}`); - } - - assert.deepEqual( - found, - [], - `non-ASCII bytes: ${found.slice(0, 8).join(', ')}. With no encoding hint emitted, a non-ASCII ` + - "byte decodes according to the consuming page's charset. Emit the character as a CSS escape " + - "instead - e.g. string.unquote('\"\\\\26A0\"') - so the sheet stays ASCII.", - ); -}); - -test('the loader keyframes rule is present in the built stylesheet', async (t) => { - const bytes = await readCssBytes(); - - if (!bytes) { - t.skip('dist/virtual-select.min.css not built - run `npm run build` first'); - return; - } - - const css = bytes.toString('utf8'); - - assert.ok( - css.includes('@keyframes vscomp-animation-spin'), - 'the options loader animation is missing from the build output', - ); - /** matched loosely on purpose: the minifier rewrites `0.8s` to `.8s`, and pinning the exact - * shorthand asserts the minifier's formatting rather than that the reference survives */ - assert.match( - css, - /animation:[^;}]*vscomp-animation-spin/, - 'the loader rule no longer references the animation', - ); -});