From 28931dca42455cf31936974a4c92be32cc951dbb Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 5 Oct 2026 23:05:56 +0000 Subject: [PATCH] fix: make dev rebuilds dist/ on source changes (closes #332) make dev passed --watch to a build.js that read no arguments, so it built once and exited. build.js --watch now builds, then builds again after every change to a file under src/, manifest/ or icons/, until interrupted; any other argument fails. Each directory gets its own watcher, because Node's recursive watch on Linux loses a file that an editor saves by renaming a new copy over it. A watch build writes no build receipt and cannot be verified; README.md and the Makefile say so and point to make build. Model: opus-5-5 --- Makefile | 8 +++ README.md | 10 +++- TODO.md | 11 ++++ build.js | 110 +++++++++++++++++++++++++++++++++------ tests/buildWatch.test.js | 104 ++++++++++++++++++++++++++++++++++++ 5 files changed, 227 insertions(+), 16 deletions(-) create mode 100644 tests/buildWatch.test.js diff --git a/Makefile b/Makefile index 881f8e6..59e2422 100644 --- a/Makefile +++ b/Makefile @@ -107,6 +107,14 @@ vendor-blocklist: clean: @rm -rf dist/ release/ +# Run the build make build runs, without the checks that follow it, then run it +# again after every change to a file under src/, manifest/ or icons/, until +# interrupted. A failed build is reported, may leave dist/ partly written, and +# watching carries on. It writes no build receipt, so nothing can verify what it +# leaves in dist/: anything handed on comes from make build. A release build +# unless AUTISTMASK_DEBUG=1 is exported. A change anywhere else, package.json +# and build.js included, starts no build, and a directory created while it runs +# is not watched; restart it after either. dev: @echo "Building in watch mode..." @yarn run build --watch 2>&1 diff --git a/README.md b/README.md index ab3472f..cabe6f4 100644 --- a/README.md +++ b/README.md @@ -306,7 +306,15 @@ The Makefile shims to those. It also carries a few targets that have no debug build, and keeping its `dist/` on failure (see [Debug Builds](#debug-builds)) - `make clean` — remove `dist/` and `release/` -- `make dev` — build in watch mode +- `make dev` — run the build `make build` runs, without the checks that follow + it, then run it again after every change to a file under `src/`, `manifest/` + or `icons/`, until interrupted. A failed build is reported, may leave `dist/` + partly written, and watching carries on. It writes no build receipt, so + nothing can verify what it leaves in `dist/` (see + [Build Receipts](#build-receipts)): anything handed on comes from + `make build`. A release build unless `AUTISTMASK_DEBUG=1` is exported. A + change anywhere else, `package.json` and `build.js` included, starts no build, + and a directory created while it runs is not watched; restart it after either ## End-to-End Tests diff --git a/TODO.md b/TODO.md index e9c0025..d20db25 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,17 @@ but the review is broader than any of them. # Completed Steps +- 2026-10-05: `make dev` watches + ([#332](https://git.eeqj.de/sneak/AutistMask/issues/332)). It used to pass + `--watch` to a `build.js` that read no arguments, so it built once and exited. + `build.js --watch` now builds, then builds again after every change to a file + under `src/`, `manifest/` or `icons/`; any other argument fails. It watches + each directory rather than using Node's recursive watch, which on Linux stops + seeing a file an editor saves by renaming a new copy over it. It writes no + build receipt, so nothing can verify what it builds; `make build` remains the + way to produce a `dist/` to hand on. `tests/buildWatch.test.js` covers the + watch loop against a temp directory. + - 2026-10-05: Every CI job has a `timeout-minutes` cap ([#294](https://git.eeqj.de/sneak/AutistMask/issues/294)): `check` 10 minutes, `e2e-firefox` 15 and `e2e-chrome` 20, each over two and a half times the job's diff --git a/build.js b/build.js index 20d553f..76accf9 100644 --- a/build.js +++ b/build.js @@ -15,6 +15,15 @@ const DIST_CHROME = path.join(DIST, "chrome"); const DIST_FIREFOX = path.join(DIST, "firefox"); const SRC = path.join(__dirname, "src"); +// What `make dev` watches: the directories whose files build() bundles, +// compiles or copies. A change anywhere else, package.json and build.js +// included, starts no build; restart make dev after one. +const WATCHED_DIRS = [ + SRC, + path.join(__dirname, "manifest"), + path.join(__dirname, "icons"), +]; + // The module whose compiled DEBUG state script/verify-build asserts. Which // bundles contain it is derived from esbuild's own dependency graph rather // than from a hardcoded list, so it tracks the bundle layout instead of @@ -441,6 +450,10 @@ function getBuildInfo() { async function build() { console.log("Building AutistMask extension..."); + // Under make dev this runs once per rebuild in the same process, and each + // build accounts only for what it emits itself. + emittedFiles.length = 0; + const receiptPath = receiptTarget(); if (!receiptPath) { console.warn( @@ -597,27 +610,94 @@ async function build() { console.log("Build complete: dist/chrome/ and dist/firefox/"); } -// Run only as a program. Required as a module — which is how -// tests/buildForbiddenInputs.test.js reaches the checks below — this file -// builds nothing and writes nothing. -if (require.main === module) { - build().catch((err) => { - console.error( - `Build failed: ${err && err.message ? err.message : err}`, - ); - process.exit(1); - }); +// make dev: build, then build again after every change under `dirs`, until +// interrupted. A failed build is reported and watching carries on, so a +// half-finished edit does not end it. A change that arrives while a build is +// running is not lost: it causes one more build as soon as that one finishes. +// A directory created after this starts is not watched until a restart. +// +// make dev sets no AUTISTMASK_BUILD_RECEIPT, so these builds write no receipt +// and nothing can verify what they leave in dist/. +// +// `rebuild` is build() everywhere but tests/buildWatch.test.js, which also +// closes the returned watchers. +function watch(dirs, rebuild) { + let building = false; + let changedAgain = false; + + async function run() { + if (building) { + changedAgain = true; + return; + } + building = true; + do { + // Let the rest of one save's events arrive first, so that one + // save is one build. + await new Promise((resolve) => setTimeout(resolve, 100)); + changedAgain = false; + try { + await rebuild(); + } catch (err) { + console.error( + `Build failed: ${err && err.message ? err.message : err}`, + ); + } + } while (changedAgain); + building = false; + console.log("Watching for changes (Ctrl-C to stop)..."); + } + + // One watcher per directory rather than fs.watch's recursive option: on + // Linux, Node's recursive watch watches each file, and stops seeing one + // that an editor saves by renaming a new copy over it. A directory's + // watcher reports every change to the files in it, however they are saved. + const subdirectories = dirs.flatMap((dir) => + fs + .readdirSync(dir, { recursive: true, withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => path.join(entry.parentPath, entry.name)), + ); + const watchers = [...dirs, ...subdirectories].map((dir) => + fs.watch(dir, run), + ); + run(); + return watchers; } -// Exported for tests/buildForbiddenInputs.test.js only. The prohibition these -// three functions enforce is the guarantee behind -// https://git.eeqj.de/sneak/AutistMask/issues/324, and `make check` does not -// run `make build` — so they are unit tested against synthetic metafiles -// rather than being exercised only by CI, where "it ran" is not "it works". +// Run only as a program. Required as a module — which is how +// tests/buildForbiddenInputs.test.js and tests/buildWatch.test.js reach the +// functions below — this file builds nothing and writes nothing. +if (require.main === module) { + const args = process.argv.slice(2); + // An argument this file does not know fails rather than being ignored. + if (args.length > 1 || (args.length === 1 && args[0] !== "--watch")) { + console.error("usage: node build.js [--watch]"); + process.exit(2); + } + if (args[0] === "--watch") { + watch(WATCHED_DIRS, build); + } else { + build().catch((err) => { + console.error( + `Build failed: ${err && err.message ? err.message : err}`, + ); + process.exit(1); + }); + } +} + +// Exported for tests only: watch() for tests/buildWatch.test.js, the rest for +// tests/buildForbiddenInputs.test.js. The prohibition those enforce is the +// guarantee behind https://git.eeqj.de/sneak/AutistMask/issues/324, and +// `make check` does not run `make build` — so they are unit tested against +// synthetic metafiles rather than being exercised only by CI, where "it ran" +// is not "it works". module.exports = { importChain, newForbiddenRecord, recordBundledInputs, assertNoForbiddenInputs, assertForbiddenTableCovered, + watch, }; diff --git a/tests/buildWatch.test.js b/tests/buildWatch.test.js new file mode 100644 index 0000000..87409dc --- /dev/null +++ b/tests/buildWatch.test.js @@ -0,0 +1,104 @@ +// `make dev` runs `node build.js --watch` +// (https://git.eeqj.de/sneak/AutistMask/issues/332). These drive build.js's +// watch() over a temp directory with a stand-in for build(), so nothing here +// builds or writes dist/. + +const fs = require("fs"); +const os = require("os"); +const path = require("path"); + +const { watch } = require("../build"); + +let dir; +let watchers = []; + +beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), "autistmask-watch-")); + fs.mkdirSync(path.join(dir, "nested")); + jest.spyOn(console, "log").mockImplementation(() => {}); + jest.spyOn(console, "error").mockImplementation(() => {}); +}); + +afterEach(() => { + for (const watcher of watchers) watcher.close(); + watchers = []; + fs.rmSync(dir, { recursive: true, force: true }); + jest.restoreAllMocks(); +}); + +const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); + +// Wait until `condition()` holds; fail the test if it has not within 3s. +async function until(condition) { + const deadline = Date.now() + 3000; + while (!condition()) { + if (Date.now() > deadline) throw new Error("timed out waiting"); + await sleep(10); + } +} + +// Save the way many editors do: write a new copy, then rename it over the +// original, so the file at that path is a different one afterwards. +function saveByRename(file, contents) { + fs.writeFileSync(`${file}.tmp`, contents); + fs.renameSync(`${file}.tmp`, file); +} + +test("builds at start, then once per change to a file in a subdirectory", async () => { + const file = path.join(dir, "nested", "a.js"); + fs.writeFileSync(file, "1"); + let builds = 0; + watchers = watch([dir], async () => { + builds++; + }); + await until(() => builds === 1); + + fs.writeFileSync(file, "2"); + await until(() => builds === 2); + await sleep(300); + expect(builds).toBe(2); +}); + +test("keeps seeing a file that is saved by renaming a new copy over it", async () => { + const file = path.join(dir, "nested", "a.js"); + fs.writeFileSync(file, "1"); + let builds = 0; + watchers = watch([dir], async () => { + builds++; + }); + await until(() => builds === 1); + + saveByRename(file, "2"); + await until(() => builds === 2); + saveByRename(file, "3"); + await until(() => builds === 3); +}); + +test("a failed build is reported and watching carries on", async () => { + let builds = 0; + watchers = watch([dir], async () => { + builds++; + if (builds === 1) throw new Error("unexpected token"); + }); + await until(() => console.error.mock.calls.length === 1); + expect(console.error).toHaveBeenCalledWith( + "Build failed: unexpected token", + ); + + fs.writeFileSync(path.join(dir, "a.js"), "1"); + await until(() => builds === 2); +}); + +test("a change made while a build runs causes one more build after it", async () => { + let builds = 0; + watchers = watch([dir], async () => { + builds++; + if (builds === 1) { + fs.writeFileSync(path.join(dir, "a.js"), "1"); + await sleep(200); + } + }); + await until(() => builds === 2); + await sleep(300); + expect(builds).toBe(2); +}); -- 2.54.0