From cc0610a96adcbda3565d33a69c18b691c1cca604 Mon Sep 17 00:00:00 2001 From: Peter Hurst Date: Fri, 21 Aug 2026 14:34:26 +0100 Subject: [PATCH 1/2] =?UTF-8?q?fix(e2e):=20823=20=E2=80=94=20update=20regi?= =?UTF-8?q?stry-smoke=20to=20the=20exports=20675/789=20renamed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolvedAtom was removed by 675 in favour of asyncRouteAtom(...).data; followResolvedRedirects became followAsyncRedirects. 789 suffixed every route atom with Route: rootAtom/createRootAtom split into the bare instance rootRoute and its factory rootRouteAtom, and redirectAtom, queryParamAtom and validateAtom each gained the suffix. Update every reference in the smoke consumer (routes, App, the vitest suite, and both entrypoint smoke scripts) to match. Ticket: 823 --- e2e/registry-smoke/cjs-smoke.cjs | 2 +- e2e/registry-smoke/esm-smoke.mjs | 12 ++++++------ e2e/registry-smoke/src/App.tsx | 6 +++--- e2e/registry-smoke/src/routes.ts | 19 +++++++++++++------ e2e/registry-smoke/src/smoke.test.tsx | 16 ++++++++-------- 5 files changed, 31 insertions(+), 24 deletions(-) diff --git a/e2e/registry-smoke/cjs-smoke.cjs b/e2e/registry-smoke/cjs-smoke.cjs index 52bb8eaa..6417d6b2 100644 --- a/e2e/registry-smoke/cjs-smoke.cjs +++ b/e2e/registry-smoke/cjs-smoke.cjs @@ -3,7 +3,7 @@ const atoms = require("jarl-atoms"); const bindings = require("jarl-react"); assert.equal(typeof atoms.staticRouteAtom, "function"); -assert.equal(typeof atoms.redirectAtom, "function"); +assert.equal(typeof atoms.redirectRouteAtom, "function"); assert.equal(typeof bindings.Link, "function"); assert.equal(typeof bindings.Route, "function"); assert.equal(typeof bindings.useNavigate, "function"); diff --git a/e2e/registry-smoke/esm-smoke.mjs b/e2e/registry-smoke/esm-smoke.mjs index 4a1ce932..813ceaec 100644 --- a/e2e/registry-smoke/esm-smoke.mjs +++ b/e2e/registry-smoke/esm-smoke.mjs @@ -4,9 +4,9 @@ import * as bindings from "jarl-react"; const EXPECTED_ATOMS = [ "appendQueryParam", - "createRootAtom", + "asyncRouteAtom", + "followAsyncRedirects", "followRedirects", - "followResolvedRedirects", "isRedirect", "joinHref", "locationAtom", @@ -14,11 +14,11 @@ const EXPECTED_ATOMS = [ "paramRouteAtom", "parseQuery", "queryAtom", - "queryParamAtom", + "queryParamRouteAtom", "redirect", - "redirectAtom", - "resolvedAtom", - "rootAtom", + "redirectRouteAtom", + "rootRoute", + "rootRouteAtom", "routeAtom", "splitHref", "staticRouteAtom", diff --git a/e2e/registry-smoke/src/App.tsx b/e2e/registry-smoke/src/App.tsx index 53868391..f01435ec 100644 --- a/e2e/registry-smoke/src/App.tsx +++ b/e2e/registry-smoke/src/App.tsx @@ -1,6 +1,6 @@ import { useState } from "react"; import { Link, Route, useIsActive, useNavigate } from "jarl-react"; -import { aboutRoute, productRoute, productsRoute, rootAtom, searchQueryRoute } from "./routes"; +import { aboutRoute, productRoute, productsRoute, rootRoute, searchQueryRoute } from "./routes"; const SearchForm = () => { const [searchText, setSearchText] = useState(""); @@ -23,7 +23,7 @@ const App = () => { return ( <> - +

Home

 diff --git a/e2e/registry-smoke/src/routes.ts b/e2e/registry-smoke/src/routes.ts index 43e7fab5..1c86f4b9 100644 --- a/e2e/registry-smoke/src/routes.ts +++ b/e2e/registry-smoke/src/routes.ts @@ -1,16 +1,23 @@ -import { paramRouteAtom, queryParamAtom, redirectAtom, resolvedAtom, rootAtom, staticRouteAtom } from "jarl-atoms"; +import { + asyncRouteAtom, + paramRouteAtom, + queryParamRouteAtom, + redirectRouteAtom, + rootRoute, + staticRouteAtom, +} from "jarl-atoms"; -export { rootAtom }; +export { rootRoute }; export const aboutRoute = staticRouteAtom("about"); export const productsRoute = staticRouteAtom("products"); export const productRoute = paramRouteAtom("productId", { parent: productsRoute }); -export const searchQueryRoute = queryParamAtom("q"); +export const searchQueryRoute = queryParamRouteAtom("q"); export const movedRoute = staticRouteAtom("moved"); -export const movedRedirect = redirectAtom("/about", { parent: movedRoute }); +export const movedRedirect = redirectRouteAtom("/about", { parent: movedRoute }); -export const productData = resolvedAtom(productRoute, async ({ productId }) => ({ +export const productData = asyncRouteAtom(productRoute, "product", async ({ productId }) => ({ productId, title: `Product ${productId}`, -})); +})).data; diff --git a/e2e/registry-smoke/src/smoke.test.tsx b/e2e/registry-smoke/src/smoke.test.tsx index f7354bd1..e958fe30 100644 --- a/e2e/registry-smoke/src/smoke.test.tsx +++ b/e2e/registry-smoke/src/smoke.test.tsx @@ -3,10 +3,10 @@ import { Provider, createStore } from "jotai"; import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { followRedirects, followResolvedRedirects, isRedirect, locationAtom, redirect, resolvedAtom } from "jarl-atoms"; +import { asyncRouteAtom, followAsyncRedirects, followRedirects, isRedirect, locationAtom, redirect } from "jarl-atoms"; import { Route, useAtomValue } from "jarl-react"; import App from "./App"; -import { aboutRoute, movedRedirect, productData, productRoute, rootAtom, searchQueryRoute } from "./routes"; +import { aboutRoute, movedRedirect, productData, productRoute, rootRoute, searchQueryRoute } from "./routes"; type Store = ReturnType; @@ -76,7 +76,7 @@ describe("navigation", () => { }); describe("redirects", () => { - it("follows a matched redirectAtom to its target", () => { + it("follows a matched redirectRouteAtom to its target", () => { seed(store, "/moved"); const unsubscribe = followRedirects(store, [movedRedirect]); expect(store.get(locationAtom).pathname).toBe("/about"); @@ -84,10 +84,10 @@ describe("redirects", () => { unsubscribe(); }); - it("follows a Redirect returned from a resolver", async () => { - const gated = resolvedAtom(productRoute, async () => redirect("/about")); + it("follows a Redirect returned from a loader", async () => { + const gated = asyncRouteAtom(productRoute, "gated", async () => redirect("/about")).data; seed(store, "/products/999"); - const unsubscribe = followResolvedRedirects(store, [gated]); + const unsubscribe = followAsyncRedirects(store, [gated]); expect(isRedirect(await store.get(gated))).toBe(true); await Promise.resolve(); expect(store.get(locationAtom).pathname).toBe("/about"); @@ -96,7 +96,7 @@ describe("redirects", () => { }); describe("atoms", () => { - it("resolves async route data via resolvedAtom", async () => { + it("resolves async route data via asyncRouteAtom", async () => { seed(store, "/products/42"); await expect(store.get(productData)).resolves.toEqual({ productId: "42", title: "Product 42" }); }); @@ -107,7 +107,7 @@ describe("atoms", () => { }); it("re-exports jotai's hooks so a consumer needs no direct jotai import", () => { - const Probe = () => {String(useAtomValue(rootAtom).match)}; + const Probe = () => {String(useAtomValue(rootRoute).match)}; seed(store, "/"); render( From 7bdd11528cb8f87116e76e6af866605f7dbff628 Mon Sep 17 00:00:00 2001 From: Peter Hurst Date: Fri, 21 Aug 2026 14:34:36 +0100 Subject: [PATCH 2/2] =?UTF-8?q?ci(e2e):=20823=20=E2=80=94=20run=20registry?= =?UTF-8?q?-smoke=20on=20PRs=20against=20a=20locally-packed=20build?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit registry-smoke installed jarl-atoms/jarl-react from the npm registry at `latest`, on a job wired to workflow_dispatch only. That tests the published package, which was the point, but it meant a source rename here couldn't fail until the next release republished it — and it would fail in a job nobody watches by default. This is exactly the gap that let 675 and 789 drift unnoticed until this ticket. Of the three ways to close it (track renames in lockstep, pin to an explicit version so the mismatch is legible, or run on PRs against a local build), this takes the third: pack-local.mjs packs jarl-atoms/ jarl-react from the working tree, the same way cjs-nodenext/ already does for its own narrower check, and installs the tarballs as a real `file:` dependency in place of `latest`. The registry-smoke CI job now runs unconditionally alongside build-and-test instead of gating on workflow_dispatch, so drift surfaces on the PR that introduces it. Traded away: this no longer proves the currently-published npm tarball works end-to-end (a botched `npm publish`, a stale `files`/ `exports` entry only a real publish would expose). What it buys back is catching everything short of the publish step itself, on every PR rather than after a release ships broken — recorded in e2e/registry-smoke/README.md, "Tracking source renames", along with how to point it at a real published version instead. Ticket: 823 --- .github/workflows/ci.yml | 21 ++++++---- CLAUDE.md | 11 ++--- README.md | 4 +- e2e/registry-smoke/.gitignore | 3 +- e2e/registry-smoke/README.md | 67 ++++++++++++++++++++----------- e2e/registry-smoke/pack-local.mjs | 34 ++++++++++++++++ e2e/registry-smoke/package.json | 8 ++-- package.json | 2 +- 8 files changed, 106 insertions(+), 44 deletions(-) create mode 100644 e2e/registry-smoke/pack-local.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1c25064a..d5eb68c3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,10 +40,6 @@ on: description: "Release everything outstanding since the last tag" type: boolean default: false - registry_smoke: - description: "Smoke-test the currently published packages" - type: boolean - default: false jobs: build-and-test: @@ -98,18 +94,27 @@ jobs: run: npm run test:e2e registry-smoke: - name: Registry smoke test (published packages) + name: Registry smoke test (packed local build) runs-on: ubuntu-latest - # Tests the published packages, not this commit — pointless on a PR build; run by hand after a release. - if: github.event_name == 'workflow_dispatch' && inputs.registry_smoke + needs: build-and-test + # Ticket 823: this used to install from the npm registry at `latest`, gated behind + # workflow_dispatch only — so a source rename here couldn't fail until the next release + # republished, in a job nobody was watching. It now packs jarl-atoms/jarl-react from this + # commit as `npm pack` would for a release and installs those tarballs as a real npm + # dependency (see e2e/registry-smoke/README.md, "Tracking source renames"), so drift shows up + # on the PR that introduced it instead of after a release ships broken. steps: - uses: actions/checkout@v5 - uses: actions/setup-node@v5 with: node-version: 24 + cache: npm + + - name: Install dependencies + run: npm ci - - name: Install the smoke consumer from the registry + - name: Pack local build and install the smoke consumer run: npm run test:smoke:install - name: Run the smoke consumer diff --git a/CLAUDE.md b/CLAUDE.md index bf06c0b3..1de868d8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -14,11 +14,12 @@ JARL ("JARL: Atomic Routing Library") is a controlled-component router for React dogfoods the two packages above for its own navigation - `e2e/` — Playwright suite plus the minimal Vite fixture app it drives. A separate npm project (not a workspace) with its own deps: `npm run test:e2e:install` first. -- `e2e/registry-smoke/` — a consumer project that installs both packages from the npm - registry and uses them unlinked, so the published tarballs get exercised. Also a - separate npm project; run it after a release, not against working-tree changes. - `cjs-nodenext/` inside it is the exception: it packs and installs from the working - tree, to typecheck a `node16`-resolution CommonJS consumer against uncommitted builds. +- `e2e/registry-smoke/` — a consumer project that installs both packages as a real npm + dependency, unlinked from the workspace — `pack-local.mjs` packs them from the working + tree and installs the tarballs, so it exercises the same `dist`/`exports`/`.d.ts` a + release would ship, on every PR (see its README, "Tracking source renames"). Also a + separate npm project. `cjs-nodenext/` inside it does the same pack-and-install, to + typecheck a `node16`-resolution CommonJS consumer. - `infra/` — AWS CDK app provisioning the hosting for jarl.randomdev.co.uk. Also a separate npm project, kept out of the workspaces so it is never published: `infra/README.md`. diff --git a/README.md b/README.md index 60f0e5d1..518f5f07 100644 --- a/README.md +++ b/README.md @@ -204,8 +204,8 @@ npm run test:e2e:install # once, to install the suite's deps and browsers npm run test:e2e ``` -To check the packages as actually published on npm — installed from the registry into a -clean consumer project, with no workspace linking (see +To check the packages as a real npm dependency would see them — installed from a locally +packed tarball into a clean consumer project, with no workspace linking (see [`e2e/registry-smoke`](./e2e/registry-smoke)): ``` diff --git a/e2e/registry-smoke/.gitignore b/e2e/registry-smoke/.gitignore index 0502d25c..43297ff1 100644 --- a/e2e/registry-smoke/.gitignore +++ b/e2e/registry-smoke/.gitignore @@ -1,3 +1,4 @@ node_modules/ -# Committing one would pin the versions under test to whatever was latest the day it was written. +tarballs/ +# Committing one would pin the tarball integrity hash to whatever pack-local.mjs last produced. package-lock.json diff --git a/e2e/registry-smoke/README.md b/e2e/registry-smoke/README.md index 54e253e5..ee9e023a 100644 --- a/e2e/registry-smoke/README.md +++ b/e2e/registry-smoke/README.md @@ -1,56 +1,75 @@ # Registry smoke test -A throwaway consumer project that installs `jarl-atoms` and `jarl-react` **from the npm -registry** and uses them the way a third party would. Nothing here resolves to -`packages/*` — no workspace link, no path alias — so it is the only test in this repo that -exercises the actual published tarballs: the bundled `dist/`, the `exports` map, the -emitted `.d.ts`, and the dependency ranges npm resolves from the manifest. - -Run it after a release; it says nothing useful about uncommitted work. +A throwaway consumer project that installs `jarl-atoms` and `jarl-react` **as a real npm +dependency** — a `file:` tarball, not a workspace link or path alias — and uses them the way a +third party would. It exercises the actual packed artifacts: the bundled `dist/`, the `exports` +map, the emitted `.d.ts`, and the dependency ranges npm resolves from the manifest. ```bash -npm run test:smoke:install # from the repo root, once per version under test +npm run test:smoke:install # builds packages/{jarl-atoms,jarl-react} and packs them here npm run test:smoke ``` `npm test` here runs three checks in order: 1. **`typecheck`** — `tsc` over the consumer sources with `skipLibCheck` off, so the - published declaration files are themselves typechecked. + packed declaration files are themselves typechecked. 2. **`test:entrypoints`** — `esm-smoke.mjs` and `cjs-smoke.cjs` load both packages through real Node resolution (not Vite's), asserting the full export list is present and that route matching works with no DOM. 3. **`test:unit`** — a jsdom app covering routing, link reversal, click and programmatic - navigation, query params, redirects, `resolvedAtom` and server rendering from a seeded + navigation, query params, redirects, `asyncRouteAtom` and server rendering from a seeded location. -## Versions under test +## Tracking source renames (ticket 823) + +Until 2026-08-21 this project installed `jarl-atoms`/`jarl-react` **from the npm registry at +`latest`**, on a job wired to `workflow_dispatch` only — never a PR. That tests the *published* +package, which is the point, but it meant a source rename (`resolvedAtom` removed by ticket 675, +five atoms gaining a `Route` suffix by ticket 789) went unnoticed here: the test kept passing +against whatever was already on the registry and would only have broken in a job nobody watches, +the moment the next release republished. + +Three ways to close that were on the table: + +1. update this project in lockstep with every source rename — it then tests the *next* release + rather than the current one, and still can't fail until someone remembers to touch it; +2. pin to an explicit `jarl-atoms`/`jarl-react` version, so a mismatch is at least legible when + someone looks; +3. run it on every PR against a **locally-packed tarball** of this branch, so drift is caught the + moment it's introduced. -Both packages are declared as `latest` and the lockfile is gitignored, so `npm install` -always fetches whatever is currently published. To pin a specific version instead: +(3) is what's implemented, via `pack-local.mjs` and the `registry-smoke` CI job running +unconditionally alongside `build-and-test` (see `.github/workflows/ci.yml`) rather than only on +manual dispatch. **What this trades away**: the job no longer proves the currently-published npm +tarball works end-to-end — a botched `npm publish`, a stale `files`/`exports` entry that only a +real publish would expose, can't be caught this way. What it buys back: the build this job packs +uses the same `files`/`main`/`exports`/`types` fields and the same `dist/` a release would ship, +so it catches everything short of the publish step itself, and it catches it on every PR rather +than after a release ships broken. + +To point it at a real published version instead — e.g. to actually smoke-test a release — +override the dependency after install: ```bash -npm --prefix e2e/registry-smoke install jarl-atoms@2.0.1 jarl-react@2.0.1 +npm --prefix e2e/registry-smoke install jarl-atoms@2.7.0 jarl-react@2.7.0 ``` +`pack-local.mjs` packs both packages from the working tree and installs the tarballs here. +Repeat runs must remove the previously-extracted `node_modules/` copies and `package-lock.json` +first — npm treats an unchanged `file:` dependency spec as satisfied and won't re-read a +same-named tarball whose contents changed. (The script does this itself.) + ## CommonJS consumer under `node16`/`nodenext` resolution `cjs-nodenext/` type-checks a CommonJS consumer against `dist/index.d.cts` — the declaration file the `require` condition's `types` points at — under `moduleResolution: node16` (the only setting that actually raises TS1479 for a masquerading-as-ESM package; `nodenext` resolves the same files but the compiler's -own gate for that diagnostic excludes it). - -It runs against **local tarballs**, not the registry: this check exists to catch -regressions before a release, so it must work against uncommitted `dist/` output, -and separately the registry can carry a broken version of either package. +own gate for that diagnostic excludes it). It uses the same locally-packed-tarball +approach as the rest of this project, via its own `pack-local.mjs`: ```bash npm run build --workspace packages/jarl-atoms --workspace packages/jarl-react npm --prefix e2e/registry-smoke run test:cjs-nodenext ``` - -`cjs-nodenext/pack-local.mjs` packs both packages from the working tree and installs -the tarballs here. Repeat runs must remove the previously-extracted `node_modules/` -copies and `package-lock.json` first — npm treats an unchanged `file:` dependency -spec as satisfied and won't re-read a same-named tarball whose contents changed. diff --git a/e2e/registry-smoke/pack-local.mjs b/e2e/registry-smoke/pack-local.mjs new file mode 100644 index 00000000..d929a19d --- /dev/null +++ b/e2e/registry-smoke/pack-local.mjs @@ -0,0 +1,34 @@ +// Packs jarl-atoms/jarl-react from the working tree and installs the tarballs +// here, so the smoke consumer runs against this branch's build rather than +// whatever happens to be `latest` on the registry. See README.md for why: +// a registry-pinned `latest` install can't catch a source rename until the +// next release actually ships it, in a job nobody watches by default. +import { execFileSync } from "node:child_process"; +import { mkdirSync, renameSync, rmSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const here = path.dirname(fileURLToPath(import.meta.url)); +const repoRoot = path.resolve(here, "../.."); +const tarballDir = path.join(here, "tarballs"); + +rmSync(tarballDir, { recursive: true, force: true }); +mkdirSync(tarballDir, { recursive: true }); + +for (const pkg of ["jarl-atoms", "jarl-react"]) { + const pkgDir = path.join(repoRoot, "packages", pkg); + const output = execFileSync("npm", ["pack", "--json", "--pack-destination", tarballDir], { + cwd: pkgDir, + }).toString(); + const [{ filename }] = JSON.parse(output); + renameSync(path.join(tarballDir, filename), path.join(tarballDir, `${pkg}.tgz`)); + + // npm doesn't detect a changed file: tarball under an unchanged version/spec: + // package-lock.json pins the integrity hash from the first pack, and the + // lockfile alone is enough for a repeat install to skip re-reading the + // tarball. Both the lock and the extracted copy have to go. + rmSync(path.join(here, "node_modules", pkg), { recursive: true, force: true }); +} +rmSync(path.join(here, "package-lock.json"), { force: true }); + +execFileSync("npm", ["install"], { cwd: here, stdio: "inherit" }); diff --git a/e2e/registry-smoke/package.json b/e2e/registry-smoke/package.json index f932f676..9638b2f2 100644 --- a/e2e/registry-smoke/package.json +++ b/e2e/registry-smoke/package.json @@ -2,9 +2,10 @@ "name": "jarl-registry-smoke", "version": "0.0.0", "private": true, - "description": "Consumes the published jarl-atoms/jarl-react from the npm registry, exactly as a third-party project would, and exercises routing, navigation, redirects and the atoms API against the real build artifacts.", + "description": "Consumes jarl-atoms/jarl-react as a third-party project would — real package resolution, not a workspace link — and exercises routing, navigation, redirects and the atoms API against a locally-packed build of this branch.", "type": "module", "scripts": { + "pack-local": "node ./pack-local.mjs", "test": "npm run typecheck && npm run test:entrypoints && vitest run", "test:entrypoints": "node ./esm-smoke.mjs && node ./cjs-smoke.cjs", "test:unit": "vitest run", @@ -12,8 +13,9 @@ "test:cjs-nodenext": "npm --prefix cjs-nodenext run test" }, "dependencies": { - "jarl-atoms": "latest", - "jarl-react": "latest" + "jarl-atoms": "file:./tarballs/jarl-atoms.tgz", + "jarl-react": "file:./tarballs/jarl-react.tgz", + "jotai": "^2" }, "devDependencies": { "@testing-library/dom": "^10.4.1", diff --git a/package.json b/package.json index d7d90ece..eb20e9a8 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "ci-test": "npm run ci-test --workspaces --if-present", "test:e2e:install": "npm --prefix e2e install && npx --prefix e2e playwright install --with-deps chromium", "test:e2e": "npm --prefix e2e run test", - "test:smoke:install": "npm --prefix e2e/registry-smoke install", + "test:smoke:install": "npm run build --workspace packages/jarl-atoms --workspace packages/jarl-react && npm --prefix e2e/registry-smoke run pack-local", "test:smoke": "npm --prefix e2e/registry-smoke run test", "test:smoke:local": "npm run build --workspace packages/jarl-atoms --workspace packages/jarl-react && npm --prefix e2e/registry-smoke run test:cjs-nodenext", "ci-publish": "npm publish --workspace packages/jarl-atoms --workspace packages/jarl-react",