Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
11ba7b3180 | ||
|
|
7f8e185d7c | ||
|
|
42483c4b61 |
@@ -130,7 +130,13 @@ reference, because you need them *before* the failure, not after.
|
||||
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.
|
||||
which its own warning had been read twice. **The same command carries
|
||||
a second CSS check**: a nested rule whose selector starts with an
|
||||
element name (`audio-player { … }` rather than `& audio-player { … }`)
|
||||
is silently dropped by the device's Chrome 113 and by nothing else, so
|
||||
every tier you can run renders it correctly. Run it after touching
|
||||
`index.css` or any `css` literal; a rule directly inside a top-level
|
||||
`@media` is not nested and is not flagged.
|
||||
- **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:
|
||||
|
||||
@@ -3496,6 +3496,27 @@ android-inspect` forwards the WebView's devtools socket and `make
|
||||
android-eval` asks the real page — raw CDP, because `connectOverCDP`
|
||||
calls `Browser.setDownloadBehavior` and a WebView refuses it.
|
||||
|
||||
**One of those gaps is checked rather than remembered.** A nested rule
|
||||
whose selector starts with an element name is not a parse error anyone
|
||||
would notice on 113 — the rule simply does not exist, there and nowhere
|
||||
else, which is how the bottom bar's `text-overflow: ellipsis` came to
|
||||
have never truncated on the device. `make css-check`
|
||||
(`frontend/scripts/check-css-nesting.mjs`, a pre-commit hook and a CI
|
||||
step) fails on one, over every `frontend/*.css` and the `css` literals
|
||||
alike — a glob rather than `index.css` by name, because the hook fires
|
||||
on `frontend/**/*.{ts,css}` and a sweep that names one file goes green
|
||||
over a stylesheet it never opened — and
|
||||
says the fix is a leading `&` — valid in both syntaxes, so no nested
|
||||
rule here has a reason to omit it. Two things it has to get right, and
|
||||
both follow from asking whether a *style* rule is anywhere above rather
|
||||
than what the immediate parent is: `@media (…) { bottom-nav { … } }` at
|
||||
the top level is an ordinary rule and is the majority of what a regex
|
||||
over the file would report, while the same rule one level inside
|
||||
`.bar { @media (…) { … } }` is nested and is flagged. The check is the
|
||||
cheap version of the answer; a build-time downlevel (Lightning CSS
|
||||
targeting 113) would fix the class permanently and is a dependency and
|
||||
a build step rather than twenty lines.
|
||||
|
||||
`build/config.yml`'s `version` is the
|
||||
*metadata* version and is not what the app reports — `main.version` is
|
||||
stamped at link time from the packaging recipe's git-derived version.
|
||||
|
||||
@@ -172,12 +172,17 @@ ui-setup: ## Install the Vitest browser provider's own Chromium (once)
|
||||
bindings-check: ## Fail if the generated bindings are stale
|
||||
@./scripts/bindings-check.sh
|
||||
|
||||
# Two CSS traps that report a long way from their cause, or not at all.
|
||||
# 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.
|
||||
# the suite failing to import. Four sessions, three plans. And a nested
|
||||
# rule starting with an element name is dropped by the device's
|
||||
# Chrome 113 in silence -- no tier here runs an engine that can see it.
|
||||
# Instant.
|
||||
.PHONY: css-check
|
||||
css-check: ## Fail if a css`` literal was ended early by a backtick in a comment
|
||||
css-check: ## Fail on a css`` literal ended early by a backtick, or a nested rule needing an &
|
||||
@cd frontend && node scripts/check-css-literals.mjs
|
||||
@cd frontend && node scripts/check-css-nesting.mjs
|
||||
|
||||
# .pi/ and CLAUDE.md document commands, and a doc that documents a
|
||||
# command wrongly is worse than no doc: an agent runs it confidently.
|
||||
|
||||
@@ -23,97 +23,23 @@
|
||||
* 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';
|
||||
import { globSync, readFileSync } from 'node:fs';
|
||||
|
||||
import { taggedLiterals } from './css-literals.mjs';
|
||||
|
||||
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));
|
||||
for (const { tag, body, line } of taggedLiterals(src, TAGS)) {
|
||||
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 (opens > closes) problems.push({ file, line, tag });
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,79 @@
|
||||
#!/usr/bin/env node
|
||||
/**
|
||||
* Fail on a nested rule whose selector starts with an element name.
|
||||
*
|
||||
* See `css-nesting.mjs` for what the phone does with one. No tier here
|
||||
* can see it: the component tier, the e2e tier and `make ui-visual` all
|
||||
* run a current Chromium, where the rule applies normally, so the only
|
||||
* report is a screenshot of the device — which is how the bottom bar's
|
||||
* title came to have never truncated there.
|
||||
*
|
||||
* It covers `index.css` and the `css` literals in the components alike,
|
||||
* because a shadow-root stylesheet is parsed by the same engine.
|
||||
*/
|
||||
import { globSync, readFileSync } from 'node:fs';
|
||||
|
||||
import { taggedLiterals } from './css-literals.mjs';
|
||||
import { findBareNestedRules } from './css-nesting.mjs';
|
||||
|
||||
const problems = [];
|
||||
|
||||
// Every stylesheet, not `index.css` by name: the hook that runs this
|
||||
// fires on `frontend/**/*.{ts,css}`, so naming one file promises a
|
||||
// coverage the sweep does not deliver -- a second stylesheet would be
|
||||
// silently unswept while the hook still went green over it. There is
|
||||
// only `index.css` today, which is exactly when this is free to fix.
|
||||
const stylesheets = globSync('*.css', { cwd: process.cwd() });
|
||||
|
||||
if (stylesheets.length === 0) {
|
||||
console.error('css-nesting-check: no stylesheet matched *.css');
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
for (const file of stylesheets) {
|
||||
for (const { line, selector } of findBareNestedRules(
|
||||
readFileSync(file, 'utf8'),
|
||||
)) {
|
||||
problems.push({ file, line, selector });
|
||||
}
|
||||
}
|
||||
|
||||
const sources = globSync('src/**/*.ts', { cwd: process.cwd() });
|
||||
|
||||
// A sweep over an empty glob passes, and this one is expected to find
|
||||
// nothing, so "it found nothing" has to mean it looked.
|
||||
if (sources.length === 0) {
|
||||
console.error('css-nesting-check: no sources matched src/**/*.ts');
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
for (const file of sources) {
|
||||
const src = readFileSync(file, 'utf8');
|
||||
|
||||
for (const literal of taggedLiterals(src, ['css'])) {
|
||||
for (const { line, selector } of findBareNestedRules(literal.body)) {
|
||||
problems.push({ file, line: literal.line + line - 1, selector });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (problems.length > 0) {
|
||||
for (const p of problems) {
|
||||
console.error(
|
||||
`${p.file}:${p.line}: nested rule "${p.selector.split('\n')[0]}" starts ` +
|
||||
'with an element name — write it as "& ' +
|
||||
`${p.selector.split('\n')[0]}"`,
|
||||
);
|
||||
}
|
||||
|
||||
console.error(
|
||||
`\ncss-nesting-check: ${problems.length} problem(s). ` +
|
||||
'Chrome 113 (the device) drops a nested rule that does not start ' +
|
||||
'with a symbol; the leading & is valid in both syntaxes.',
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
console.log(
|
||||
`css-nesting-check: ${stylesheets.length} stylesheet(s) + ${sources.length} files, no bare nested rules`,
|
||||
);
|
||||
@@ -0,0 +1,103 @@
|
||||
/**
|
||||
* Finding the `css` tagged templates in a TypeScript source.
|
||||
*
|
||||
* Two checks read them — the unterminated-comment one and the nesting
|
||||
* one — and a second scanner would be a second thing to keep in step
|
||||
* with how a template literal actually ends.
|
||||
*/
|
||||
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
export 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.
|
||||
*
|
||||
* Newlines inside them are kept, so a line number taken from the
|
||||
* stripped text still names the right line of the file it came from.
|
||||
*/
|
||||
export 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];
|
||||
else if (text[i] === '\n') out += '\n';
|
||||
}
|
||||
|
||||
return out;
|
||||
}
|
||||
|
||||
/** The 1-based line number of `index` in `src`. */
|
||||
export function lineOf(src, index) {
|
||||
return src.slice(0, index).split('\n').length;
|
||||
}
|
||||
|
||||
/**
|
||||
* Every tagged template literal in `src` whose tag is in `tags`.
|
||||
*
|
||||
* `body` has its substitutions stripped and `line` is the line its
|
||||
* opening backtick sits on, so `line + (n - 1)` is the file line of the
|
||||
* body's own line `n`.
|
||||
*/
|
||||
export function taggedLiterals(src, tags) {
|
||||
const pattern = new RegExp(`(^|[^\\w$.])(${tags.join('|')})\``, 'g');
|
||||
const found = [];
|
||||
|
||||
let match;
|
||||
|
||||
while ((match = pattern.exec(src)) !== null) {
|
||||
const open = match.index + match[0].length - 1;
|
||||
const close = endOfTemplate(src, open);
|
||||
|
||||
if (close === -1) continue;
|
||||
|
||||
found.push({
|
||||
tag: match[2],
|
||||
body: stripSubstitutions(src.slice(open + 1, close)),
|
||||
line: lineOf(src, open),
|
||||
});
|
||||
}
|
||||
|
||||
return found;
|
||||
}
|
||||
@@ -0,0 +1,119 @@
|
||||
/**
|
||||
* A nested rule whose selector starts with an element name is silently
|
||||
* dropped on the phone.
|
||||
*
|
||||
* The device renders in Chrome 113, which predates relaxed CSS nesting
|
||||
* (Chrome 120): before that a nested selector had to start with
|
||||
* something that could not be read as the beginning of a declaration,
|
||||
* so `.bottom-bar { audio-player { … } }` is not a parse error anyone
|
||||
* would notice — the inner rule simply does not exist, on the phone and
|
||||
* only on the phone. Three were live in `index.css`, one of them the
|
||||
* `text-overflow: ellipsis` on the bottom bar's title, which had
|
||||
* therefore never truncated on the device.
|
||||
*
|
||||
* `& audio-player` is valid in both syntaxes, so no nested rule here
|
||||
* has any reason to omit it.
|
||||
*
|
||||
* Two things the detection has to get right:
|
||||
*
|
||||
* - **A rule directly inside an at-rule is not nested.**
|
||||
* `@media (…) { bottom-nav { … } }` at the top level is an ordinary
|
||||
* rule and is fine — and it is the majority of the matches a regex
|
||||
* over the file would produce. What decides it is whether a *style*
|
||||
* rule is somewhere above, not what the immediate parent is: inside
|
||||
* `.bar { @media (…) { audio-player { … } } }` the inner rule is
|
||||
* nested, at-rule in between or not.
|
||||
* - **A declaration is not a rule.** `background: url(…)` and any
|
||||
* string or comment can hold a brace, so this tracks them rather than
|
||||
* matching lines.
|
||||
*/
|
||||
|
||||
/** Does this selector start with an identifier, rather than a symbol? */
|
||||
function startsWithIdent(selector) {
|
||||
return /^[A-Za-z_\u00A0-\uFFFF]/.test(selector);
|
||||
}
|
||||
|
||||
/**
|
||||
* Every nested style rule in `css` whose selector starts with an
|
||||
* element name, as `{ line, selector }` with a 1-based line.
|
||||
*/
|
||||
export function findBareNestedRules(css) {
|
||||
const found = [];
|
||||
/** The blocks we are inside, innermost last: 'style' or 'at'. */
|
||||
const stack = [];
|
||||
/** The text since the last `{`, `}` or `;` — a prelude, if a `{` follows. */
|
||||
let prelude = '';
|
||||
let preludeLine = 1;
|
||||
let line = 1;
|
||||
|
||||
const startPrelude = () => {
|
||||
prelude = '';
|
||||
preludeLine = line;
|
||||
};
|
||||
|
||||
for (let i = 0; i < css.length; i++) {
|
||||
const c = css[i];
|
||||
|
||||
if (c === '\n') {
|
||||
line++;
|
||||
if (prelude.trim() === '') preludeLine = line;
|
||||
prelude += c;
|
||||
continue;
|
||||
}
|
||||
|
||||
if (c === '/' && css[i + 1] === '*') {
|
||||
const end = css.indexOf('*/', i + 2);
|
||||
const comment = css.slice(i, end === -1 ? css.length : end + 2);
|
||||
|
||||
line += (comment.match(/\n/g) ?? []).length;
|
||||
i += comment.length - 1;
|
||||
if (prelude.trim() === '') preludeLine = line;
|
||||
continue;
|
||||
}
|
||||
|
||||
if (c === '"' || c === "'") {
|
||||
let j = i + 1;
|
||||
|
||||
while (j < css.length && css[j] !== c) {
|
||||
if (css[j] === '\\') j++;
|
||||
j++;
|
||||
}
|
||||
|
||||
prelude += css.slice(i, j + 1);
|
||||
i = j;
|
||||
continue;
|
||||
}
|
||||
|
||||
if (c === '{') {
|
||||
const selector = prelude.trim();
|
||||
const kind = selector.startsWith('@') ? 'at' : 'style';
|
||||
|
||||
if (
|
||||
kind === 'style' &&
|
||||
stack.includes('style') &&
|
||||
startsWithIdent(selector)
|
||||
) {
|
||||
found.push({ line: preludeLine, selector });
|
||||
}
|
||||
|
||||
stack.push(kind);
|
||||
startPrelude();
|
||||
continue;
|
||||
}
|
||||
|
||||
if (c === '}') {
|
||||
stack.pop();
|
||||
startPrelude();
|
||||
continue;
|
||||
}
|
||||
|
||||
if (c === ';') {
|
||||
startPrelude();
|
||||
continue;
|
||||
}
|
||||
|
||||
prelude += c;
|
||||
}
|
||||
|
||||
return found;
|
||||
}
|
||||
@@ -0,0 +1,79 @@
|
||||
/**
|
||||
* The nesting check's own semantics.
|
||||
*
|
||||
* `make css-check` runs it over a tree that currently has no violation,
|
||||
* so the check passing says nothing about whether it can still find
|
||||
* one. What it has to get right is two distinctions, and both are the
|
||||
* kind a regex over the file gets wrong: a rule directly inside an
|
||||
* at-rule is not nested, and a brace inside a string or a comment is
|
||||
* not a block.
|
||||
*
|
||||
* The rule it enforces is the device's: Chrome 113 predates relaxed CSS
|
||||
* nesting, so a nested selector starting with an element name is
|
||||
* dropped in silence. See `scripts/css-nesting.mjs`.
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import { findBareNestedRules } from '../../scripts/css-nesting.mjs';
|
||||
|
||||
describe('the nested-rule check', () => {
|
||||
it('flags a nested rule that starts with an element name', () => {
|
||||
const found = findBareNestedRules(
|
||||
'.bottom-bar {\n color: red;\n\n audio-player { margin: 0 }\n}',
|
||||
);
|
||||
|
||||
expect(found).toEqual([{ line: 4, selector: 'audio-player' }]);
|
||||
});
|
||||
|
||||
it('accepts the same rule written with a leading &', () => {
|
||||
expect(
|
||||
findBareNestedRules('.bottom-bar {\n & audio-player { margin: 0 }\n}'),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it('accepts a nested selector that starts with any other symbol', () => {
|
||||
expect(
|
||||
findBareNestedRules('.bar {\n #track-info { color: red }\n}'),
|
||||
).toEqual([]);
|
||||
expect(findBareNestedRules('.bar {\n :host { color: red }\n}')).toEqual(
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
it('leaves a top-level rule alone, element name or not', () => {
|
||||
expect(findBareNestedRules('p {\n margin: 0;\n}')).toEqual([]);
|
||||
});
|
||||
|
||||
/**
|
||||
* The majority of what a naive sweep would report: a media query at
|
||||
* the top level holds ordinary rules, not nested ones.
|
||||
*/
|
||||
it('leaves a rule directly inside an at-rule alone', () => {
|
||||
expect(
|
||||
findBareNestedRules(
|
||||
'@media (max-width: 599px) {\n bottom-nav { display: flex }\n}',
|
||||
),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
/**
|
||||
* And the other half of that: what decides it is whether a style rule
|
||||
* is anywhere above, not what the immediate parent is.
|
||||
*/
|
||||
it('flags one inside an at-rule that is itself inside a rule', () => {
|
||||
expect(
|
||||
findBareNestedRules(
|
||||
'.bar {\n @media (min-width: 900px) {\n audio-player { margin: 0 }\n }\n}',
|
||||
),
|
||||
).toEqual([{ line: 3, selector: 'audio-player' }]);
|
||||
});
|
||||
|
||||
it('reads through a brace in a string or a comment', () => {
|
||||
expect(
|
||||
findBareNestedRules('.a {\n background: url("x{y}");\n}'),
|
||||
).toEqual([]);
|
||||
expect(
|
||||
findBareNestedRules('.a {\n /* audio-player { x: y } */\n}'),
|
||||
).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -58,6 +58,15 @@ pre-commit:
|
||||
root: "frontend/"
|
||||
run: node scripts/check-css-literals.mjs
|
||||
|
||||
# A nested rule whose selector starts with an element name is
|
||||
# silently dropped by the device's Chrome 113, and by nothing else --
|
||||
# so every tier here renders it correctly and only a screenshot of
|
||||
# the phone disagrees. Instant.
|
||||
css-nesting:
|
||||
glob: "frontend/**/*.{ts,css}"
|
||||
root: "frontend/"
|
||||
run: node scripts/check-css-nesting.mjs
|
||||
|
||||
# Deliberately sequential, unlike pre-commit. `go test -race`
|
||||
# saturates every core for the better part of a minute and the UI tier
|
||||
# is a real browser with wall-clock timeouts, so run together the
|
||||
|
||||
Reference in New Issue
Block a user