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/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/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/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( 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",