diff --git a/packages/theme/doc/cover-image.png b/packages/theme/doc/cover-image.png deleted file mode 100644 index 58f227edf..000000000 Binary files a/packages/theme/doc/cover-image.png and /dev/null differ diff --git a/packages/theme/package.json b/packages/theme/package.json index 8db24655b..4f3caafeb 100644 --- a/packages/theme/package.json +++ b/packages/theme/package.json @@ -11,6 +11,7 @@ }, "scripts": { "pack:dry": "npm pack --dry-run", + "test": "node --test \"test/**/*.test.mjs\"", "format": "prettier --write scripts/ src/", "build": "pnpm build:tokens", "build:tokens": "node src/scripts/build-tokens.mjs", diff --git a/packages/theme/src/scripts/build-tokens.mjs b/packages/theme/src/scripts/build-tokens.mjs index 90ac34ba4..474c05101 100644 --- a/packages/theme/src/scripts/build-tokens.mjs +++ b/packages/theme/src/scripts/build-tokens.mjs @@ -35,6 +35,7 @@ import { containersData } from '../tokens/semantic/containers.data.js'; import { spacingsData } from '../tokens/semantic/spacings.data.js'; import { textsData } from '../tokens/semantic/texts.data.js'; import { zIndicesData } from '../tokens/semantic/z-indices.data.js'; +import { assertNoZeroWithUnit } from './zero-unit.mjs'; const BREAKPOINT_ORDER = ['sm', 'md', 'lg', 'xl', '2xl']; @@ -359,41 +360,6 @@ const emitCssV4 = () => { ].join('\n'); }; -// Keep in sync with the `zero-with-unit` / `zero-unit-in-calc` checks in -// packages/webkit/src/eslint-plugin/token-checks.js — theme sits below webkit in the -// dependency graph, so the shared engine cannot be imported here. Same known limit: -// the math-function lookbehind balances parens one level deep. -const MATH_FN = '(?:calc|min|max|clamp)\\((?:[^()]|\\([^()]*\\))*'; -const LENGTH_UNITS = - 'px|rem|em|ex|ch|cap|ic|lh|rlh|vw|vh|vmin|vmax|svw|svh|lvw|lvh|dvw|dvh|cqw|cqh|cqi|cqb|cqmin|cqmax|cm|mm|Q|in|pt|pc'; - -const ZERO_WITH_UNIT = new RegExp(`(? { - const lines = cssText.split('\n').map((line, i) => ({ line: line.trim(), n: i + 1 })); - const bare = lines.filter(({ line }) => ZERO_WITH_UNIT.test(line)); - const inMath = lines.filter(({ line }) => ZERO_UNIT_IN_MATH.test(line)); - if (bare.length === 0 && inMath.length === 0) return; - const detail = (rows) => rows.map(({ line, n }) => ` globals.css:${n} ${line}`).join('\n'); - const parts = [`build:tokens — ${bare.length + inMath.length} token value(s) misuse a zero.`]; - if (bare.length > 0) { - parts.push( - `A zero length takes no unit: write '0', not '0px' / '0rem' / '0em'.\n${detail(bare)}`, - ); - } - if (inMath.length > 0) { - parts.push( - `Inside calc()/min()/max()/clamp() the zero needs a unit, and that unit is rem: write '0rem'.\n${detail(inMath)}`, - ); - } - parts.push('Fix the token source under src/tokens/, not the generated CSS.'); - throw new Error(parts.join('\n')); -}; - // ─── 5. Write to disk ────────────────────────────────────────────────────── const __dirname = dirname(fileURLToPath(import.meta.url)); const distRoot = resolve(__dirname, '../../dist'); @@ -415,7 +381,7 @@ const importIdx = rawCss.indexOf(IMPORT_LINE); if (importIdx === -1) throw new Error('emitCssV4 output is missing the tailwind import line'); const afterImport = importIdx + IMPORT_LINE.length; const css = `${rawCss.slice(0, afterImport)}\n\n${fontsCss}${rawCss.slice(afterImport)}`; -assertNoZeroWithUnit(css); +assertNoZeroWithUnit(css, 'globals.css'); await writeFile(resolve(dir, 'globals.css'), css, 'utf8'); await writeFile(resolve(dir, 'globals.scss'), css, 'utf8'); console.log(`✓ v4 → ${dir}`); diff --git a/packages/theme/src/scripts/zero-unit.mjs b/packages/theme/src/scripts/zero-unit.mjs new file mode 100644 index 000000000..3ea46f048 --- /dev/null +++ b/packages/theme/src/scripts/zero-unit.mjs @@ -0,0 +1,64 @@ +/** + * Zero-with-unit gate for the compiled token stylesheet — the only check that sees token + * values (authored in JS, so no linter does), run on the final CSS before globals.css is + * written. A zero length is `0`; inside calc()/min()/max()/clamp() it must carry a unit, and + * that unit is `rem` (.claude/rules/styling.md). Mirror of the zero checks in packages/webkit + * token-checks.js, kept in sync — LENGTH_UNITS is the same string. Pure: node built-ins only. + */ + +// Opens a math function; balances parens one level deep (a zero nested deeper falls back to +// the bare-zero check). +const MATH_FN = '(?:calc|min|max|clamp)\\((?:[^()]|\\([^()]*\\))*'; + +// Complete CSS length-unit set — must be complete, since a missing unit silently passes. +export const LENGTH_UNITS = + 'px|rem|em|ex|rex|ch|rch|cap|rcap|ic|ric|lh|rlh|vw|vh|vi|vb|vmin|vmax|svw|svh|svi|svb|svmin|svmax|lvw|lvh|lvi|lvb|lvmin|lvmax|dvw|dvh|dvi|dvb|dvmin|dvmax|cqw|cqh|cqi|cqb|cqmin|cqmax|cm|mm|Q|in|pt|pc'; + +// Mutually exclusive, so a value is reported once. Non-global so `.test()` is stateless — +// findZeroMisuse adds `g` locally to enumerate matches. +export const ZERO_WITH_UNIT = new RegExp(`(? { + const scan = new RegExp(regex.source, `${regex.flags.replace('g', '')}g`); + const out = []; + for (let m; (m = scan.exec(cssText)); ) { + const lineStart = cssText.lastIndexOf('\n', m.index - 1) + 1; + const nlIdx = cssText.indexOf('\n', m.index); + const lineEnd = nlIdx === -1 ? cssText.length : nlIdx; + out.push({ line: cssText.slice(lineStart, lineEnd).trim(), n: cssText.slice(0, m.index).split('\n').length }); + } + return out; +}; + +// The `--custom-property` a line declares, to name the token to fix; null off a `--token:` line. +const tokenOf = (line) => line.match(/(--[\w-]+)\s*:/)?.[1] ?? null; + +/** Throw if `cssText` misuses a zero length. `source` names the artifact in the message. */ +export const assertNoZeroWithUnit = (cssText, source = 'globals.css') => { + const bare = findZeroMisuse(cssText, ZERO_WITH_UNIT); + const inMath = findZeroMisuse(cssText, ZERO_UNIT_IN_MATH); + if (bare.length === 0 && inMath.length === 0) return; + + const detail = (rows) => rows.map(({ line, n }) => ` ${source}:${n} ${line}`).join('\n'); + const tokens = [...new Set([...bare, ...inMath].map(({ line }) => tokenOf(line)).filter(Boolean))]; + const parts = [`build:tokens — ${bare.length + inMath.length} token value(s) misuse a zero.`]; + if (bare.length > 0) { + parts.push(`A zero length takes no unit: write '0', not '0px' / '0rem' / '0em'.\n${detail(bare)}`); + } + if (inMath.length > 0) { + parts.push( + `Inside calc()/min()/max()/clamp() the zero needs a unit, and that unit is rem: write '0rem'.\n${detail(inMath)}`, + ); + } + const grepHint = tokens.length > 0 ? ` — grep the offending token name (${tokens.join(', ')})` : ''; + parts.push(`Fix the token at its source under src/tokens/**${grepHint}, not the generated ${source}.`); + throw new Error(parts.join('\n')); +}; diff --git a/packages/theme/test/zero-unit.test.mjs b/packages/theme/test/zero-unit.test.mjs new file mode 100644 index 000000000..ade3f4244 --- /dev/null +++ b/packages/theme/test/zero-unit.test.mjs @@ -0,0 +1,134 @@ +// Pins the zero-with-unit gate (src/scripts/zero-unit.mjs): both sides of the rule (bare +// zero outside math; `0rem`-only inside), the completed unit list, the multi-line calc scan, +// mutual exclusivity, and the error message. + +import { test } from 'node:test'; +import assert from 'node:assert/strict'; + +import { + ZERO_WITH_UNIT, + ZERO_UNIT_IN_MATH, + findZeroMisuse, + assertNoZeroWithUnit, +} from '../src/scripts/zero-unit.mjs'; + +// `.test()` is stateless on these (non-global) regexes — see the module note. +const isBare = (s) => ZERO_WITH_UNIT.test(s); +const inMath = (s) => ZERO_UNIT_IN_MATH.test(s); + +test('a zero length carrying a unit is flagged, across the full unit list', () => { + for (const unit of [ + // absolute + 'px', 'cm', 'mm', 'Q', 'in', 'pt', 'pc', + // font-relative + root-relative variants + 'em', 'rem', 'ex', 'rex', 'ch', 'rch', 'cap', 'rcap', 'ic', 'ric', 'lh', 'rlh', + // viewport incl. the logical (vi/vb) + small/large/dynamic + min/max variants + 'vw', 'vh', 'vi', 'vb', 'vmin', 'vmax', + 'svw', 'svh', 'svi', 'svb', 'svmin', 'svmax', + 'lvw', 'lvh', 'lvi', 'lvb', 'lvmin', 'lvmax', + 'dvw', 'dvh', 'dvi', 'dvb', 'dvmin', 'dvmax', + // container query + 'cqw', 'cqh', 'cqi', 'cqb', 'cqmin', 'cqmax', + ]) { + assert.ok(isBare(`--x: 0${unit};`), `expected 0${unit} to be flagged as a bare zero`); + } +}); + +test('the units a partial list dropped are no longer silent', () => { + // These are exactly the units ENG-46996 called out as passing through before. + for (const unit of ['vi', 'vb', 'svi', 'svb', 'lvi', 'lvb', 'dvi', 'dvb', 'rex', 'rch', 'ric', 'rcap']) { + assert.ok(isBare(`margin: 0${unit}`), `regression: 0${unit} slipped through`); + } +}); + +test('units are case-insensitive', () => { + for (const s of ['margin: 0PX', '--x: 0Rem', 'top: 0VI', 'width: 0SvB']) { + assert.ok(isBare(s), `expected case-insensitive match for: ${s}`); + } +}); + +test('a bare zero, non-zero lengths, and meaningful-at-zero units stay silent', () => { + for (const s of [ + 'margin: 0', + 'width: 10px', + 'padding: 0.5rem', + 'flex-basis: 0%', + 'transition-duration: 0s', + 'animation-delay: 0ms', + 'rotate: 0deg', + 'grid-template-columns: 0fr', + 'z-index: 0', + ]) { + assert.ok(!isBare(s), `expected silence for: ${s}`); + assert.ok(!inMath(s), `expected silence (math) for: ${s}`); + } +}); + +test('a zero with a unit inside a math function is zero-unit-in-calc, never zero-with-unit', () => { + for (const s of [ + 'width: calc(100% - 0px)', + 'height: min(0em, 1rem)', + 'width: clamp(0px, 2vw, 1rem)', + 'inset: max(0vi, var(--x))', // a newly-covered unit, inside math + 'width: calc(var(--x) - 0px)', // one nested call before the zero + ]) { + assert.ok(inMath(s), `expected zero-unit-in-calc for: ${s}`); + assert.ok(!isBare(s), `expected NOT zero-with-unit for: ${s}`); + } +}); + +test('0rem inside a math function is the sanctioned form — both checks stay silent', () => { + for (const s of ['width: calc(100% - 0rem)', 'height: max(0rem, var(--h))']) { + assert.ok(!isBare(s), `expected no zero-with-unit for: ${s}`); + assert.ok(!inMath(s), `expected no zero-unit-in-calc for: ${s}`); + } +}); + +test('a closed math function does not leak its exemption to a later zero', () => { + const css = '--a: calc(100% - 1px);\n--b: 0px;'; + assert.equal(findZeroMisuse(css, ZERO_WITH_UNIT).length, 1, 'the zero after the closed calc() is bare'); + assert.equal(findZeroMisuse(css, ZERO_UNIT_IN_MATH).length, 0, 'nothing inside the calc() misuses a unit'); +}); + +// Why the scan runs over the whole string: a per-line scan would mis-read the wrapped tail +// (`- 0px);`) as a bare zero. +test('a math function wrapping across lines is still judged in context', () => { + const wrapped = '--x: calc(100%\n - 0px);'; + assert.equal(findZeroMisuse(wrapped, ZERO_UNIT_IN_MATH).length, 1, 'the wrapped 0px is inside calc()'); + assert.equal(findZeroMisuse(wrapped, ZERO_WITH_UNIT).length, 0, 'it must NOT read as a bare zero'); + + const wrappedOk = '--x: calc(100%\n - 0rem);'; + assert.equal(findZeroMisuse(wrappedOk, ZERO_UNIT_IN_MATH).length, 0, 'wrapped 0rem is sanctioned'); + assert.equal(findZeroMisuse(wrappedOk, ZERO_WITH_UNIT).length, 0, 'and is not a bare zero'); +}); + +test('findZeroMisuse reports the 1-based line and the trimmed source line', () => { + const css = 'a: 0\nb: 1px\n--tracking-normal: 0em;'; + const hits = findZeroMisuse(css, ZERO_WITH_UNIT); + assert.equal(hits.length, 1); + assert.deepEqual(hits[0], { line: '--tracking-normal: 0em;', n: 3 }); +}); + +test('assertNoZeroWithUnit passes on a clean stylesheet and is a no-op', () => { + assert.doesNotThrow(() => assertNoZeroWithUnit(':root { margin: 0; width: calc(100% - 0rem); }')); +}); + +test('assertNoZeroWithUnit names the artifact and the offending token', () => { + let err; + try { + assertNoZeroWithUnit('--tracking-normal: 0em;', 'globals.scss'); + } catch (e) { + err = e; + } + assert.ok(err, 'expected a throw'); + assert.match(err.message, /globals\.scss:1/, 'honest, parametrized artifact name'); + assert.match(err.message, /--tracking-normal/, 'names the token to fix'); + assert.doesNotMatch(err.message, /globals\.css/, 'no hardcoded globals.css leak'); +}); + +test('the one-level nesting limit is pinned (mirror of the webkit engine)', () => { + // A zero ≥2 calls deep falls back to the bare-zero check, by documented design. + const css = 'width: calc(min(max(1px, 2px), 3px) - 0px)'; + assert.ok(isBare(css), 'expected the depth-limited fallback report'); + assert.ok(!inMath(css), 'zero-unit-in-calc cannot see this deep'); +}); diff --git a/packages/webkit/src/eslint-plugin/token-checks.js b/packages/webkit/src/eslint-plugin/token-checks.js index 936de89a4..a35954e0a 100644 --- a/packages/webkit/src/eslint-plugin/token-checks.js +++ b/packages/webkit/src/eslint-plugin/token-checks.js @@ -74,19 +74,19 @@ export const TOKEN_CHECKS = [ // nested calls in — calc(min(max(1px, 2px), 3px) - 0px) — or behind grouping parens // is reported here (as a bare-zero violation) instead of by zero-unit-in-calc, so // the message will say `0` where the position actually requires `0rem`. Keep in - // sync with ZERO_WITH_UNIT in packages/theme/src/scripts/build-tokens.mjs (theme + // sync with ZERO_WITH_UNIT in packages/theme/src/scripts/zero-unit.mjs (theme // sits below webkit in the dependency graph, so it cannot import this engine). regex: - /(? { } }) +test('the completed unit list catches the logical/root-relative units (ENG-46996)', () => { + // Representative new units; the theme test (zero-unit.test.mjs) pins the full set. Kept + // in sync with LENGTH_UNITS in packages/theme/src/scripts/zero-unit.mjs. + for (const unit of ['vi', 'vb', 'svi', 'dvb', 'rex', 'rch', 'ric', 'rcap']) { + assert.ok(ids(`x: 0${unit}`).includes('zero-with-unit'), `0${unit} slipped through`) + } +}) + +test('a newly-covered unit inside a math function is zero-unit-in-calc, not zero-with-unit', () => { + const found = ids('inset: max(0vi, var(--x))') + assert.ok(found.includes('zero-unit-in-calc'), 'expected zero-unit-in-calc for 0vi in max()') + assert.ok(!found.includes('zero-with-unit'), 'expected no zero-with-unit for 0vi in max()') +}) + test('zero-with-unit stays silent for bare zeros, non-zero lengths and meaningful units', () => { for (const content of [ 'margin: 0',