diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index ae789f6..34b4ad8 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -155,6 +155,16 @@ jobs: working-directory: /src/frontend run: npx tsc --noEmit + # A backtick inside a comment in a css`` literal ends the literal. + # tsc above does fail on it, with a message about CSSResult + # pointing at a line of prose; this one names the cause. It runs + # after tsc for exactly that reason — whichever fails, the log has + # the sentence in it. + - name: CSS template literals are intact + if: ${{ !cancelled() }} + working-directory: /src + run: make css-check + - name: Component and store suite working-directory: /src run: make ui-test diff --git a/.pi/skills/yellowjacket-dev/SKILL.md b/.pi/skills/yellowjacket-dev/SKILL.md index 4411d21..ed30b03 100644 --- a/.pi/skills/yellowjacket-dev/SKILL.md +++ b/.pi/skills/yellowjacket-dev/SKILL.md @@ -97,6 +97,12 @@ reference, because you need them *before* the failure, not after. old behaviour. `make dev-headless` prints the esbuild error; a reload does not. One way to cause one is a stray backtick inside a comment in a `css` tagged template literal, which ends the literal. + **That one is a check now** — `make css-check` (instant, a pre-commit + hook and a CI step) names the file, the line and the cause, because + what you otherwise get is `Property 'scroll' does not exist on type + 'CSSResult'` pointing at a line of prose, or every test in the suite + failing to import. It went in after the trap cost a fourth session in + which its own warning had been read twice. - **A failing CI job's log is reachable even when `gitea_ci job_logs` says it is not.** That endpoint 404s on this Gitea build. The REST API answers, with the `GITEA_TOKEN` already in the environment: @@ -162,7 +168,8 @@ Two rules about climbing: do not exist. Before a commit, the gate is `make lint`, `make test`, `make ui-test`, -`make bindings-check` and — from `frontend/` — `npx tsc --noEmit`. The +`make bindings-check`, `make css-check` and — from `frontend/` — +`npx tsc --noEmit`. The first four are lefthook hooks, so skipping them locally only defers the failure; the typecheck is a hook too but only CI runs it over the test tree, which is where it has actually broken. diff --git a/Makefile b/Makefile index f26e82f..55e45b2 100644 --- a/Makefile +++ b/Makefile @@ -107,6 +107,13 @@ ui-setup: ## Install the Vitest browser provider's own Chromium (once) bindings-check: ## Fail if frontend/wailsjs is stale against the Go bindings @./scripts/bindings-check.sh +# A backtick inside a comment in a css`` literal ends the literal, and +# what you get back is a type error about CSSResult, or every test in +# the suite failing to import. Four sessions, three plans. Instant. +.PHONY: css-check +css-check: ## Fail if a css`` literal was ended early by a backtick in a comment + @cd frontend && node scripts/check-css-literals.mjs + # .pi/ documents commands, and a skill that documents a command wrongly # is worse than no skill: an agent runs it confidently. Every command # in there is a make target on purpose, so this is checkable. diff --git a/frontend/scripts/check-css-literals.mjs b/frontend/scripts/check-css-literals.mjs new file mode 100644 index 0000000..c8738e3 --- /dev/null +++ b/frontend/scripts/check-css-literals.mjs @@ -0,0 +1,136 @@ +#!/usr/bin/env node +/** + * A backtick inside a comment in a `css` tagged template literal ends + * the literal. + * + * This has cost four sessions across three plans. It is written down in + * CLAUDE.md, in the yellowjacket-dev skill and in NOTES.md, and it was + * read twice in the session it then cost a cycle in — so it is a check + * now rather than a fourth paragraph. Knowledge that has been ignored + * three times is not a knowledge problem. + * + * What makes it expensive is not the mistake but the *report*. The + * literal ends early, the rest of the CSS is parsed as JavaScript, and + * what comes back is `Expected "]" but found "inline"` pointing at a + * line of prose — or, when it happens in a shared module like + * `tokens.css.ts`, every test in the suite failing to import and an + * output that reads like a broken test runner. And `make dev-headless` + * leaves the dev server serving the last good bundle, so the page still + * works and still shows the old behaviour. + * + * The detection is exact rather than heuristic. If a backtick inside a + * comment closed the literal early, then the text the parser *did* take + * as the literal contains an unterminated `/*`. Nothing else produces + * that, and a legitimate literal cannot contain one. + */ +import { readFileSync } from 'node:fs'; +import { globSync } from 'node:fs'; + +const TAGS = ['css', 'html', 'svg']; + +/** + * Find the end of a template literal that starts at `start` (the index + * of its opening backtick), respecting escapes and `${}` substitutions. + * Returns the index of the closing backtick, or -1. + */ +function endOfTemplate(src, start) { + let depth = 0; + + for (let i = start + 1; i < src.length; i++) { + const c = src[i]; + + if (c === '\\') { + i++; + continue; + } + + if (c === '$' && src[i + 1] === '{') { + depth++; + i++; + continue; + } + + if (c === '}' && depth > 0) { + depth--; + continue; + } + + if (c === '`' && depth === 0) return i; + } + + return -1; +} + +/** Strip `${...}` substitutions, which may legitimately contain anything. */ +function stripSubstitutions(text) { + let out = ''; + let depth = 0; + + for (let i = 0; i < text.length; i++) { + if (text[i] === '$' && text[i + 1] === '{') { + depth++; + i++; + continue; + } + + if (text[i] === '}' && depth > 0) { + depth--; + continue; + } + + if (depth === 0) out += text[i]; + } + + return out; +} + +function lineOf(src, index) { + return src.slice(0, index).split('\n').length; +} + +const files = globSync('src/**/*.ts', { cwd: process.cwd() }); +const problems = []; + +for (const file of files) { + const src = readFileSync(file, 'utf8'); + const tagPattern = new RegExp(`(^|[^\\w$.])(${TAGS.join('|')})\``, 'g'); + + let match; + + while ((match = tagPattern.exec(src)) !== null) { + const open = match.index + match[0].length - 1; + const close = endOfTemplate(src, open); + + if (close === -1) continue; + + const body = stripSubstitutions(src.slice(open + 1, close)); + const opens = (body.match(/\/\*/g) ?? []).length; + const closes = (body.match(/\*\//g) ?? []).length; + + if (opens > closes) { + problems.push({ + file, + line: lineOf(src, open), + tag: match[2], + }); + } + } +} + +if (problems.length > 0) { + for (const p of problems) { + console.error( + `${p.file}:${p.line}: unterminated /* inside a ${p.tag}\`\` literal — ` + + 'a backtick in a comment ends the literal early', + ); + } + + console.error( + `\ncss-literal-check: ${problems.length} problem(s). ` + + 'Remove the backticks from the comment; markdown quoting does not ' + + 'survive a tagged template.', + ); + process.exit(1); +} + +console.log(`css-literal-check: ${files.length} files, no broken literals`); diff --git a/lefthook.yml b/lefthook.yml index dc64500..7acb02a 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -47,6 +47,15 @@ pre-commit: root: "frontend/" run: ./node_modules/.bin/tsc --noEmit + # A backtick in a comment inside a css`` literal ends the literal. + # tsc does catch it, as "Class static side incorrectly extends base + # class static side" pointing at a line of prose; this says what + # actually happened. Instant. + css-literals: + glob: "frontend/**/*.ts" + root: "frontend/" + run: node scripts/check-css-literals.mjs + pre-push: parallel: true commands: