diff --git a/docs/badges.json b/docs/badges.json index d928bca6..43170d6e 100644 --- a/docs/badges.json +++ b/docs/badges.json @@ -10,7 +10,7 @@ "packages": 43, "goFiles": 192, "goLines": 48278, - "testFiles": 931, + "testFiles": 932, "directDeps": 3, "transitiveDeps": 15, "totalTests": 6479, diff --git a/tests/spec/css_modifier_layer_test.go b/tests/spec/css_modifier_layer_test.go new file mode 100644 index 00000000..c0dd6e5d --- /dev/null +++ b/tests/spec/css_modifier_layer_test.go @@ -0,0 +1,109 @@ +package spec_test + +import ( + "path/filepath" + "regexp" + "sort" + "strings" + "testing" +) + +// A size modifier has to be able to beat the class it modifies. +// +// Tailwind v4 compiles every @utility into @layer utilities. A plain CSS rule +// written outside any @layer is unlayered, and unlayered CSS wins over layered +// CSS in the cascade whatever the specificity. So when the base class is a plain +// rule and its modifier is an @utility, the base overrides the modifier and the +// modifier does nothing at all. +// +// That is what happened to vault42-spinner-sm. .vault42-spinner was a plain rule +// setting w-5 h-5; vault42-spinner-sm was an @utility setting w-4 h-4. Every +// "small" spinner in the app rendered at the default size, on two call sites, +// and nothing could see it: the class was present in the markup, the rule was +// present in the stylesheet, and the build emitted both. Only the layer differed. +// vault42-spinner-lg escaped only because somebody happened to write it as a +// plain rule next to the base. +// +// This gate holds the pairing rather than the sizes: a modifier and its base +// must be declared the same way. +// +// That is a necessary condition and not a sufficient one, and the difference is +// worth stating here so nobody reads a pass as proof. Order inside @layer +// utilities is Tailwind's property-set sort, not source order, so two utilities +// declared identically can still emit in the wrong order -- measured, adding +// w-full to vault42-btn-sm moves it ahead of vault42-btn and makes it inert at +// every one of its nine call sites, and this gate passes throughout. The gate +// that settles it compiles the stylesheet and reads the emitted offsets: +// web/src/__tests__/spinnerCascade.test.ts. This one is the cheap early signal +// that runs with the Go suite. +// +// The tests are read-only. They never write to the source tree. + +var ( + styleSheet = filepath.Join("web", "src", "style.css") + + // `@utility name {` -- the layered form. + utilityDecl = regexp.MustCompile(`(?m)^@utility\s+([a-zA-Z0-9_-]+)\s*\{`) + // `.name {` at column zero -- the unlayered form. + plainDecl = regexp.MustCompile(`(?m)^\.([a-zA-Z0-9_-]+)\s*\{`) + + // Suffixes that mean "this class modifies another one" rather than standing + // alone. A class ending in one of these is checked against its base. + modifierSuffixes = []string{"-sm", "-lg", "-xs", "-xl"} +) + +func TestACSSModifierIsDeclaredTheSameWayAsTheClassItModifies(t *testing.T) { + css := readFileString(t, filepath.Join(repoRoot(t), styleSheet)) + + // class name -> "utility" or "plain" + form := map[string]string{} + for _, m := range utilityDecl.FindAllStringSubmatch(css, -1) { + form[m[1]] = "@utility" + } + for _, m := range plainDecl.FindAllStringSubmatch(css, -1) { + form[m[1]] = "a plain rule" + } + + if len(form) == 0 { + t.Fatalf("no class declarations found in %s. If the stylesheet moved, move this "+ + "gate with it: what it holds is that a size modifier can actually beat the "+ + "class it modifies.", styleSheet) + } + + var checked int + names := make([]string, 0, len(form)) + for name := range form { + names = append(names, name) + } + sort.Strings(names) + + for _, name := range names { + for _, suffix := range modifierSuffixes { + if !strings.HasSuffix(name, suffix) { + continue + } + base := strings.TrimSuffix(name, suffix) + baseForm, ok := form[base] + if !ok { + // Not a modifier of anything in this sheet; it just ends that way. + continue + } + checked++ + if baseForm != form[name] { + t.Errorf("%s is %s and %s is %s.\n"+ + "@utility rules land in @layer utilities and plain rules are unlayered, "+ + "and unlayered beats layered whatever the specificity -- so the plain one "+ + "wins and the other is inert no matter what it declares. Declare both the "+ + "same way and let source order decide.", + name, form[name], base, baseForm) + } + } + } + + // A gate that compares no pairs proves nothing. + if checked == 0 { + t.Fatalf("no modifier/base pairs found in %s, so this gate compared nothing. "+ + "If the naming convention changed, update modifierSuffixes.", styleSheet) + } + t.Logf("checked %d modifier/base pairs", checked) +} diff --git a/web/src/__tests__/spinnerCascade.test.ts b/web/src/__tests__/spinnerCascade.test.ts new file mode 100644 index 00000000..f223f4ed --- /dev/null +++ b/web/src/__tests__/spinnerCascade.test.ts @@ -0,0 +1,120 @@ +import { describe, it, expect } from 'vitest' +import { readFileSync, readdirSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import { dirname, join, resolve as resolvePath } from 'node:path' +import { compile } from 'tailwindcss' + +/** + * A size modifier has to actually beat the class it modifies, in the output. + * + * Declaring both the same way is necessary and not sufficient, which is the + * whole reason this gate compiles instead of reading. Tailwind orders rules + * inside @layer utilities by a property-set sort, not by source order, so two + * utilities written adjacently can still emit in the wrong order -- measured: + * adding w-full, h-4, block, inline-flex or z-10 to vault42-btn-sm moves it + * ahead of vault42-btn and makes it inert at every one of its call sites, while + * adding rounded-md or leading-none does not. Nothing about the source says + * which of those you just did. + * + * The defect this was written for: vault42-spinner-sm set w-4 h-4 as an + * @utility, which lands in @layer utilities, while .vault42-spinner was a plain + * unlayered rule setting w-5 h-5. Unlayered beats layered whatever the + * specificity, so every "small" spinner rendered at the default size. The class + * was in the markup, the rule was in the stylesheet, the build emitted both. + * Only the layer differed. + */ + +const srcDir = resolvePath(dirname(fileURLToPath(import.meta.url)), '..') +const repoRoot = resolvePath(srcDir, '../..') + +/** Modifier/base pairs whose relative order in the output is load-bearing. */ +const PAIRS: Array<[string, string]> = [ + ['vault42-spinner-sm', 'vault42-spinner'], + ['vault42-spinner-lg', 'vault42-spinner'], + ['vault42-btn-sm', 'vault42-btn'], +] + +function sourceFiles(dir: string, out: string[] = []): string[] { + for (const e of readdirSync(dir, { withFileTypes: true })) { + if (e.name === 'node_modules' || e.name === 'dist' || e.name === '.git') continue + const p = join(dir, e.name) + if (e.isDirectory()) sourceFiles(p, out) + else if (/\.(vue|ts|js|html)$/.test(e.name)) out.push(p) + } + return out +} + +async function buildCSS(): Promise { + const files = [ + ...sourceFiles(srcDir), + ...sourceFiles(resolvePath(repoRoot, 'packages/vue/src')), + resolvePath(repoRoot, 'web/index.html'), + ] + const candidates = new Set() + for (const f of files) { + for (const m of readFileSync(f, 'utf8').matchAll(/[^\s"'`<>=(){};,]+/g)) candidates.add(m[0]) + } + + const twBase = resolvePath(repoRoot, 'web/node_modules/tailwindcss') + const compiler = await compile(readFileSync(join(srcDir, 'style.css'), 'utf8'), { + base: srcDir, + loadStylesheet: async (id: string, base: string) => { + const file = id === 'tailwindcss' + ? join(twBase, 'index.css') + : id.startsWith('tailwindcss/') + ? join(twBase, id.slice('tailwindcss/'.length)) + : resolvePath(base, id) + return { path: file, base: dirname(file), content: readFileSync(file, 'utf8') } + }, + loadModule: async () => { + throw new Error('no modules') + }, + }) + return compiler.build([...candidates]) +} + +/** + * Offset of a class's own rule in the emitted CSS, or -1. + * + * Plain string scanning rather than a regex built from the class name. The + * first version escaped the name with cls.replace(/[-]/g, ...), which handles + * exactly one metacharacter and is the shape CodeQL calls incomplete + * sanitization -- correctly, even though every name here is a literal from the + * table above. Not building a pattern from a value is simpler than escaping one. + * + * The delimiter check is what stops `.vault42-spinner` matching inside + * `.vault42-spinner-sm`: a class name ends where the selector does. + */ +function ruleOffset(css: string, cls: string): number { + const needle = `.${cls}` + for (let i = css.indexOf(needle); i !== -1; i = css.indexOf(needle, i + 1)) { + const after = css[i + needle.length] + if (after === undefined) continue + if (after === '{' || after === ',' || after === ' ' || after === '\n' || after === ':') { + return i + } + } + return -1 +} + +describe('utility cascade', () => { + it('emits every size modifier after the class it modifies', async () => { + const css = await buildCSS() + + for (const [modifier, base] of PAIRS) { + const mOff = ruleOffset(css, modifier) + const bOff = ruleOffset(css, base) + + expect(mOff, `${modifier} is not in the built CSS at all`).toBeGreaterThan(-1) + expect(bOff, `${base} is not in the built CSS at all`).toBeGreaterThan(-1) + + expect( + mOff > bOff, + `${modifier} is emitted at ${mOff}, before ${base} at ${bOff}, so ${base} wins and ` + + `${modifier} does nothing. Order inside @layer utilities is Tailwind's property-set ` + + `sort, not source order -- adding a property to the modifier can move it ahead of its ` + + `base without anything in the source looking different.`, + ).toBe(true) + } + }) +}) diff --git a/web/src/style.css b/web/src/style.css index 8bda7cbc..8e4a546e 100644 --- a/web/src/style.css +++ b/web/src/style.css @@ -148,11 +148,6 @@ max-w-md w-full mx-4 shadow-lg; } -@utility vault42-spinner-sm { - /* Small spinner variant */ - @apply w-4 h-4; -} - @layer base { body { font-family: 'Inter', system-ui, -apple-system, sans-serif; @@ -288,11 +283,29 @@ @keyframes vault42-spin { to { transform: rotate(360deg); } } -.vault42-spinner { +/* All three are @utility, and that is the point rather than a formality. + Tailwind compiles @utility into @layer utilities; a plain rule written outside + any @layer is unlayered, and unlayered beats layered whatever the specificity. + With the base plain and vault42-spinner-sm an @utility, the base simply + overrode it and every "small" spinner rendered at 20px. vault42-spinner-lg + escaped only because somebody happened to write it plain beside the base. + + Making all three layered fixes that and one more thing: while .vault42-spinner + was unlayered it silently beat 45 of the utilities present in the bundle, so + an ordinary h-4 or w-4 on a spinner did nothing either. Moving the modifier + out instead would have restored the 16px and left that hazard in place. + + Order within the layer is Tailwind's property-set sort, not source order, so + this is measured rather than assumed -- see the compile-time gate in + web/src/__tests__/spinnerCascade.test.ts. */ +@utility vault42-spinner { @apply inline-block w-5 h-5 border-2 border-vault42-control border-t-vault42-accent rounded-full; animation: vault42-spin 0.6s linear infinite; } -.vault42-spinner-lg { +@utility vault42-spinner-sm { + @apply w-4 h-4; +} +@utility vault42-spinner-lg { @apply w-8 h-8; }