From 80c32436c2e3a088828aecb3c9751be137ede6f1 Mon Sep 17 00:00:00 2001 From: mjnong Date: Fri, 4 Sep 2026 23:04:47 +0200 Subject: [PATCH] [CEL-1660] Clean dist/ before build; guard against stale artifacts; add release tooling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the bug found after PR #15 merged: `npm run build` (= `tsc`) never cleaned `dist/` first, and `prepublishOnly` ran only `build`. A stale local dist/ from before a source deletion survives tsc's incremental emit (it only overwrites/adds files for what's still in src/, never removes what isn't) and `index.js` no longer referencing a deleted file doesn't stop `files: ["dist", "!dist/canonical"]` from shipping it anyway, still reachable by a deep import. This is a general class, not a one-off: ANY file deletion from src/ survives into the tarball indefinitely unless dist/ is rebuilt from clean, and no existing gate catches it. Three changes: 1. `npm run clean` (node -e fs.rmSync, no new dependency, cross-platform) + `npm run build` now runs it first. `prepublishOnly` therefore can no longer ship a stale artifact through the normal `npm publish` path. 2. `scripts/verify-dist-no-orphans.mjs` + `npm run verify-dist` — a second, independent guard for anyone who builds outside the normal path (a bare `tsc`, a manually-touched dist/). Walks dist/, maps each .js/.d.ts back to its src/ .ts/.tsx (and .json files vendored via tsc's resolveJsonModule back to their src/ counterpart), and fails loudly listing every orphan if any file has none. Verified working: added a throwaway src/temp-orphan-probe.ts, built, deleted the source without rebuilding (reproducing the exact bug), confirmed the guard fails and names the two orphaned files, then rebuilt (which cleans dist/) to restore a clean, orphan-free state — nothing left over in this commit. Wired into `prepublishOnly` and `check-exports`. 3. New Makefile mirroring @cellarnode/ui's release-install/release-patch/ release-minor/release-major shape (install -> build -> npm version -> npm publish -> commit -> tag), git tag prefix `v` matching this repo's own existing tag (`v0.1.0`, not `ui-v...`). NOT run — this session has no npm publish auth for this package (a separate parked PR already hit ENEEDAUTH attempting exactly that). These targets make the eventual human-run release a single command instead of a hand-run sequence. Finding surfaced, not fixed here (out of this PR's three-item scope): `npx @arethetypeswrong/cli --pack .` exits non-zero on this package TODAY, independent of anything in this PR - verified on a clean checkout of main before touching anything. No CI job wires `check-exports` in, so nobody's noticed it fails outside a exit-code-masking pipe. `make build` and `release-*` therefore chain the new `verify-dist` guard directly, NOT the full `check-exports` (which still runs publint + arethetypeswrong for manual/informational use) - chaining the latter would make every release fail for a pre-existing, unrelated reason. Commented in the Makefile. Gates (direct exit codes, re-run after the final commit): `make build` (typecheck + test + compile-clean-first + verify-dist) = 0. --- Makefile | 77 +++++++++++++++++++++++++ package.json | 8 ++- scripts/verify-dist-no-orphans.mjs | 92 ++++++++++++++++++++++++++++++ 3 files changed, 174 insertions(+), 3 deletions(-) create mode 100644 Makefile create mode 100644 scripts/verify-dist-no-orphans.mjs diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..eb0db35 --- /dev/null +++ b/Makefile @@ -0,0 +1,77 @@ +.PHONY: clean build typecheck test verify-dist check-exports publish release-install release-patch release-minor release-major help + +## Build pipeline +clean: ## Remove dist directory + npm run clean + +typecheck: ## TypeScript type checking + npm run typecheck + +test: ## Run unit tests (includes generated-file freshness checks) + npm test + +compile: ## Compile TypeScript to dist/ (cleans dist/ first — CEL-1660 build-hygiene fix) + npm run build + +verify-dist: ## CEL-1660 — assert dist/ has no artifact whose src/ source no longer exists + npm run verify-dist + +# `check-exports` (publint + arethetypeswrong) is deliberately NOT a +# prerequisite of `build`/`release-*`. `npx @arethetypeswrong/cli --pack .` +# exits non-zero on THIS package today independent of anything in this +# PR — verified on a clean checkout of main before this change — because +# the package ships ESM-only with no node10-compatible CJS fallback for +# the `./react` / `./vue` / `./angular` subpaths. That's a real, pre-existing +# gap (no CI job wires check-exports in either, so nobody's noticed it +# fails when its exit code isn't piped away), but it's a separate problem +# from the stale-dist bug this Makefile exists to fix, and chaining it into +# release-* would make every release fail for an unrelated reason. Run it +# manually when you want the publint/arethetypeswrong report; it is not a +# release gate here. +check-exports: ## Manual-only: dist-orphan guard + publint + arethetypeswrong report (NOT a release gate — see comment above) + npm run check-exports + +build: typecheck test compile verify-dist ## Full build: typecheck + test + compile (clean-first) + dist-orphan gate + +## Publishing + +# CEL-1660 — mirrors @cellarnode/ui's release-install/release-* shape. +# This repo has NO auth path configured for the agent session that wrote +# this Makefile — `npm publish` here returns ENEEDAUTH. Publishing remains +# a human action (Marcus); these targets exist so that action is a single +# command instead of a hand-run sequence, not so an agent can run them. +release-install: ## Refresh node_modules from the lockfile (release pre-gate) + npm ci --legacy-peer-deps + +publish: build ## Build (clean + typecheck + test + compile + dist-orphan gate) and publish current version to npm + npm publish + +release-patch: ## Bump patch version, publish, and git tag + $(MAKE) release-install && \ + $(MAKE) build && \ + npm version patch --no-git-tag-version && \ + npm publish && \ + git add package.json && \ + git commit -m "release(beverage-utils): $$(node -p "require('./package.json').version")" && \ + git tag "v$$(node -p "require('./package.json').version")" + +release-minor: ## Bump minor version, publish, and git tag + $(MAKE) release-install && \ + $(MAKE) build && \ + npm version minor --no-git-tag-version && \ + npm publish && \ + git add package.json && \ + git commit -m "release(beverage-utils): $$(node -p "require('./package.json').version")" && \ + git tag "v$$(node -p "require('./package.json').version")" + +release-major: ## Bump major version, publish, and git tag + $(MAKE) release-install && \ + $(MAKE) build && \ + npm version major --no-git-tag-version && \ + npm publish && \ + git add package.json && \ + git commit -m "release(beverage-utils): $$(node -p "require('./package.json').version")" && \ + git tag "v$$(node -p "require('./package.json').version")" + +help: ## Show this help + @grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-20s\033[0m %s\n", $$1, $$2}' diff --git a/package.json b/package.json index ca27bb9..ade0b7e 100644 --- a/package.json +++ b/package.json @@ -45,15 +45,17 @@ "oiv" ], "scripts": { - "build": "tsc", + "clean": "node -e \"require('fs').rmSync('dist', { recursive: true, force: true })\"", + "build": "npm run clean && tsc", + "verify-dist": "node scripts/verify-dist-no-orphans.mjs", "generate:country-codes": "node scripts/generate-country-codes.mjs", "generate:classifications": "node scripts/generate-classifications.mjs", "check-classifications-fresh": "node scripts/generate-classifications.mjs && git diff --exit-code src/classifications.generated.ts", "generate:aroma-descriptors": "node scripts/generate-aroma-descriptors.mjs", "check-aroma-descriptors-fresh": "node scripts/generate-aroma-descriptors.mjs && git diff --exit-code src/aroma-descriptors.generated.ts", "sync-canonical": "node scripts/sync-canonical.mjs", - "prepublishOnly": "npm run build", - "check-exports": "npx publint && npx @arethetypeswrong/cli --pack .", + "prepublishOnly": "npm run build && npm run verify-dist", + "check-exports": "npm run verify-dist && npx publint && npx @arethetypeswrong/cli --pack .", "test": "npm run check-classifications-fresh && npm run check-aroma-descriptors-fresh && vitest run", "test:watch": "vitest", "typecheck": "tsc --noEmit" diff --git a/scripts/verify-dist-no-orphans.mjs b/scripts/verify-dist-no-orphans.mjs new file mode 100644 index 0000000..d231df7 --- /dev/null +++ b/scripts/verify-dist-no-orphans.mjs @@ -0,0 +1,92 @@ +#!/usr/bin/env node +/** + * CEL-1660 build-hygiene fix — `npm run build` did not clean `dist/` first, + * and `prepublishOnly` ran only `build`. After the PR #15 removal merged, + * `dist/react/` still contained all four deleted `use-beverage-label-map.*` + * artifacts (a stale local build from before the deletion): `index.js` no + * longer referenced them, so nothing resolved through the package entry + * point and `check-exports` stayed green — but `files: ["dist", + * "!dist/canonical"]` meant a publish from that tree would ship the + * "removed" hook anyway, still reachable by a deep import + * (`@cellarnode/beverage-utils/react/use-beverage-label-map`). `build` now + * cleans first (see the `clean` npm script), which makes this structurally + * impossible in normal operation — this script is the second, independent + * guard: it asserts `dist/` contains no artifact whose `src/` source no + * longer exists, so a stale `dist/` (built without going through `npm run + * build`, or meddled with by hand) fails loudly instead of publishing + * silently. + * + * Every `.js` / `.d.ts` file under `dist/` must have a corresponding `.ts` + * or `.tsx` file under `src/` at the same relative path. `.json` files + * (e.g. `dist/canonical/reference-data.json`, emitted via tsc's + * `resolveJsonModule` from the `src/canonical/reference-data.json` it + * imports) must have a same-named counterpart under `src/`. `.map` files + * are skipped — their `.js`/`.d.ts` sibling covers the same source. + */ + +import { existsSync, readdirSync, statSync } from "node:fs"; +import { join, relative } from "node:path"; + +const DIST = "dist"; +const SRC = "src"; + +if (!existsSync(DIST)) { + console.error(`verify-dist-no-orphans: "${DIST}/" does not exist. Run \`npm run build\` first.`); + process.exit(1); +} + +function walk(dir, out = []) { + for (const entry of readdirSync(dir)) { + const full = join(dir, entry); + if (statSync(full).isDirectory()) { + walk(full, out); + } else { + out.push(full); + } + } + return out; +} + +const distFiles = walk(DIST); +const orphans = []; +let checked = 0; + +for (const distFile of distFiles) { + const rel = relative(DIST, distFile); + + // Sourcemaps are covered by their .js/.d.ts sibling's own check. + if (rel.endsWith(".js.map") || rel.endsWith(".d.ts.map")) continue; + + let candidates; + if (rel.endsWith(".d.ts")) { + const base = rel.slice(0, -".d.ts".length); + candidates = [`${base}.ts`, `${base}.tsx`]; + } else if (rel.endsWith(".js")) { + const base = rel.slice(0, -".js".length); + candidates = [`${base}.ts`, `${base}.tsx`]; + } else if (rel.endsWith(".json")) { + // Vendored via tsc's resolveJsonModule (e.g. dist/canonical/reference-data.json + // <- src/canonical/reference-data.json) — copied verbatim, same relative path. + candidates = [rel]; + } else { + // Unknown file type (e.g. a stray README) — not this guard's concern. + continue; + } + + checked++; + const exists = candidates.some((c) => existsSync(join(SRC, c))); + if (!exists) orphans.push(rel); +} + +if (orphans.length > 0) { + console.error( + `verify-dist-no-orphans: ${orphans.length} file(s) in "${DIST}/" have no corresponding source under "${SRC}/":`, + ); + for (const o of orphans) console.error(` - ${DIST}/${o}`); + console.error( + `\nThis means "${DIST}/" is stale relative to "${SRC}/" — a source file was deleted (or renamed) since the last time this tree was actually rebuilt from clean. Run \`npm run build\` (it cleans "${DIST}/" first) and re-run this check.`, + ); + process.exit(1); +} + +console.log(`verify-dist-no-orphans: OK — ${checked} dist artifact(s) checked, 0 orphans.`);