also looking at this
LSU's own coaching surface, and what auditing the site turned up
#12
86 files
+3943
-1190
Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.
| @@ -204,6 +204,56 @@ export function auditPage(page: string, raw: string, served: Set<string>): Findi | ||
| 204 | 204 | return findings |
| 205 | 205 | } |
| 206 | 206 | |
| 207 | /** | |
| 208 | * Class names the markup uses that no stylesheet defines. | |
| 209 | * | |
| 210 | * This is how /lottery's entry steps and open events, /404's whole heading | |
| 211 | * block and the results tables' .h3 were found: markup written against a | |
| 212 | * design system that was never shipped renders as unstyled body text, and | |
| 213 | * nothing errors. Read against the BUILT css, so the generated utilities | |
| 214 | * count as defined. | |
| 215 | * | |
| 216 | * The allowlist is a ratchet, not an exemption. Each entry is markup whose | |
| 217 | * class carries no styling today; listing them stops the set growing while | |
| 218 | * leaving what is already there visible. | |
| 219 | */ | |
| 220 | const UNSTYLED_ALLOWED = new Set([ | |
| 221 | // Structural or scripting hooks with nothing to paint. | |
| 222 | 'race-board', 'lap-link', 'carousel', 'seg-count', 'enter-count', | |
| 223 | 'ls-cmp-yr', 'edon-l', 'ws-soon', | |
| 224 | // Wrappers whose typography comes from the block around them. | |
| 225 | 'sr-lead', 'ws-lead', 'ws-apply', 'tiers-title', 'begin-band', | |
| 226 | // Utilities inside @stacksjs/components' Dialog, generated from that | |
| 227 | // package's own source rather than from this app's markup. | |
| 228 | 'bg-black/25', 'dark:bg-black/50', | |
| 229 | ]) | |
| 230 | ||
| 231 | /** Every class name any shipped stylesheet keys a rule on. */ | |
| 232 | export function definedClasses(files: string[]): Set<string> { | |
| 233 | const names = new Set<string>() | |
| 234 | for (const file of files) { | |
| 235 | for (const [, name] of readFileSync(file, 'utf8').matchAll(/\.(-?[_a-zA-Z][\w-]*)/g)) | |
| 236 | names.add(name!) | |
| 237 | } | |
| 238 | return names | |
| 239 | } | |
| 240 | ||
| 241 | export function unstyledClasses(page: string, raw: string, defined: Set<string>): Finding[] { | |
| 242 | const findings: Finding[] = [] | |
| 243 | const body = visible(raw) | |
| 244 | const seen = new Set<string>() | |
| 245 | for (const [, value] of body.matchAll(/\sclass="([^"]+)"/g)) { | |
| 246 | for (const name of value!.split(/\s+/)) { | |
| 247 | // stx writes a scope class per render and a wrapper id per component. | |
| 248 | if (!name || name.startsWith('lsu-sz') || name.startsWith('stx_')) continue | |
| 249 | if (defined.has(name) || UNSTYLED_ALLOWED.has(name) || seen.has(name)) continue | |
| 250 | seen.add(name) | |
| 251 | findings.push({ page, rule: 'unstyled-class', detail: name }) | |
| 252 | } | |
| 253 | } | |
| 254 | return findings | |
| 255 | } | |
| 256 | ||
| 207 | 257 | /** Findings grouped by rule, for a terminal. */ |
| 208 | 258 | export function formatFindings(findings: Finding[]): string { |
| 209 | 259 | const byRule = new Map<string, Finding[]>() |
| @@ -225,7 +275,12 @@ export function formatFindings(findings: Finding[]): string { | ||
| 225 | 275 | if (import.meta.main) { |
| 226 | 276 | const files = builtPages() |
| 227 | 277 | const served = new Set([...files.map(urlOf), ...redirectSources()]) |
| 228 | const findings = files.flatMap(file => auditPage(urlOf(file), readFileSync(file, 'utf8'), served)) | |
| 278 | const styles = [...new Bun.Glob('**/*.css').scanSync({ cwd: ROOT, absolute: true })] | |
| 279 | const defined = definedClasses(styles) | |
| 280 | const findings = files.flatMap((file) => { | |
| 281 | const html = readFileSync(file, 'utf8') | |
| 282 | return [...auditPage(urlOf(file), html, served), ...unstyledClasses(urlOf(file), html, defined)] | |
| 283 | }) | |
| 229 | 284 | |
| 230 | 285 | console.log(`audited ${files.length} pages`) |
| 231 | 286 | console.log(findings.length === 0 ? '\nno findings' : `\n${formatFindings(findings)}`) |
| @@ -2,7 +2,7 @@ import { describe, expect, it } from 'bun:test' | ||
| 2 | 2 | import { readdirSync } from 'node:fs' |
| 3 | 3 | import { join, resolve } from 'node:path' |
| 4 | 4 | import { processDirectives } from '@stacksjs/stx' |
| 5 | import { assetExists, assetUrls, auditPage, formatFindings, redirectSources } from '../scripts/audit-html' | |
| 5 | import { assetExists, assetUrls, auditPage, definedClasses, formatFindings, redirectSources, unstyledClasses } from '../scripts/audit-html' | |
| 6 | 6 | |
| 7 | 7 | /** |
| 8 | 8 | * The same rules scripts/audit-html.ts runs over dist, run here over the |
| @@ -143,3 +143,40 @@ describe('each rule', () => { | ||
| 143 | 143 | expect(rules('')).not.toContain('no-lang') |
| 144 | 144 | }) |
| 145 | 145 | }) |
| 146 | ||
| 147 | describe('classes the markup uses and no stylesheet defines', () => { | |
| 148 | const sheets = readdirSync('resources/assets/styles/site') | |
| 149 | .filter(name => name.endsWith('.css')) | |
| 150 | .map(name => join('resources/assets/styles/site', name)) | |
| 151 | const defined = definedClasses(sheets) | |
| 152 | ||
| 153 | it('reads the real sheets, not an empty set', () => { | |
| 154 | expect(defined.size).toBeGreaterThan(300) | |
| 155 | expect(defined.has('kicker')).toBe(true) | |
| 156 | expect(defined.has('h3')).toBe(true) | |
| 157 | }) | |
| 158 | ||
| 159 | it('names a class nothing defines', () => { | |
| 160 | const findings = unstyledClasses('/t', '<p class="kicker made-up-class">x</p>', defined) | |
| 161 | expect(findings.map(finding => finding.detail)).toEqual(['made-up-class']) | |
| 162 | }) | |
| 163 | ||
| 164 | it('names it once however often it appears', () => { | |
| 165 | const html = '<p class="made-up-class"></p><p class="made-up-class"></p>' | |
| 166 | expect(unstyledClasses('/t', html, defined)).toHaveLength(1) | |
| 167 | }) | |
| 168 | ||
| 169 | it('ignores the scope and wrapper names stx writes per render', () => { | |
| 170 | const html = '<p class="lsu-sz7xmlte stx_race_portal_2_1r3g0n"></p>' | |
| 171 | expect(unstyledClasses('/t', html, defined)).toEqual([]) | |
| 172 | }) | |
| 173 | ||
| 174 | it('allows the hooks that are on the ratchet, and nothing else', () => { | |
| 175 | expect(unstyledClasses('/t', '<div class="race-board"></div>', defined)).toEqual([]) | |
| 176 | expect(unstyledClasses('/t', '<div class="race-boardx"></div>', defined)).toHaveLength(1) | |
| 177 | }) | |
| 178 | ||
| 179 | it('does not look inside a script', () => { | |
| 180 | expect(unstyledClasses('/t', '<script>el.className = "made-up-class"</script>', defined)).toEqual([]) | |
| 181 | }) | |
| 182 | }) | |