mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
545bb46b82
## Summary
On a Windows checkout the desktop quality gate does not work. This fixes
four defects in it. Two checks report success without examining
anything, one fails on every file, and one reports violations that its
own allowlist already covers.
**1. `pnpm test` finds no tests and still exits 0.** The `test` script
quotes the glob with single quotes. On Windows pnpm runs scripts through
`cmd.exe`, which does not strip single quotes, so node receives them as
part of the pattern and matches nothing. The run prints `# tests 0` and
exits 0 — a silent green. Double quotes are stripped by `cmd.exe` and by
POSIX shells alike, so Linux CI behaviour is unchanged.
**2. Every text file is checked out as CRLF.** There is no
`.gitattributes`, and `core.autocrlf=true` is the Git for Windows
default. Biome formats with LF, so `biome check .` fails on 1632 of 1633
files. `desktop/src/features/messages/ui/virtuaWheelModePatch.test.mjs`
fails too, because it matches `patches/*.patch` with `\n`-joined
patterns. The stored blobs are already LF, so `eol=lf` adds no
renormalisation churn — `git status` stays clean after the change.
**3. `check:px-text` never finds its own allowlist.**
`scripts/check-px-text-core.mjs` builds the key from `path.relative`,
which returns `\` separators on Windows, while the allowlist in
`desktop/scripts/check-px-text.mjs` is written with `/`. Nothing
matches, so the check reports 5 false violations on a clean tree.
**4. `check:file-sizes` examines nothing at all.** `findRule` compares
against `` `${rule.root}${path.sep}` ``. The roots are multi-segment
(`src/app`, `src/features`, `src-tauri/src`), so on Windows `src/app\`
never matches `src\app\...`. No rule matches any file: the check walks 0
of 1097 files and exits 0.
`scripts/check-pubkey-truncation-core.mjs` already normalises paths this
way (`relativePath.split(path.sep).join("/")`). This applies the same
idiom to the other two.
### Related issue
None found — no open issue covers this. The closest open PR is #2758,
which fixes a fifth Windows defect in `desktop/test-loader-hooks.mjs`;
it is required before the desktop unit tests can pass here, and it does
not overlap with these files. I checked the changed-file list of every
open PR: none touch `.gitattributes`, `desktop/package.json`,
`scripts/check-px-text-core.mjs` or `scripts/check-file-sizes-core.mjs`.
### Testing
Windows 11 (10.0.26200), node 22.17.1, pnpm 11.4.0, clean checkout with
the default `core.autocrlf=true`.
| Command | Before | After |
| --- | --- | --- |
| `pnpm test` | `# tests 0`, exit 0 | 374 test files discovered, exit 1
|
| `biome check .` | 1632 of 1633 files fail | 1633 checked, 0 errors |
| `pnpm check:px-text` | 5 false violations | passes |
| `pnpm check:file-sizes` | 0 of 1097 files examined, exit 0 | 1097
examined |
`check:file-sizes` now reports `src-tauri/src/managed_agents/runtime.rs:
2220 lines (limit 2216)`. That violation is pre-existing and not
introduced here — `main` currently fails on the same line in CI (Desktop
Core, run 30185213010, commit c2a4ee7). Before this change Windows
reported success while CI was red; now the Windows result agrees with
CI.
Desktop unit tests still fail on Windows until #2758 lands. With #2758
applied on top of this branch the full suite passes: 3515 tests, 0
failures. This change stops hiding those failures rather than fixing
them.
Signed-off-by: Seydi Charyyev <seydi.charyev@gmail.com>
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
130 lines
4.7 KiB
JavaScript
130 lines
4.7 KiB
JavaScript
import { promises as fs } from "node:fs";
|
|
import path from "node:path";
|
|
|
|
/**
|
|
* Shared "no hardcoded px text size" guard.
|
|
*
|
|
* Zoom (Cmd +/-) scales the root <html> font-size, so only **rem**-based text
|
|
* scales. Hardcoded px text sizes (`text-[15px]`, `font-size: 15px`) freeze
|
|
* against zoom — that's the timeline regression we fixed. This guard stops new
|
|
* px text sizes from creeping back in. Use a rem-based Tailwind token instead
|
|
* (e.g. the stock `text-base`, `text-sm`, `text-xs` scale — chat === base).
|
|
*
|
|
* It flags:
|
|
* - Tailwind arbitrary text-size utilities: `text-[NNpx]`, `text-[N.NNrem]`,
|
|
* `text-[N.NNem]` — any arbitrary font-size literal. Use a named token
|
|
* (`text-2xs`, `text-3xs`, or the stock `text-base`/`text-sm`/`text-xs`
|
|
* scale) so the size lives in `tailwind.config.js` as rem and stays
|
|
* consistent. px literals freeze against zoom; arbitrary rem literals
|
|
* re-fragment the scale we just consolidated.
|
|
* - CSS px font sizes: `font-size: NNpx`
|
|
*
|
|
* Decorative/chrome exceptions (avatar initials sized to a fixed avatar box,
|
|
* the `text-[6rem]` emoji glyph, etc.) live in the `overrides` allowlist
|
|
* supplied by each app.
|
|
*/
|
|
|
|
// Any arbitrary Tailwind text-size literal — px, rem, or em. Color literals
|
|
// like `text-[#fff]` or `text-[var(--x)]` don't match (no unit-bearing number).
|
|
const TEXT_ARBITRARY_RE = /\btext-\[\d+(?:\.\d+)?(?:px|rem|em)\]/g;
|
|
// Match the CSS `font-size` property, but NOT custom properties like
|
|
// `--font-size:` (third-party widget vars) which merely contain the substring.
|
|
const FONT_SIZE_PX_RE = /(?<!-)\bfont-size:\s*\d+(?:\.\d+)?px/g;
|
|
|
|
async function walkFiles(directory) {
|
|
const entries = await fs.readdir(directory, { withFileTypes: true });
|
|
const files = await Promise.all(
|
|
entries.map(async (entry) => {
|
|
const fullPath = path.join(directory, entry.name);
|
|
if (entry.isDirectory()) {
|
|
return walkFiles(fullPath);
|
|
}
|
|
return [fullPath];
|
|
}),
|
|
);
|
|
return files.flat();
|
|
}
|
|
|
|
/**
|
|
* @param {object} options
|
|
* @param {string} options.projectRoot Absolute path the rule roots resolve against.
|
|
* @param {Array<{root: string, extensions: Set<string>}>} options.rules Where to scan.
|
|
* @param {string} options.label Human label for the failure header.
|
|
* @param {Set<string>} [options.overrides] Allowlisted "relativePath:matchedLiteral" entries.
|
|
* @param {string} options.scriptPath Path mentioned in the failure hint.
|
|
*/
|
|
export async function runPxTextCheck({
|
|
projectRoot,
|
|
rules,
|
|
label,
|
|
overrides = new Set(),
|
|
scriptPath,
|
|
}) {
|
|
const candidateFiles = (
|
|
await Promise.all(
|
|
rules.map((rule) => {
|
|
const dir = path.join(projectRoot, rule.root);
|
|
return fs
|
|
.access(dir)
|
|
.then(() => walkFiles(dir))
|
|
.catch(() => []);
|
|
}),
|
|
)
|
|
).flat();
|
|
|
|
const violations = [];
|
|
|
|
for (const filePath of candidateFiles) {
|
|
// `rules[].root` and the `overrides` keys are authored with `/`, but
|
|
// path.relative yields `\` on Windows — so every comparison against them
|
|
// has to happen in posix form or it silently matches nothing.
|
|
const relativePath = path
|
|
.relative(projectRoot, filePath)
|
|
.split(path.sep)
|
|
.join("/");
|
|
const rule = rules.find((r) => relativePath.startsWith(`${r.root}/`));
|
|
if (!rule) {
|
|
continue;
|
|
}
|
|
if (!rule.extensions.has(path.extname(relativePath))) {
|
|
continue;
|
|
}
|
|
// Optional per-rule basename allowlist — scopes the scan to specific files.
|
|
if (rule.files && !rule.files.has(path.basename(relativePath))) {
|
|
continue;
|
|
}
|
|
|
|
const content = await fs.readFile(filePath, "utf8");
|
|
const lines = content.split(/\r?\n/);
|
|
lines.forEach((line, index) => {
|
|
const lineNumber = index + 1;
|
|
const matches = [
|
|
...(line.match(TEXT_ARBITRARY_RE) ?? []),
|
|
...(line.match(FONT_SIZE_PX_RE) ?? []),
|
|
];
|
|
for (const match of matches) {
|
|
const key = `${relativePath}:${match}`;
|
|
if (!overrides.has(key)) {
|
|
violations.push({ relativePath, lineNumber, match });
|
|
}
|
|
}
|
|
});
|
|
}
|
|
|
|
if (violations.length > 0) {
|
|
console.error(`${label} px-text check failed:`);
|
|
for (const v of violations) {
|
|
console.error(`- ${v.relativePath}:${v.lineNumber}: ${v.match}`);
|
|
}
|
|
console.error(
|
|
"Use a rem-based Tailwind text token (e.g. the stock `text-base`, " +
|
|
"`text-sm`, `text-xs` scale, or the `text-2xs` / `text-3xs` meta-text " +
|
|
"tokens) so the text scales with Cmd +/- zoom and stays on one scale. " +
|
|
"If this size is " +
|
|
"genuinely decorative/chrome (not readable message text), add a " +
|
|
`narrowly scoped \`relativePath:matchedLiteral\` exception in \`${scriptPath}\`.`,
|
|
);
|
|
process.exit(1);
|
|
}
|
|
}
|