From 469ceb585dc76395c4da47325c23ee079072452b Mon Sep 17 00:00:00 2001 From: borskyj Date: Wed, 2 Sep 2026 22:08:21 +0200 Subject: [PATCH] fix(publisher): keep classes a runtime script toggles Publish tree-shakes the class registry against node class ids, so a rule survives only when some authored node carries its class. A modifier that only exists at runtime, toggled by a script, is carried by no node, so publish dropped it. The rule is present and correct at every point before publish. site_read_styles returns it, the canvas renders it, the stored document round-trips it. It is absent only from the published stylesheet, so a nav that opens in the editor does nothing on the live site and the search starts on the script and reaches the stylesheet last. Collect the identifier-shaped runs in each script file and treat a class whose name appears among them as used. Splitting the source on characters a CSS class name cannot contain covers classList.add, className assignment, template literals and lookup tables without modelling any of them, at the cost of over-collecting ordinary identifiers. Over-collecting is the safe direction: a false positive costs a few bytes of CSS, a false negative costs a feature that works everywhere except in production. Script-referenced ids are unioned into the used set and never subtracted, so this can only keep more CSS than before, never less. Widen collectUsedStyleRuleIds and usedStyleRuleIdSignature to take files and styleRules. Both callers already pass a whole site document, and routing the canvas through the same collection keeps the editor and the publisher agreeing on which rules are live. The run set is cached on files-array identity because the signature helper runs inside a canvas store selector. Fixes #465 Co-Authored-By: Claude Opus 5 --- .../publisher/classStyleInjector.test.ts | 49 +++++++++++++++ src/core/publisher/styleRuleTreeShake.ts | 59 ++++++++++++++++++- 2 files changed, 105 insertions(+), 3 deletions(-) diff --git a/src/__tests__/publisher/classStyleInjector.test.ts b/src/__tests__/publisher/classStyleInjector.test.ts index 4b5f5efc2..f120609cb 100644 --- a/src/__tests__/publisher/classStyleInjector.test.ts +++ b/src/__tests__/publisher/classStyleInjector.test.ts @@ -684,6 +684,7 @@ describe('generateClassCSS', () => { function makeSite( styleRules: SiteDocument['styleRules'], nodeClassIds: Record = {}, + files: SiteDocument['files'] = [], ): SiteDocument { const node: PageNode = { id: 'root', @@ -722,12 +723,60 @@ function makeSite( shortcuts: {}, }, styleRules, + files, createdAt: 0, updatedAt: 0, } } +function makeScript(path: string, content: string): SiteDocument['files'][number] { + return { id: path, path, type: 'script', content } as SiteDocument['files'][number] +} + describe('collectClassCSS', () => { + it('keeps a class no node carries when a runtime script names it', () => { + // The reason tree-shaking cannot work from node class ids alone: a modifier + // toggled by script is never authored onto a node, so publish saw it as + // dead and dropped it. The rule read back correctly everywhere up to + // publish, so the failure only showed on the live site. + const site = makeSite( + { + nav: makeClass('nav', { display: 'none' }), + 'nav-open': makeClass('nav-open', { display: 'flex' }, {}, 'nav--open'), + }, + { root: ['nav'] }, + [makeScript('scripts/nav.js', "button.addEventListener('click', () => menu.classList.toggle('nav--open'))")], + ) + const css = collectClassCSS(site) + expect(css).toContain('.nav {') + expect(css).toContain('.nav--open {') + expect(css).toContain('display: flex') + }) + + it('still drops a class that neither a node nor a script references', () => { + const site = makeSite( + { + nav: makeClass('nav', { display: 'none' }), + 'nav-open': makeClass('nav-open', { display: 'flex' }, {}, 'nav--open'), + orphan: makeClass('orphan', { color: 'red' }), + }, + { root: ['nav'] }, + [makeScript('scripts/nav.js', "menu.classList.toggle('nav--open')")], + ) + const css = collectClassCSS(site) + expect(css).toContain('.nav--open {') + expect(css).not.toContain('.orphan') + }) + + it('only counts script files, not other site file types', () => { + const site = makeSite( + { orphan: makeClass('orphan', { color: 'red' }) }, + {}, + [{ id: 'notes', path: 'docs/notes.md', type: 'doc', content: 'the orphan class is for later' } as SiteDocument['files'][number]], + ) + expect(collectClassCSS(site)).not.toContain('.orphan') + }) + it('emits user-authored CSS but skips framework-generated CSS', () => { const userClass = makeClass('user-class', { color: 'green' }) const frameworkClass: StyleRule = { diff --git a/src/core/publisher/styleRuleTreeShake.ts b/src/core/publisher/styleRuleTreeShake.ts index 8033396bb..854b15e79 100644 --- a/src/core/publisher/styleRuleTreeShake.ts +++ b/src/core/publisher/styleRuleTreeShake.ts @@ -6,9 +6,54 @@ import { type StyleRule, } from '@core/page-tree' -/** Collect every registry class id referenced by page and Visual Component nodes. */ +let lastScriptFiles: SiteDocument['files'] | null = null +let lastScriptRuns: Set = new Set() + +/** + * Identifier-shaped runs in a runtime script's source. + * + * A modifier a script toggles never appears on an authored node, so node class + * ids alone cannot see it. The script is the only place it is written down, in + * whatever call the author used — `classList.add('nav--open')`, a `className` + * assignment, a template literal, a lookup table of state names. Rather than + * model those shapes, split the source on everything a CSS class name cannot + * contain and keep the runs that survive. + * + * This over-collects: `add`, `length` and every other identifier in the file + * land in the set too, so a class named after one of them is kept even when no + * script really references it. That is the safe direction. The cost of a false + * positive is a few bytes of CSS; the cost of a false negative is a rule that + * is correct everywhere until publish drops it and the feature dies on the + * live site with nothing to point at. + */ +function scriptIdentifierRuns(files: SiteDocument['files']): Set { + // `usedStyleRuleIdSignature` runs inside a canvas store selector, so this is + // hit on every store change. The store snapshot is immutable, so identity on + // the files array is enough to skip re-splitting unchanged sources. + if (files === lastScriptFiles) return lastScriptRuns + + const runs = new Set() + for (const file of files) { + if (file.type !== 'script' || typeof file.content !== 'string') continue + for (const run of file.content.split(/[^A-Za-z0-9_-]+/)) { + if (run) runs.add(run) + } + } + + lastScriptFiles = files + lastScriptRuns = runs + return runs +} + +/** + * Collect every registry class id referenced by page and Visual Component + * nodes, plus every class a runtime script names. + * + * Script-referenced ids are unioned in, never subtracted, so this can only + * ever keep more CSS than node class ids alone would. + */ export function collectUsedStyleRuleIds( - site: Pick, + site: Pick, ): Set { const usedIds = new Set() for (const page of site.pages) { @@ -22,6 +67,14 @@ export function collectUsedStyleRuleIds( for (const id of node.classIds ?? []) usedIds.add(id) } } + + const runs = scriptIdentifierRuns(site.files ?? []) + if (runs.size > 0) { + for (const rule of Object.values(site.styleRules ?? {})) { + if (rule.kind === 'class' && runs.has(rule.name)) usedIds.add(rule.id) + } + } + return usedIds } @@ -30,7 +83,7 @@ export function collectUsedStyleRuleIds( * only when the set of assigned class ids changes, not for unrelated edits. */ export function usedStyleRuleIdSignature( - site: Pick, + site: Pick, ): string { return [...collectUsedStyleRuleIds(site)].sort().join('\0') }