Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -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}'
8 changes: 5 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
92 changes: 92 additions & 0 deletions scripts/verify-dist-no-orphans.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
#!/usr/bin/env node
/**
* CEL-1660 build-hygiene fix — `npm run build` did not clean `dist/` first,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: This PR is filed under Linear CEL-1660 but does not implement that ticket's requirements: repoint 21 consumer call sites onto useReferenceData/useBeverageClassifications, delete useBeverageLabelMap from the package, bump consumers to a caret range, and add direct hook test coverage are all absent (the Vue/angular label-map hooks remain in src/). The diff is valid build-hygiene work (clean-first dist plus a stale-artifact guard) but is out of scope for CEL-1660; retitle/re-scope the PR or split it so the ticket association is accurate.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/verify-dist-no-orphans.mjs, line 3:

<comment>This PR is filed under Linear CEL-1660 but does not implement that ticket's requirements: repoint 21 consumer call sites onto useReferenceData/useBeverageClassifications, delete useBeverageLabelMap from the package, bump consumers to a caret range, and add direct hook test coverage are all absent (the Vue/angular label-map hooks remain in src/). The diff is valid build-hygiene work (clean-first dist plus a stale-artifact guard) but is out of scope for CEL-1660; retitle/re-scope the PR or split it so the ticket association is accurate.</comment>

<file context>
@@ -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.*`
</file context>

* 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.`);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: On an empty or near-empty dist/ the guard reports success. walk(DIST) returns [] when dist/ exists with no files (e.g. after make clean, after a build that aborted between clean and tsc, or after any failure path that leaves dist/ present but unpopulated), so orphans.length === 0 and the script prints "OK — 0 dist artifact(s) checked" and exits 0. A stale or never-produced build therefore passes the very gate meant to fail loudly on a bad tree. Note the inconsistency: a missing dist/ fails (lines 24-26) but an empty one succeeds. Since the script only ever counts files that already exist in dist/, it cannot distinguish "clean build" from "no build happened". Fail when checked === 0 so an empty/partial tree is never reported as OK.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/verify-dist-no-orphans.mjs, line 92:

<comment>On an empty or near-empty `dist/` the guard reports success. `walk(DIST)` returns `[]` when `dist/` exists with no files (e.g. after `make clean`, after a build that aborted between `clean` and `tsc`, or after any failure path that leaves `dist/` present but unpopulated), so `orphans.length === 0` and the script prints "OK — 0 dist artifact(s) checked" and exits 0. A stale or never-produced build therefore passes the very gate meant to fail loudly on a bad tree. Note the inconsistency: a *missing* `dist/` fails (lines 24-26) but an *empty* one succeeds. Since the script only ever counts files that already exist in `dist/`, it cannot distinguish "clean build" from "no build happened". Fail when `checked === 0` so an empty/partial tree is never reported as OK.</comment>

<file context>
@@ -0,0 +1,92 @@
+  process.exit(1);
+}
+
+console.log(`verify-dist-no-orphans: OK — ${checked} dist artifact(s) checked, 0 orphans.`);
</file context>
Suggested change
console.log(`verify-dist-no-orphans: OK — ${checked} dist artifact(s) checked, 0 orphans.`);
if (checked === 0) {
console.error(`verify-dist-no-orphans: no artifacts found in "${DIST}/" — dist is empty. Run \`npm run build\` first.`);
process.exit(1);
}
console.log(`verify-dist-no-orphans: OK — ${checked} dist artifact(s) checked, 0 orphans.`);

Loading