diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c7d57aa87b..ce12291912 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -124,6 +124,52 @@ jobs: - name: Release script tests run: pnpm test:scripts + # The published packages version in lockstep, so a changeset that names + # only some of them still bumps them all — and the ones it left out get a + # version with no changelog entry beside it. Generating the frontmatter from + # the config is not enough on its own, because the group grows: a list + # generated before a package joined it is complete against the config it was + # read from and short against the current one. + # + # Scoped to the changesets THIS pull request adds or edits. Several hundred + # are pending on `main`, most written before the group grew; rewriting them + # to satisfy a rule they predate would churn the eventual changelog for no + # gain. + # + # `HEAD^1` is what makes the scoping exact. On a pull request the checkout + # is the MERGE commit, whose first parent is the base branch — so diffing + # against it is the pull request's own diff, already relative to the merge + # base. Diffing against the base branch TIP instead would report every + # changeset the branch picked up by merging `main` as one this branch + # wrote, which is how a guard starts failing PRs over other people's files. + - name: Changeset covers the lockstep group + if: github.event_name == 'pull_request' + run: | + set -euo pipefail + # `git diff` runs on its own line so a failure — an unavailable `HEAD^1` + # in some other checkout shape, a broken object store — aborts the step + # under `set -e`. Inside a pipeline with a trailing `|| true` it would + # not: the checker would read empty stdin, report nothing to check, and + # the gate would be off while the step stayed green. + # + # Which of those paths is a changeset is decided by the checker, using + # the same filter `@changesets/read` applies. A `grep` here would be a + # second answer, and it would differ from the release tooling on a + # lowercase `readme.md` or a hidden `.template.md`. + # + # `T` sits beside `ACMR` because a changeset converted between a + # regular file and a symlink is reported as a TYPE change, and + # `@changesets/read` follows symlinks — so a release would consume a + # replacement this check never saw. `D` stays out: a deleted changeset + # has nothing left to read. + # + # The pathspec excludes subdirectories because git matches `*` across + # `/` while `@changesets/read` lists `.changeset` non-recursively — so + # a file under `.changeset/archive/` is something the release never + # reads, and checking it would fail a pull request over a note. + touched="$(git diff --name-only --diff-filter=ACMRT HEAD^1 HEAD -- '.changeset/*.md' ':(exclude).changeset/*/**')" + printf '%s\n' "$touched" | node scripts/release/check-changesets.mjs + # Enforce the design-token theming contract in the admin + plugin packages # (no hsl(var(--x)) wrappers, no hardcoded colors, no stray !important). # See packages/ui/docs/plugin-ui-authoring.md. Cheap check, fails fast. diff --git a/package.json b/package.json index 0c5ce2f6bc..9cf7f4ae81 100644 --- a/package.json +++ b/package.json @@ -80,6 +80,7 @@ "@aws-sdk/lib-storage": "^3.1090.0", "@changesets/changelog-github": "^0.5.1", "@changesets/cli": "^2.27.10", + "@changesets/parse": "^0.4.3", "@commitlint/cli": "^20.1.0", "@commitlint/config-conventional": "^20.0.0", "@testing-library/jest-dom": "^6.9.1", @@ -91,6 +92,7 @@ "husky": "^9.1.7", "jsdom": "^27.1.0", "lint-staged": "^16.2.6", + "micromatch": "^4.0.8", "prettier": "^3.6.2", "publint": "^0.3.18", "sherif": "1.13.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2158851d96..4c85d34175 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -75,6 +75,9 @@ importers: '@changesets/cli': specifier: ^2.27.10 version: 2.30.0(@types/node@24.10.0) + '@changesets/parse': + specifier: ^0.4.3 + version: 0.4.3 '@commitlint/cli': specifier: ^20.1.0 version: 20.1.0(@types/node@24.10.0)(typescript@5.9.3) @@ -108,6 +111,9 @@ importers: lint-staged: specifier: ^16.2.6 version: 16.2.6 + micromatch: + specifier: ^4.0.8 + version: 4.0.8 prettier: specifier: ^3.6.2 version: 3.6.2 @@ -13398,7 +13404,7 @@ snapshots: '@humanfs/node': 0.16.6 '@humanwhocodes/module-importer': 1.0.1 '@humanwhocodes/retry': 0.4.3 - '@types/estree': 1.0.9 + '@types/estree': 1.0.8 ajv: 6.15.0 chalk: 4.1.2 cross-spawn: 7.0.6 diff --git a/scripts/release/check-changesets.mjs b/scripts/release/check-changesets.mjs new file mode 100644 index 0000000000..b21fd6d510 --- /dev/null +++ b/scripts/release/check-changesets.mjs @@ -0,0 +1,369 @@ +// Refuses a changeset that does not cover every package in the lockstep group, +// and a `fixed` group that no longer describes the workspace. +// +// The packages version together, so a release advances all of them whatever a +// single changeset lists. What an incomplete one loses is the CHANGELOG: a +// package left out of the frontmatter gets a version bump with no entry +// explaining it, and the note that should have appeared under it is filed only +// under the packages that were named. The release is correct; the record of it +// is not. +// +// Generating the frontmatter from `.changeset/config.json` is not by itself +// enough, because the group grows: a list generated before a package joined it, +// or before the branch merged the commit that added it, is complete against the +// config it was read from and short against the current one. Only a check +// against the config as it stands at build time closes that. +// +// The GROUP is checked too, and against the workspace rather than against +// itself. A checker reading the config as the source of truth cannot see a +// package added under `packages/` and never added to `fixed`, which is the drift +// that makes every changeset written afterwards wrong while each of them passes. +// +// Scoped to the changesets a branch ADDS or EDITS, never the backlog. Several +// hundred are pending on `main`, most written before the group grew, and +// rewriting them to satisfy a rule they predate would put churn in front of +// every reader of the eventual changelog for no gain. + +import { readFileSync } from "node:fs"; +import { dirname, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; + +import parseChangeset from "@changesets/parse"; +import micromatch from "micromatch"; + +import { getWorkspacePackageNames } from "./lib.mjs"; + +const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), "..", ".."); + +/** + * The bump every changeset uses while the packages are in alpha. + * + * A `minor` or `major` here does not just mislabel one entry: the group versions + * in lockstep, so the largest bump in the release decides the number every + * package gets, and one mislabelled changeset moves the whole train. + */ +const ALPHA_BUMP = "patch"; + +/** + * Whether `fixed` has the shape THIS repository requires, as a list of sentences. + * + * Changesets allows several disjoint groups; this repository has exactly one, and + * the difference is not cosmetic. Flattening answers the same set for one group + * and for two, so a config split into two groups reads as complete here while the + * packages no longer version together — the next release can advance one group + * and leave the other behind, which is precisely the outcome every check below + * exists to prevent. + * + * The other refusals are shape: a flat `["a", "b"]` flattens identically to + * `[["a", "b"]]` and is refused by the release tooling, and an entry that is not + * a non-empty string names nothing. + * + * Checked here rather than by handing the file to `@changesets/config`, whose + * `parse` needs a resolved workspace from `@manypkg/get-packages` — a second + * dependency, for a rule that is three sentences long. That is the opposite trade + * to the YAML reader, and for the opposite reason: this rule cannot drift, and a + * frontmatter grammar can. + */ +export function fixedGroupShape(config) { + const groups = config.fixed; + if (groups === undefined) return []; + if (!Array.isArray(groups)) { + return ["`.changeset/config.json`: `fixed` must be an array of arrays."]; + } + const flat = groups.filter(group => !Array.isArray(group)); + if (flat.length > 0) { + return [ + ".changeset/config.json: `fixed` must be an array of ARRAYS — a group per " + + `line, not a flat list of names. Found ${JSON.stringify(flat[0])} where a group was expected.`, + ]; + } + if (groups.length > 1) { + return [ + `.changeset/config.json: \`fixed\` declares ${groups.length} groups. This ` + + `repository versions every package as ONE group; split into two, a release ` + + `can advance one and leave the other behind while every changeset still passes.`, + ]; + } + const problems = []; + for (const group of groups) { + for (const name of group) { + if (typeof name !== "string" || name === "") { + problems.push( + `.changeset/config.json: \`fixed\` holds ${JSON.stringify(name)}, which is not a package name.` + ); + } + } + } + return problems; +} + +/** + * Every package that must appear in a changeset: the `fixed` group expanded + * against the workspace. + * + * `fixed` accepts patterns as well as names, and the whole group goes to + * `micromatch` in ONE call — `micromatch(packageNames, group)` — because that is + * what `@changesets/config` does and the list is not a union of independent + * patterns. A negated entry only means anything in company: `["**", + * "!@nextlyhq/builder"]` excludes builder, while evaluating each pattern alone + * and unioning the results lets `**` put it straight back. + * + * A non-negated entry matching nothing is kept as written, so a typo or a + * departed package is reported by the string an author would search for. + */ +export function lockstepPackages(configText, workspaceNames) { + const group = (JSON.parse(configText).fixed ?? []) + .flat() + .filter(entry => typeof entry === "string"); + if (group.length === 0) return []; + const matched = micromatch(workspaceNames, group); + const unmatched = group.filter( + entry => + !entry.startsWith("!") && + !workspaceNames.some(name => micromatch.isMatch(name, entry)) + ); + return [...new Set([...matched, ...unmatched])]; +} + +/** + * What is wrong with the GROUP itself, as a list of sentences, empty when nothing is. + * + * The config is what every changeset is generated from, so a checker that treats + * it as the source of truth cannot see the one drift that matters most: a pull + * request adding a package under `packages/` and not adding it to `fixed`. Every + * changeset in that PR names the old members, passes, and the new package is left + * behind on the next train — discovered after a version PR has already merged. + * + * Compared against `packages/` rather than against the publishable subset, + * because Changesets versions private workspace packages too unless told + * otherwise, and four of this group's members are `private: true` build-time + * config packages. Measuring against "what we publish" would report those four as + * errors on every run. + * + * Checked in BOTH directions. A name in the group that no package answers to is + * a rename or a deletion that the config still believes in, and Changesets fails + * a release on an unknown package in `fixed`. + */ +export function groupMatchesWorkspace(packages, workspaceNames) { + const problems = []; + const absent = workspaceNames.filter(name => !packages.includes(name)); + if (absent.length > 0) { + problems.push( + `.changeset/config.json: the \`fixed\` group is missing ${absent.join(", ")}. ` + + `Every package under packages/ versions with the group; one left out is ` + + `stranded at an older version by the next release.` + ); + } + const unknown = packages.filter(name => !workspaceNames.includes(name)); + if (unknown.length > 0) { + problems.push( + `.changeset/config.json: the \`fixed\` group names ${unknown.join(", ")}, ` + + `which no package under packages/ answers to. Changesets refuses a release ` + + `on an unknown package in \`fixed\`.` + ); + } + return problems; +} + +/** + * The `package: bump` pairs a changeset declares, or `undefined` when Changesets + * itself would refuse the file. + * + * Handed to `@changesets/parse` rather than read here, so that anything this + * accepts the release accepts, by construction rather than by agreement. A + * frontmatter grammar has two ways to be wrong and both matter: reading it more + * STRICTLY than Changesets blocks a compliant pull request over a spelling the + * release tooling takes (single quotes, quoted bumps, comments), and reading it + * more LOOSELY lets malformed metadata merge and fail the CI-only release + * afterwards (`---junk` as a closing delimiter, duplicate keys, inconsistent + * indentation). Neither margin exists when the same parser decides both. + * + * `@changesets/parse` was already in the tree as a transitive dependency of + * `@changesets/cli`; declaring it changes what resolves, not what is installed. + * + * A file it refuses is reported as unreadable rather than as declaring nothing: + * "no releases" and "every release" are opposite answers, and a missing-package + * check would score them the same way. That distinction is why the empty case is + * still called out separately below — `parse` returns an empty release list for a + * frontmatter that is well-formed and says nothing. + */ +export function declaredReleases(fileText) { + let parsed; + try { + parsed = parseChangeset(fileText); + } catch { + return undefined; + } + const releases = new Map(); + for (const release of parsed.releases) releases.set(release.name, release.type); + return releases; +} + +/** + * What is wrong with one changeset, as a list of sentences, empty when nothing is. + * + * Returns every problem rather than the first, so one push answers all of them. + * A guard that reports one missing package at a time turns a stale frontmatter + * into as many build attempts as it has gaps. + */ +export function problemsWith(path, fileText, packages) { + const releases = declaredReleases(fileText); + if (releases === undefined) { + return [ + `${path}: the frontmatter is missing or has a line this cannot read. ` + + `A changeset opens with \`---\`, one \`"package": bump\` per line, and closes with \`---\`.`, + ]; + } + if (releases.size === 0) { + // Well-formed and saying nothing. Reported on its own rather than as "missing + // all of them", because the cause is different — an empty frontmatter is a + // changeset someone forgot to fill in, not one generated against an older + // group — and the fix a reader needs is not the same. + return [ + `${path}: declares no packages at all. A changeset that releases nothing ` + + `still consumes a file name and produces no changelog entry.`, + ]; + } + const problems = []; + const missing = packages.filter(name => !releases.has(name)); + if (missing.length > 0) { + problems.push( + `${path}: missing ${missing.length} of ${packages.length} lockstep packages — ` + + `${missing.join(", ")}. They version together, so a package left out is bumped ` + + `with no changelog entry. Generate the list from .changeset/config.json.` + ); + } + const unknown = [...releases.keys()].filter(name => !packages.includes(name)); + if (unknown.length > 0) { + problems.push( + `${path}: names ${unknown.join(", ")}, which the lockstep group does not contain. ` + + `A package that has left the group, or a typo, releases nothing.` + ); + } + const wrongBump = [...releases.entries()] + .filter(([, bump]) => bump !== ALPHA_BUMP) + .map(([name, bump]) => `${name}: ${bump}`); + if (wrongBump.length > 0) { + problems.push( + `${path}: uses a bump other than \`${ALPHA_BUMP}\` (${wrongBump.join(", ")}). ` + + `The group takes the largest bump in the release, so one of these moves every package.` + ); + } + return problems; +} + +/** + * Every problem: the group's own integrity first, then each changeset against it. + * + * The group is checked even when the pull request touches no changeset at all, + * because the PR that adds a package is often exactly that one — and a stale + * group makes every later changeset wrong while each of them passes. + */ +export function checkChangesets(paths, readFile, configText, workspaceNames) { + const packages = lockstepPackages(configText, workspaceNames); + if (packages.length === 0) { + // A config with no fixed group would make every check below vacuous, and a + // guard that passes because it found nothing to check is worse than none. + return [ + ".changeset/config.json declares no `fixed` group, so nothing here can be checked.", + ]; + } + const shape = fixedGroupShape(JSON.parse(configText)); + // Returned alone. Every check below reads the flattened group, and a group + // whose shape is wrong flattens to something that looks right — so reporting + // the downstream answers beside it would be reporting answers derived from a + // reading the release tooling does not share. + if (shape.length > 0) return shape; + return [ + ...groupMatchesWorkspace(packages, workspaceNames), + ...paths.flatMap(path => problemsWith(path, readFile(path), packages)), + ]; +} + +/** + * The changeset files to check: arguments when given, otherwise a newline-separated + * list on stdin. + * + * Passed in rather than discovered, because "which changesets does this branch + * add" is a question about git that the caller already has to ask, and asking it + * here would mean this script could only run one way. + * + * Stdin is what the workflow uses, and it is the reason the empty case is not a + * special one: a pipe that produced nothing arrives as an empty string, whereas + * an empty argument list has to survive shell expansion under `set -u` to get + * here at all. + */ +/** + * Whether a path names a file Changesets would READ as a changeset. + * + * Copied from `@changesets/read`, whose filter is + * `!file.startsWith(".") && file.endsWith(".md") && !/^README\.md$/i.test(file)`. + * Anything else in `.changeset/` is documentation or a helper — a template, a + * README in any casing — and passing one of those to a frontmatter check rejects + * ordinary docs as malformed. + * + * Applied HERE rather than in the workflow's `grep`, so a hand-run and the build + * agree about what a changeset is. A shell filter is one caller's answer. + */ +function isChangesetFile(path) { + const name = path.slice(path.lastIndexOf("/") + 1); + return ( + !name.startsWith(".") && name.endsWith(".md") && !/^README\.md$/i.test(name) + ); +} + +export function pathsToCheck(argv, stdinText) { + const fromArgv = argv.filter(isChangesetFile); + if (fromArgv.length > 0) return fromArgv; + return stdinText + .split("\n") + .map(line => line.trim()) + .filter(isChangesetFile); +} + +/** + * Everything piped in, as text. + * + * Streamed rather than read with `readFileSync(0)`. A pipe can be open and + * momentarily empty, and the synchronous read answers that with `EAGAIN` instead + * of waiting — which fails whenever the process on the other side is a shade + * slower than this one, and passes when it is not. A guard that depends on + * scheduling is worse than no guard. + * + * A terminal is not read at all: a hand-run with no arguments would otherwise + * wait forever for input nobody is going to type. + */ +async function readStdin() { + if (process.stdin.isTTY) return ""; + const chunks = []; + for await (const chunk of process.stdin) chunks.push(chunk); + return Buffer.concat(chunks).toString("utf8"); +} + +async function main(argv) { + const paths = pathsToCheck(argv, await readStdin()); + // No early return on an empty list. The group's own integrity still has to be + // checked, and the pull request that adds a package is often the one that + // touches no changeset at all. + const problems = checkChangesets( + paths, + path => readFileSync(resolve(REPO_ROOT, path), "utf8"), + readFileSync(resolve(REPO_ROOT, ".changeset", "config.json"), "utf8"), + getWorkspacePackageNames() + ); + if (problems.length === 0) { + console.log( + paths.length === 0 + ? "No changesets added or edited; the lockstep group matches the workspace." + : `Checked ${paths.length} changeset(s): all cover the group.` + ); + return 0; + } + for (const problem of problems) console.error(`✖ ${problem}`); + return 1; +} + +// Only when run directly, so the exported helpers stay importable from a test. +if (process.argv[1] === fileURLToPath(import.meta.url)) { + process.exit(await main(process.argv.slice(2))); +} diff --git a/scripts/release/check-changesets.test.mjs b/scripts/release/check-changesets.test.mjs new file mode 100644 index 0000000000..58c135e6f2 --- /dev/null +++ b/scripts/release/check-changesets.test.mjs @@ -0,0 +1,418 @@ +import { describe, expect, it } from "vitest"; + +import { + checkChangesets, + declaredReleases, + fixedGroupShape, + groupMatchesWorkspace, + lockstepPackages, + pathsToCheck, + problemsWith, +} from "./check-changesets.mjs"; + +/** A lockstep group small enough to read, shaped like the real one. */ +const CONFIG = JSON.stringify({ + fixed: [["nextly", "@nextlyhq/ui", "@nextlyhq/builder"]], +}); +const WORKSPACE = ["nextly", "@nextlyhq/ui", "@nextlyhq/builder"]; +const PACKAGES = lockstepPackages(CONFIG, WORKSPACE); + +/** A changeset naming the given `package: bump` pairs. */ +function changeset(pairs, body = "Something changed.") { + const frontmatter = pairs.map(([name, bump]) => `"${name}": ${bump}`); + return `---\n${frontmatter.join("\n")}\n---\n\n${body}\n`; +} + +const COMPLETE = changeset(PACKAGES.map(name => [name, "patch"])); + +describe("reading the group", () => { + it("flattens the fixed array", () => { + expect(PACKAGES).toEqual(["nextly", "@nextlyhq/ui", "@nextlyhq/builder"]); + }); + + it("refuses to check anything when the config declares no group", () => { + // A guard that passes because it found nothing to check is worse than no + // guard: it reports success on every changeset for as long as the config is + // broken. + const problems = checkChangesets(["a.md"], () => COMPLETE, "{}", PACKAGES); + expect(problems).toHaveLength(1); + expect(problems[0]).toContain("no `fixed` group"); + }); +}); + +describe("a complete changeset", () => { + it("has nothing wrong with it", () => { + // The control. Without it every assertion below could pass because the check + // rejects everything. + expect(problemsWith("a.md", COMPLETE, PACKAGES)).toEqual([]); + }); + + it("does not care what order the packages are in", () => { + const reordered = changeset( + [...PACKAGES].reverse().map(name => [name, "patch"]) + ); + expect(problemsWith("a.md", reordered, PACKAGES)).toEqual([]); + }); + + it("accepts the unquoted spelling an unscoped package may use", () => { + const mixed = `---\nnextly: patch\n"@nextlyhq/ui": patch\n"@nextlyhq/builder": patch\n---\n\nBody.\n`; + expect(problemsWith("a.md", mixed, PACKAGES)).toEqual([]); + }); +}); + +describe("the omission this exists for", () => { + it("names every package that is missing, not just the first", () => { + // Reporting one at a time turns a frontmatter written against an older group + // into as many CI rounds as it has gaps. + const partial = changeset([["nextly", "patch"]]); + const problems = problemsWith("a.md", partial, PACKAGES); + + expect(problems).toHaveLength(1); + expect(problems[0]).toContain("@nextlyhq/ui"); + expect(problems[0]).toContain("@nextlyhq/builder"); + expect(problems[0]).toContain("missing 2 of 3"); + }); + + it("catches the one-package gap a new group member leaves", () => { + // The shape a growing group produces: a frontmatter generated from the config + // BEFORE a package joined it, so it is complete against the config it was + // written from and short against the current one. + const beforeBuilder = changeset([ + ["nextly", "patch"], + ["@nextlyhq/ui", "patch"], + ]); + const problems = problemsWith("a.md", beforeBuilder, PACKAGES); + + expect(problems).toHaveLength(1); + expect(problems[0]).toContain("@nextlyhq/builder"); + expect(problems[0]).not.toContain("@nextlyhq/ui"); + }); +}); + +describe("what else a frontmatter can get wrong", () => { + it("rejects a package the group does not contain", () => { + const stale = changeset([ + ...PACKAGES.map(name => [name, "patch"]), + ["@nextlyhq/departed", "patch"], + ]); + const problems = problemsWith("a.md", stale, PACKAGES); + + expect(problems).toHaveLength(1); + expect(problems[0]).toContain("@nextlyhq/departed"); + }); + + it("rejects a bump other than patch", () => { + // The group takes the largest bump in the release, so one `minor` moves every + // package's version, not just the one it is written beside. + const minor = changeset([ + ["nextly", "minor"], + ["@nextlyhq/ui", "patch"], + ["@nextlyhq/builder", "patch"], + ]); + const problems = problemsWith("a.md", minor, PACKAGES); + + expect(problems).toHaveLength(1); + expect(problems[0]).toContain("nextly: minor"); + }); + + it("reports several problems together", () => { + const bad = changeset([ + ["nextly", "minor"], + ["@nextlyhq/departed", "patch"], + ]); + expect(problemsWith("a.md", bad, PACKAGES)).toHaveLength(3); + }); +}); + +describe("the group against the workspace", () => { + it("accepts a group that matches", () => { + // The control, and the state the repository is in today. + expect(groupMatchesWorkspace(PACKAGES, [...PACKAGES])).toEqual([]); + }); + + it("catches a package the config was never told about", () => { + // The drift the config cannot see about itself: a PR adds a package under + // packages/ and every changeset in it names the old members, so all of them + // pass while the new package is stranded on the next train. + const problems = groupMatchesWorkspace(PACKAGES, [ + ...PACKAGES, + "@nextlyhq/newcomer", + ]); + + expect(problems).toHaveLength(1); + expect(problems[0]).toContain("@nextlyhq/newcomer"); + expect(problems[0]).toContain("missing"); + }); + + it("catches a name in the group that no package answers to", () => { + // A rename or a deletion the config still believes in. Changesets refuses a + // release on an unknown package in `fixed`. + const problems = groupMatchesWorkspace( + [...PACKAGES, "@nextlyhq/departed"], + [...PACKAGES] + ); + + expect(problems).toHaveLength(1); + expect(problems[0]).toContain("@nextlyhq/departed"); + }); + + it("is asked even when the pull request touches no changeset", () => { + // The PR that adds a package is often exactly the one with no changeset of + // its own, so an early return on an empty list would skip the check that + // matters most. + const problems = checkChangesets([], () => "", CONFIG, [ + ...PACKAGES, + "@nextlyhq/newcomer", + ]); + + expect(problems).toHaveLength(1); + expect(problems[0]).toContain("@nextlyhq/newcomer"); + }); +}); + +describe("frontmatter spellings the release tooling accepts", () => { + it("reads single-quoted names", () => { + // Refusing this would block a compliant PR over a spelling the release + // tooling reads without complaint. + const single = `---\n'nextly': patch\n'@nextlyhq/ui': patch\n'@nextlyhq/builder': patch\n---\n\nBody.\n`; + expect(problemsWith("a.md", single, PACKAGES)).toEqual([]); + }); + + it("reads a quoted bump", () => { + const quotedBump = `---\n"nextly": "patch"\n"@nextlyhq/ui": 'patch'\n"@nextlyhq/builder": patch\n---\n\nBody.\n`; + expect(problemsWith("a.md", quotedBump, PACKAGES)).toEqual([]); + }); + + it("reads comments, whole-line and trailing", () => { + const commented = `---\n# why this exists\n"nextly": patch # the core\n"@nextlyhq/ui": patch\n"@nextlyhq/builder": patch\n---\n\nBody.\n`; + expect(problemsWith("a.md", commented, PACKAGES)).toEqual([]); + }); +}); + +describe("frontmatter the release tooling refuses", () => { + it("refuses a closing delimiter with anything after it", () => { + // `---junk` does not close the block, so everything below it is frontmatter + // as far as Changesets is concerned. A reader that stops at the first three + // dashes reports the file as complete while the release tooling rejects it. + const junk = `---\n"nextly": patch\n"@nextlyhq/ui": patch\n"@nextlyhq/builder": patch\n---junk\n\nBody.\n`; + expect(declaredReleases(junk)).toBeUndefined(); + }); + + it("refuses a duplicate key rather than taking the last one", () => { + // YAML's own answer is last-wins, which would let a compliant-looking + // `"nextly": patch` sit above a `"nextly": major` that decides the release. + const duplicated = `---\n"nextly": patch\n"nextly": major\n"@nextlyhq/ui": patch\n"@nextlyhq/builder": patch\n---\n\nBody.\n`; + expect(declaredReleases(duplicated)).toBeUndefined(); + }); + + it("refuses inconsistent indentation", () => { + // Valid-looking to any reader that trims each line, and rejected by the + // release tooling as `bad indentation of a mapping entry`. A file like this + // merging is a release that fails after a version PR is already in. + const indented = `---\n"nextly": patch\n "@nextlyhq/ui": patch\n"@nextlyhq/builder": patch\n---\n\nBody.\n`; + expect(declaredReleases(indented)).toBeUndefined(); + }); + + it("reports a well-formed changeset that names nothing", () => { + // Distinct from unreadable: the file parses, it just releases nothing. The + // cause is a changeset someone forgot to fill in rather than one generated + // against an older group, so it gets its own sentence. + const empty = `---\n---\n\nBody.\n`; + expect(declaredReleases(empty)?.size).toBe(0); + expect(problemsWith("a.md", empty, PACKAGES)).toEqual([ + expect.stringContaining("declares no packages at all"), + ]); + }); + + it("refuses an entry with no bump at all", () => { + const noBump = `---\n"nextly":\n"@nextlyhq/ui": patch\n"@nextlyhq/builder": patch\n---\n\nBody.\n`; + expect(declaredReleases(noBump)).toBeUndefined(); + }); +}); + +describe("a file this cannot read", () => { + it("is reported rather than treated as naming nothing wrong", () => { + // The failure mode worth naming: a tolerant parser answers "no releases" for + // a broken frontmatter, and "no releases" and "every release" are opposite + // answers that a missing-package check would score the same way. + expect(problemsWith("a.md", "No frontmatter here.\n", PACKAGES)).toEqual([ + expect.stringContaining("frontmatter is missing"), + ]); + }); + + it("is reported when a line inside the frontmatter is malformed", () => { + const broken = `---\n"nextly" patch\n---\n\nBody.\n`; + expect(problemsWith("a.md", broken, PACKAGES)).toEqual([ + expect.stringContaining("cannot read"), + ]); + }); + + it("reads a frontmatter written with CRLF line endings", () => { + // A changeset can be authored on Windows, and a check that only splits on + // `\n` would read `patch\r` as a bump that is not `patch` and reject every + // one of them. + const crlf = COMPLETE.replace(/\n/g, "\r\n"); + expect(declaredReleases(crlf)?.get("nextly")).toBe("patch"); + expect(problemsWith("a.md", crlf, PACKAGES)).toEqual([]); + }); +}); + +describe("the shape of the fixed group itself", () => { + it("accepts the one array of names this repository uses", () => { + expect(fixedGroupShape({ fixed: [["a", "b", "c"]] })).toEqual([]); + // No `fixed` at all is not a shape error; the caller reports it separately. + expect(fixedGroupShape({})).toEqual([]); + }); + + it("refuses a flat array", () => { + // Flattening answers the same set for `[["a","b"]]` and `["a","b"]`, so every + // check downstream reports success while the release tooling refuses the file. + const problems = fixedGroupShape({ fixed: ["a", "b"] }); + expect(problems).toHaveLength(1); + expect(problems[0]).toContain("array of ARRAYS"); + }); + + it("refuses more than one group", () => { + // Changesets allows several disjoint groups; this repository has exactly one. + // Flattening answers the same set either way, so a split config reads as + // complete while a release can advance one group and leave the other behind. + const problems = fixedGroupShape({ fixed: [["a", "b"], ["c"]] }); + expect(problems).toHaveLength(1); + expect(problems[0]).toContain("2 groups"); + }); + + it("refuses a group holding something that is not a name", () => { + const problems = fixedGroupShape({ fixed: [["a", 7]] }); + expect(problems).toHaveLength(1); + expect(problems[0]).toContain("not a package name"); + }); + + it("reports the shape ALONE, without answers derived from it", () => { + // A group whose shape is wrong flattens to something that looks right, so + // reporting the coverage answers beside it would be reporting a reading the + // release tooling does not share. + const problems = checkChangesets( + [], + () => "", + JSON.stringify({ fixed: ["nextly", "@nextlyhq/ui"] }), + ["nextly", "@nextlyhq/ui", "@nextlyhq/builder"] + ); + expect(problems).toHaveLength(1); + expect(problems[0]).toContain("array of ARRAYS"); + }); +}); + +describe("a group written with globs", () => { + it("expands a pattern against the workspace", () => { + // `fixed` accepts patterns, and `@changesets/config` expands each entry with + // `micromatch.isMatch` before validating. Comparing the raw entries would + // report every matching package as missing and the pattern as unknown, which + // on a config written this way rejects every pull request. + const globbed = JSON.stringify({ fixed: [["nextly", "@nextlyhq/*"]] }); + expect(lockstepPackages(globbed, WORKSPACE).sort()).toEqual( + [...WORKSPACE].sort() + ); + }); + + it("honours a negated entry, which only means anything in company", () => { + // `micromatch` is given the WHOLE group in one call, the way + // `@changesets/config` does it. Evaluating each pattern alone and unioning + // the results would let `**` put back what `!…` excluded. + const negated = JSON.stringify({ fixed: [["**", "!@nextlyhq/builder"]] }); + expect(lockstepPackages(negated, WORKSPACE).sort()).toEqual([ + "@nextlyhq/ui", + "nextly", + ]); + }); + + it("reports a pattern that matches nothing by the name it was written as", () => { + const globbed = JSON.stringify({ fixed: [["@nowhere/*"]] }); + expect(lockstepPackages(globbed, WORKSPACE)).toEqual(["@nowhere/*"]); + }); + + it("does not report the workspace as missing when a glob covers it", () => { + const globbed = JSON.stringify({ fixed: [["nextly", "@nextlyhq/*"]] }); + expect( + checkChangesets([], () => "", globbed, WORKSPACE) + ).toEqual([]); + }); +}); + +describe("which files count as changesets", () => { + it("skips what Changesets itself skips", () => { + // `@changesets/read` filters on + // `!startsWith(".") && endsWith(".md") && !/^README\.md$/i`. A docs-only PR + // touching any of these must not be told its documentation is malformed + // frontmatter. + expect( + pathsToCheck( + [], + [ + ".changeset/real-one.md", + ".changeset/README.md", + ".changeset/readme.md", + ".changeset/.template.md", + ".changeset/config.json", + ].join("\n") + ) + ).toEqual([".changeset/real-one.md"]); + }); + + it("applies the same rule to arguments", () => { + expect(pathsToCheck([".changeset/readme.md"], "")).toEqual([]); + }); +}); + +describe("where the file list comes from", () => { + it("prefers arguments when it is given them", () => { + expect(pathsToCheck(["a.md", "b.md"], "never.md")).toEqual([ + "a.md", + "b.md", + ]); + }); + + it("reads stdin when there are no arguments", () => { + expect(pathsToCheck([], ".changeset/a.md\n.changeset/b.md\n")).toEqual([ + ".changeset/a.md", + ".changeset/b.md", + ]); + }); + + it("reads an empty pipe as nothing to do", () => { + // The ordinary PR touches no changeset, so the pipe delivers an empty + // string. Reading that as one path named "" would fail every such build on a + // file that does not exist. + expect(pathsToCheck([], "")).toEqual([]); + expect(pathsToCheck([], "\n")).toEqual([]); + }); + + it("ignores anything that is not a changeset", () => { + expect(pathsToCheck([], ".changeset/a.md\npackage.json\n")).toEqual([ + ".changeset/a.md", + ]); + }); +}); + +describe("what the check is pointed at", () => { + it("checks every path it is given", () => { + const files = { + "good.md": COMPLETE, + "bad.md": changeset([["nextly", "patch"]]), + }; + const problems = checkChangesets( + ["good.md", "bad.md"], + path => files[path], + CONFIG, + PACKAGES + ); + + expect(problems).toHaveLength(1); + expect(problems[0]).toContain("bad.md"); + }); + + it("passes when it is given nothing", () => { + // The ordinary PR touches no changeset — a test-only or docs-only one gets + // none at all — and that must not be a failure. + expect(checkChangesets([], () => "", CONFIG, PACKAGES)).toEqual([]); + }); +}); diff --git a/scripts/release/lib.mjs b/scripts/release/lib.mjs index 5eef93c3f2..30607c749a 100644 --- a/scripts/release/lib.mjs +++ b/scripts/release/lib.mjs @@ -88,6 +88,28 @@ export function readPreState() { * A manifest that exists but cannot be parsed is an error rather than a silent * skip: dropping it here would also drop it from every check below. */ +/** + * Every package name under `packages/`, private ones included. + * + * Deliberately wider than {@link getReleaseManifest}, which answers "what do we + * publish". Changesets versions private workspace packages too unless told + * otherwise, so the set it has to be told about is every package in the + * directory rather than only the publishable ones — and a checker comparing the + * `fixed` group against the narrower list would report the config-only packages + * as errors on every run. + */ +export function getWorkspacePackageNames() { + const names = []; + for (const entry of readdirSync(PACKAGES_DIR, { withFileTypes: true })) { + if (!entry.isDirectory()) continue; + const manifestPath = join(PACKAGES_DIR, entry.name, "package.json"); + if (!existsSync(manifestPath)) continue; + const pkg = readJson(manifestPath); + if (typeof pkg.name === "string") names.push(pkg.name); + } + return names.sort((a, b) => a.localeCompare(b)); +} + export function getReleaseManifest() { const manifest = [];