diff --git a/scripts/__tests__/check-changeset-presence.test.ts b/scripts/__tests__/check-changeset-presence.test.ts index bb2c97c6a9..74846ca6d1 100644 --- a/scripts/__tests__/check-changeset-presence.test.ts +++ b/scripts/__tests__/check-changeset-presence.test.ts @@ -6,11 +6,13 @@ import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { + CONTRACT_FIELDS, changedFiles, classifyChangedPaths, describeDeclaration, discoverPackages, isPublishedSource, + manifestFieldsChanged, readReleaseConfig, resolveBaseRef, } from '../check-changeset-presence.mjs'; @@ -44,6 +46,15 @@ import { * below keep the widening honest in both directions: red-naming-the-file, * green-when-declared, and green for a non-shipped neighbour in the same * package directory. + * 2c. **The manifest's PUBLISHED CONTRACT is read by FIELD** (objectui#6736). + * `sideEffects`, `exports` and their six neighbours are read by every + * consumer and live in no file the population above covers, so PR #6735's + * `sideEffects` array got `no changeset is owed` from this gate's own + * verdict. The legs here are the widening's two directions at once: the + * eight fields go red, and the two exclusions the card NAMED — a `version` + * bump written by `changeset version`, a Dependabot `devDependencies` bump — + * stay green. Neither exclusion is a branch in the code; both are properties + * of the allowlist, which is why they need a test to stay true. * 3. **The verdicts**, against throwaway repositories rather than this one's * history, so they stay decidable when the history moves. * 4. **Every missing input fails LOUD.** A diff gate that cannot compute its @@ -460,6 +471,302 @@ describe("the population is a package's PUBLISHED, EXECUTABLE source — not `sr }); }); +// ── 2c. the manifest's PUBLISHED CONTRACT, read by field (objectui#6736) ───── + +describe("a package's published CONTRACT lives in package.json fields, not only in files", () => { + // objectui#6736: `sideEffects`, `exports`/`main`/`module`/`types`, `files`, + // `peerDependencies`/`engines` are read by every consumer and are under no + // path the population above covers. Measured on PR #6735, which gave + // `@object-ui/app-shell` a `sideEffects` ARRAY — modules the array does not + // name become droppable in every consumer's build — and got this gate's + // `No source of a released package changed in this range, so no changeset is + // owed`. The changeset in that PR was there because its author decided it was. + // + // The reading is by FIELD and not by file, and that is what makes the card's + // two named exclusions expressible at all. Both are exercised below, because + // neither is a branch in the gate: they hold only for as long as the allowlist + // stays an allowlist. + + /** Rewrites one workspace manifest, keeping `name` (discovery reads it). */ + const manifest = (extra: Record): string => + JSON.stringify({ name: '@fixture/alpha', version: '1.0.0', ...extra }, null, 2); + + it('POSITIVE — a sideEffects array appears with no changeset, and the gate goes red naming the FIELD', () => { + const repo = fixtureRepo('contract-side-effects'); + repo.write('packages/alpha/package.json', manifest({ sideEffects: ['./src/register.ts'] })); + repo.commit('perf(alpha): declare precise sideEffects, undeclared'); + + const run = runGate(repo.root, lastCommitRange(repo)); + expect(run.status, "every consumer's bundler reads this and nothing declared it").toBe(1); + expect(run.output).toMatch(/adds no changeset/); + expect(run.output).toMatch(/@fixture\/alpha/); + expect(run.output).toMatch(/packages\/alpha\/package\.json/); + // Naming the FIELD is the load-bearing half here, and more so than naming + // the file was for objectui#5733: "your package.json changed" is also true + // of a version bump this gate passes, so a red without the field name sends + // the author looking for a change the gate did not object to. + expect(run.output).toMatch(/sideEffects/); + }); + + it('NEGATIVE CONTROL — the same change WITH a changeset is green', () => { + const repo = fixtureRepo('contract-declared'); + repo.write('packages/alpha/package.json', manifest({ sideEffects: ['./src/register.ts'] })); + repo.write('.changeset/eager-moths-shake.md', '---\n"@fixture/alpha": patch\n---\n\nPrecise sideEffects.\n'); + repo.commit('perf(alpha): declared'); + + const run = runGate(repo.root, lastCommitRange(repo)); + expect(run.status).toBe(0); + expect(run.output).toMatch(/declares 1 changeset/); + // Green BECAUSE of the declaration, not because nothing was looked at — the + // distinction the old gate could not make on this change class at all. + expect(run.output).toMatch(/published-contract change\(s\)/); + }); + + it('EXCLUSION 1 — a `version` bump alone, which is what `changeset version` writes, is green', () => { + // A gate that went red here would go red on the release commit that answers + // it. Measured against this repository's own history: `59f61cfb8` + // (`chore: release packages`, #4655) rewrites 40 released package.json files + // and adds no changeset — it EMPTIES `.changeset/`. Field-level: exit 0. + // Any file-level reading of the same commit: exit 1. + const repo = fixtureRepo('contract-version-bump'); + repo.write('packages/alpha/package.json', JSON.stringify({ name: '@fixture/alpha', version: '1.1.0' }, null, 2)); + repo.commit('chore: release packages'); + + const run = runGate(repo.root, lastCommitRange(repo)); + expect(run.status, '`version` is not in CONTRACT_FIELDS, so nothing was owed').toBe(0); + expect(run.output).toMatch(/no changeset is owed/); + expect(run.output).toMatch(/0 of them a manifest whose published contract moved/); + }); + + it('EXCLUSION 2 — a devDependencies bump alone, which is what Dependabot writes, is green', () => { + // Measured against real history: `590dd6356` (#4948, `chore(deps-dev): bump + // the dev-dependencies group ... with 11 updates`) rewrites 9 released + // package.json files with no changeset. Field-level: exit 0. File-level: 1. + const repo = fixtureRepo('contract-devdeps-bump'); + repo.write( + 'packages/alpha/package.json', + manifest({ devDependencies: { vitest: '^3.2.0' }, dependencies: { clsx: '^2.1.1' } }), + ); + repo.commit('chore(deps-dev): bump vitest'); + + const run = runGate(repo.root, lastCommitRange(repo)); + expect(run.status).toBe(0); + expect(run.output).toMatch(/no changeset is owed/); + }); + + it('every guarded field goes red on its own', () => { + // One fixture per field rather than one fixture moving all eight: a single + // combined case passes as long as ANY one field is read, which is how a + // half-wired allowlist stays green. + const values: Record = { + engines: { node: '>=24' }, + exports: { '.': './dist/index.js' }, + files: ['dist', 'plugin.ts'], + main: './dist/index.js', + module: './dist/index.mjs', + peerDependencies: { react: '^19.0.0' }, + sideEffects: false, + types: './dist/index.d.ts', + }; + expect(Object.keys(values).sort(), 'a field with no case here would be untested').toEqual([...CONTRACT_FIELDS].sort()); + + for (const field of CONTRACT_FIELDS) { + const repo = fixtureRepo(`contract-field-${field.toLowerCase()}`); + repo.write('packages/alpha/package.json', manifest({ [field]: values[field] })); + repo.commit(`chore(alpha): move ${field}`); + + const run = runGate(repo.root, lastCommitRange(repo)); + expect(run.status, `${field} is part of the published contract`).toBe(1); + expect(run.output).toMatch(new RegExp(field)); + } + }); + + it('an UNGUARDED field moving on its own is green, however large the edit', () => { + // The other direction of the same allowlist. `scripts`, `dependencies` and + // the rest are not read, and `dependencies` staying out is a KEPT part of + // the pre-#6736 trade rather than an oversight — stated in the gate's header + // so nobody reads "the manifest is guarded now" off this change. + const repo = fixtureRepo('contract-unguarded-fields'); + repo.write( + 'packages/alpha/package.json', + manifest({ + description: 'now with a description', + scripts: { build: 'tsc', test: 'vitest run' }, + dependencies: { clsx: '^2.1.1' }, + publishConfig: { access: 'public' }, + }), + ); + repo.commit('chore(alpha): everything except the contract'); + + const run = runGate(repo.root, lastCommitRange(repo)); + expect(run.status).toBe(0); + expect(run.output).toMatch(/no changeset is owed/); + }); + + it('reformatting a manifest without moving a value is green — the diff is of VALUES, not bytes', () => { + // Why the fields are parsed rather than the file's mtime or its bytes read: + // re-indenting, or moving `version` above `name`, changes the file and moves + // no contract. + const repo = fixtureRepo('contract-reformat'); + repo.write( + 'packages/alpha/package.json', + `${JSON.stringify({ version: '1.0.0', name: '@fixture/alpha' }, null, 4)}\n`, + ); + repo.commit('style(alpha): reindent and reorder the manifest'); + + const run = runGate(repo.root, lastCommitRange(repo)); + expect(run.status).toBe(0); + expect(run.output).toMatch(/no changeset is owed/); + }); + + it("a `files` respelling that clause (c) reads as the SAME tarball is green — one normalisation, both readings", () => { + // `['./plugin.ts', 'dist/']` and `['plugin.ts', 'dist']` describe one + // tarball, and `publishedEntries` already says so for clause (c). If the + // field diff compared raw values instead, the gate could demand a + // declaration for a `files` edit its own clause (c) reports as changing + // nothing shipped — code contradicting itself inside a single run. + const repo = fixtureRepo('contract-files-respell'); + repo.write( + 'apps/console/package.json', + JSON.stringify( + { name: '@fixture/console', version: '1.0.0', files: ['./dist/', './plugin.ts', 'README.md'] }, + null, + 2, + ), + ); + repo.commit('chore(console): respell the files list'); + + const run = runGate(repo.root, lastCommitRange(repo)); + expect(run.status).toBe(0); + expect(run.output).toMatch(/no changeset is owed/); + }); + + it('an `exports` condition REORDER is red — condition order is resolution order in Node', () => { + // The half that makes the order-preserving comparison correct rather than + // merely convenient. Putting `types` after `default` is a real breakage and + // moves no key and no value, only their order. + const repo = fixtureRepo('contract-exports-reorder'); + repo.write( + 'packages/alpha/package.json', + manifest({ exports: { '.': { types: './dist/index.d.ts', default: './dist/index.js' } } }), + ); + repo.commit('feat(alpha): declare exports'); + repo.write('.changeset/first.md', '---\n"@fixture/alpha": patch\n---\n\nDeclare exports.\n'); + repo.commit('chore(alpha): declare it'); + + repo.write( + 'packages/alpha/package.json', + manifest({ exports: { '.': { default: './dist/index.js', types: './dist/index.d.ts' } } }), + ); + repo.commit('chore(alpha): reorder the exports conditions, undeclared'); + + const run = runGate(repo.root, lastCommitRange(repo)); + expect(run.status).toBe(1); + expect(run.output).toMatch(/exports/); + }); + + it('a contract move inside a package changesets IGNORES is skipped, not demanded', () => { + const repo = fixtureRepo('contract-ignored'); + repo.write( + 'packages/ignored-demo/package.json', + JSON.stringify({ name: '@fixture/ignored-demo', version: '1.0.0', sideEffects: false }, null, 2), + ); + repo.commit('chore(ignored-demo): sideEffects on a package no release covers'); + + const run = runGate(repo.root, lastCommitRange(repo)); + expect(run.status, 'changesets never versions it, so a declaration would be about nothing').toBe(0); + expect(run.output).toMatch(/1 file\(s\) under a package changesets ignores were skipped/); + }); + + it('the ROOT manifest is not a package contract', () => { + // The repository root publishes nothing. The reading is keyed to a + // DISCOVERED package directory, so only a workspace package's own manifest + // is ever read. + const repo = fixtureRepo('contract-root-manifest'); + repo.write('package.json', JSON.stringify({ name: 'fixture-root', private: true, sideEffects: false }, null, 2)); + repo.commit('chore: sideEffects at the repository root'); + + const run = runGate(repo.root, lastCommitRange(repo)); + expect(run.status).toBe(0); + expect(run.output).toMatch(/no changeset is owed/); + }); + + it('a manifest that will not parse is a LOUD failure, not a quiet pass', () => { + // Same direction as every other missing input here: this gate decides + // whether a declaration is owed, so "cannot tell" is a red build. + const repo = fixtureRepo('contract-unparseable'); + repo.write('packages/alpha/package.json', '{ "name": "@fixture/alpha", "sideEffects": [ }\n'); + repo.commit('chore(alpha): break the manifest'); + + const run = runGate(repo.root, lastCommitRange(repo)); + expect(run.status).toBe(1); + expect(run.output).toMatch(/cannot tell whether one is owed/); + }); + + it('the FIELD reading and clause (c) stay two different questions about one file', () => { + // `isPublishedSource` is path-only and must remain so: a manifest edit can + // never be answered by "the file changed", or `version` bumps come back in + // through the population rather than through the field list. + expect(isPublishedSource('package.json', { files: ['dist', 'plugin.ts', 'README.md'] })).toBe(false); + expect(isPublishedSource('package.json', { files: ['package.json'] })).toBe(true); + + const packages = discoverPackages(repoRoot, readReleaseConfig(repoRoot)); + expect( + classifyChangedPaths(['apps/console/package.json', 'packages/i18n/package.json'], packages).guarded, + 'the manifest is still not published SOURCE — it is judged by its field values instead', + ).toEqual([]); + }); + + it('pins CONTRACT_FIELDS — the list is a RULING, so changing it must be a deliberate edit here', () => { + // objectui#6736's triage: the field list is implementable as given, and + // "⛔ 若实现者想增删该清单，停下回帖 —— 那才是需要裁决的一步". A test that + // pins the exact list is what makes that mechanical rather than a hope. + expect(CONTRACT_FIELDS).toEqual([ + 'engines', + 'exports', + 'files', + 'main', + 'module', + 'peerDependencies', + 'sideEffects', + 'types', + ]); + expect(CONTRACT_FIELDS, 'the two named exclusions are properties of the LIST, not branches').not.toContain( + 'version', + ); + expect(CONTRACT_FIELDS).not.toContain('devDependencies'); + expect(CONTRACT_FIELDS, 'kept out deliberately — the pre-#6736 trade is narrowed, not abandoned').not.toContain( + 'dependencies', + ); + }); + + it('manifestFieldsChanged — appearing, disappearing, and equal-by-normalisation', () => { + expect(manifestFieldsChanged({ name: 'a' }, { name: 'a' })).toEqual([]); + // Absent -> present and present -> absent are both moves, and `false` is a + // real `sideEffects` value that must not read as "not there". + expect(manifestFieldsChanged({}, { sideEffects: false })).toEqual(['sideEffects']); + expect(manifestFieldsChanged({ sideEffects: false }, {})).toEqual(['sideEffects']); + // An absent `files` is not the same declaration as an empty one: absent + // publishes the directory, `[]` does not. + expect(manifestFieldsChanged({}, { files: [] })).toEqual(['files']); + expect(manifestFieldsChanged({ files: ['dist'] }, { files: ['./dist/'] })).toEqual([]); + expect(manifestFieldsChanged({ files: ['dist'] }, { files: ['dist', 'plugin.ts'] })).toEqual(['files']); + // A whole manifest arriving or leaving — a package added or removed. + expect(manifestFieldsChanged(null, { main: './index.js', version: '1.0.0' })).toEqual(['main']); + expect(manifestFieldsChanged({ main: './index.js' }, null)).toEqual(['main']); + expect(manifestFieldsChanged(null, null)).toEqual([]); + // Reported in CONTRACT_FIELDS order, so a failure names them the same way + // whatever moved. + expect(manifestFieldsChanged({}, { types: './d.ts', engines: { node: '>=22' }, main: './i.js' })).toEqual([ + 'engines', + 'main', + 'types', + ]); + // Unguarded fields are invisible to it, however they move. + expect(manifestFieldsChanged({ version: '1.0.0' }, { version: '2.0.0', scripts: { build: 'tsc' } })).toEqual([]); + }); +}); + // ── 3. the verdicts ────────────────────────────────────────────────────────── describe('a change to guarded source must declare a changeset', () => { diff --git a/scripts/check-changeset-presence.mjs b/scripts/check-changeset-presence.mjs index 1944bddc34..05cc46b835 100644 --- a/scripts/check-changeset-presence.mjs +++ b/scripts/check-changeset-presence.mjs @@ -134,9 +134,13 @@ * * Deliberate boundaries, stated so they are not mistaken for oversights: * - * - **`package.json` itself never counts.** Clause (c) reads that file; it does - * not match it. A dependency bump can be just as user-visible, and this gate - * still does not see it. That was the first draft's trade and it is kept. + * - **`package.json` is not a FILE in this population, and never becomes one.** + * Clause (c) reads that file; it does not match it, and no clause added + * since does either. What changed under objectui#6736 is a different + * question asked of the same file — see "The manifest's published contract" + * below — and it is deliberately not a fourth clause: `isPublishedSource` + * stays path-only, so a manifest edit can never be answered by "the file + * changed". * - **`/public/` does not count.** Vite copies it verbatim into the * published `dist/`, so it genuinely IS published — a favicon, a logo, a PWA * manifest. None of it is executable, and demanding a declaration for a logo @@ -160,6 +164,93 @@ * line. The alternative — teaching the gate which files "don't count" — is * where the holes live, and this gate asks for a sentence, not a release. * + * ## The manifest's published contract — a FIELD reading, not a file (objectui#6736) + * + * Everything above answers "did a published FILE change". A package's published + * contract is not only files: `sideEffects` tells every consumer's bundler what + * it may drop, `exports` decides what a consumer can import at all, `files` + * decides what is in the tarball. None of those live under `src`, all of them + * are read by consumers, and until objectui#6736 a change to any of them was + * owed no declaration. The live instance the card was filed on is PR #6735, + * which gave `@object-ui/app-shell` a `sideEffects` ARRAY — modules the array + * does not name became droppable in every consumer's build — and got this + * gate's own verdict, quoted as it read then: + * + * 7 file(s) changed, 0 of them published source of a package the release + * covers, 0 under a package changesets ignores, 1 changeset(s) added. + * OK No source of a released package changed in this range, so no + * changeset is owed. + * + * That changeset was there because its author decided it was owed. Nothing + * asked for it. + * + * ### The list is fixed, and adding to it is a ruling + * + * `CONTRACT_FIELDS` below is the list objectui#6736 named and triage ratified as + * implementable-as-given. It is NOT a starting point to grow by taste: the + * triage comment says in as many words that adding or dropping a field is the + * step that needs a decision, not an implementation choice. What is in it: + * `sideEffects`; `exports`, `main`, `module`, `types`; `files`; + * `peerDependencies`, `engines`. + * + * ### What stays out, and why that is the SAME trade as before + * + * `version`, `dependencies`, `devDependencies`, `scripts`, `name`, + * `publishConfig` and everything else are not read. Two of those are named + * exclusions rather than leftovers, and they are the reason this is a field + * reading rather than a file one: + * + * - **`version`** is written by `changeset version` itself. A gate that went + * red on it would go red on the release commit that answers it. + * - **`devDependencies`** is what Dependabot bumps. Demanding a changeset per + * bot bump is the noise this gate cannot afford. + * + * Neither is special-cased in the code, and that is the point worth keeping: + * they are excluded BY CONSTRUCTION, because the reading is an allowlist of + * eight fields and they are not in it. There is no branch to forget, no bot + * identity to sniff, and no commit message to parse — the two exclusions the + * card names are properties of the list, and `check-changeset-presence.test.ts` + * exercises both against real fixtures so that stays true. + * + * `dependencies` staying out is the part of the old trade that is KEPT rather + * than reversed: a runtime dependency bump can be just as user-visible as any + * of the eight, and this gate still does not see it. That was the first draft's + * trade, it is narrowed here rather than abandoned, and it is stated so nobody + * reads "the manifest is guarded now" off this section. + * + * ### Two readings of the same file, and why they do not collide + * + * `package.json` was already being read — clause (c) asks its `files` list + * whether some OTHER changed path ships. That reading is about a different + * path, takes the manifest as it stands AFTER the change (the tarball a + * consumer will get is the one the new list describes), and is unchanged here. + * The field reading asks whether the manifest's own guarded values MOVED, so it + * needs both sides and reads them out of git. + * + * Where the two readings touch — `files` is in both — they are held to one + * normalisation: `publishedEntries` is what clause (c) compares against, and it + * is what the field diff compares too, so `dist` and `dist/` are one value to + * both. Without that, the gate could demand a declaration for a `files` edit + * that its own clause (c) reports as changing nothing shipped: code + * contradicting itself inside one run. + * + * ### Why a diff of parsed values, and not the file's mtime + * + * Reformatting a manifest, sorting its keys, or bumping its version touches the + * file. Only a value moving inside one of the eight fields is a contract + * change, so both sides are PARSED and the guarded fields compared value by + * value. Comparison is order-preserving serialisation, which is correct rather + * than merely convenient: `exports` condition order is resolution order in + * Node, so reordering conditions IS a behaviour change and must not compare + * equal. For `peerDependencies` and `engines` a pure key reorder would be + * reported too — accepted, and cheap: it costs one empty-frontmatter changeset, + * and nobody reorders those keys without meaning something. + * + * An unparseable manifest on EITHER side is a hard failure, like every other + * unreadable input here (see "Never silent" below). A manifest absent on one + * side — a package added or removed — reads as every guarded field appearing + * or disappearing, which is what it is. + * * ## The empty-frontmatter exemption * * A changeset whose frontmatter declares no package (`---` immediately followed @@ -700,6 +791,153 @@ export function classifyChangedPaths(paths, packages) { return { guarded, unclassified, skipped }; } +// -- the manifest's published contract (objectui#6736) ------------------------ + +/** A package's own manifest, by name. */ +export const MANIFEST = 'package.json'; + +/** + * The `package.json` fields whose VALUE is the package's published contract. + * + * FIXED BY RULING, not by taste. objectui#6736 named this list and triage + * ratified it as implementable as given, with the explicit rider that adding or + * dropping a field is a decision to be taken on the card and not in a patch. The + * header section "The manifest's published contract" carries the reasoning, + * including what is deliberately absent (`version`, `dependencies`, + * `devDependencies`, `scripts`) and why the two exclusions the card names fall + * out of the list itself rather than out of a special case. + * + * Sorted alphabetically so the fields a failure names come out in one order + * whatever moved. + */ +export const CONTRACT_FIELDS = [ + 'engines', + 'exports', + 'files', + 'main', + 'module', + 'peerDependencies', + 'sideEffects', + 'types', +]; + +/** + * A field that is not present at all. + * + * A bare word rather than a sentinel object because it is compared against + * `JSON.stringify` output, which can never produce it: a string serialises WITH + * its quotes, and every other type starts with a digit, a sign, `t`, `f`, `n`, + * `[` or `{`. So "absent" and "the value is the string absent" stay two + * different readings. Not a control byte, deliberately — see the `NUL` comment + * above for what one of those costs a repository. + */ +const ABSENT = 'absent'; + +/** The one field both readings of the manifest share. */ +const MANIFEST_FILES = 'files'; + +/** + * One guarded field of one manifest, as a comparable string. + * + * `files` goes through `publishedEntries` — the SAME normalisation clause (c) + * compares changed paths against — so `['dist']` and `['dist/']` are one value + * to both readings of that field. Anything else is compared by + * order-preserving serialisation: `exports` condition order is resolution order + * in Node, so a reorder is a behaviour change and must not compare equal. + * + * @param {Record | null} manifest parsed manifest, or null when + * the file is absent on that side of the diff + * @param {string} field + */ +function fieldValue(manifest, field) { + if (manifest === null) return ABSENT; + if (!Object.prototype.hasOwnProperty.call(manifest, field)) return ABSENT; + if (field === MANIFEST_FILES) return JSON.stringify(publishedEntries(manifest)); + return JSON.stringify(manifest[field]); +} + +/** + * Which of `CONTRACT_FIELDS` moved between two parsed manifests. + * + * Either side may be `null` (the manifest does not exist there): a package added + * or removed reads as every guarded field it declares appearing or + * disappearing, which is what happened. + * + * @param {Record | null} before + * @param {Record | null} after + * @returns {string[]} field names, in `CONTRACT_FIELDS` order + */ +export function manifestFieldsChanged(before, after) { + return CONTRACT_FIELDS.filter((field) => fieldValue(before, field) !== fieldValue(after, field)); +} + +/** + * One manifest as parsed JSON at `ref`, or `null` when it is not there. + * + * `ref === null` means the WORKING TREE, matching `changedFiles`' default "after" + * side and `check-changeset-overwrite`'s `contentAt`: an author running this + * locally is judged on what is on disk, before committing. + * + * A manifest that will not parse THROWS. It is an unreadable input like any + * other here, and the direction is the one "Never silent" fixes: this gate + * decides whether a declaration is owed, so "cannot tell" is a red build. + */ +function manifestAt(root, ref, file) { + let source; + if (ref === null) { + const onDisk = join(root, file); + source = existsSync(onDisk) ? readFileSync(onDisk, 'utf8') : null; + } else { + source = gitQuiet(root, ['show', `${ref}:${file}`]); + } + if (source === null) return null; + try { + return JSON.parse(source); + } catch (error) { + throw new Error( + `cannot parse ${file} at ${ref === null ? 'the working tree' : ref}: ${error.message}`, + ); + } +} + +/** + * The changed manifests whose published contract moved, split the same three + * ways as `classifyChangedPaths`. + * + * Only a WORKSPACE PACKAGE's own manifest is read: the path has to be exactly + * `` + `/package.json`. The repository root + * manifest publishes nothing, and a `package.json` deeper inside a package is a + * fixture or a nested config, not that package's contract. + * + * `unclassified` carries the same meaning it does for source — a package in + * neither the `fixed` group nor `ignore`, which this gate refuses to guess + * about — so the two classifications compose into one verdict instead of two. + * + * @param {string[]} paths changed paths, as `changedFiles` returned them + * @param {Map} packages + */ +export function contractChanges(root, { base, head = null, paths, packages }) { + const guarded = []; + const unclassified = []; + const skipped = []; + + for (const path of paths) { + if (!path.endsWith(`/${MANIFEST}`)) continue; + const directory = path.slice(0, path.length - MANIFEST.length - 1); + const pkg = packages.get(directory); + if (pkg === undefined) continue; + + const fields = manifestFieldsChanged(manifestAt(root, base, path), manifestAt(root, head, path)); + if (fields.length === 0) continue; + + const entry = { file: path, pkg: pkg.name, directory, fields }; + if (pkg.versioned) guarded.push(entry); + else if (pkg.ignored) skipped.push(entry); + else unclassified.push(entry); + } + return { guarded, unclassified, skipped }; +} + // -- declarations ------------------------------------------------------------- /** @@ -766,8 +1004,9 @@ export function addedDeclarations(root, { base, head = null }) { /** * @typedef {object} Analysis * @property {{ file: string, pkg: string, directory: string }[]} guarded - * @property {{ file: string, pkg: string, directory: string }[]} unclassified - * @property {{ file: string, pkg: string, directory: string }[]} skipped + * @property {{ file: string, pkg: string, directory: string, fields: string[] }[]} contract + * @property {{ file: string, pkg: string, directory: string, fields?: string[] }[]} unclassified + * @property {{ file: string, pkg: string, directory: string, fields?: string[] }[]} skipped * @property {{ file: string, declaration: { kind: string, entries?: number } }[]} declarations * @property {number} changedFileCount */ @@ -782,11 +1021,13 @@ export function analyze(root, { base, head = null }) { const config = readReleaseConfig(root); const packages = discoverPackages(root, config); const paths = changedFiles(root, { base, head }); - const { guarded, unclassified, skipped } = classifyChangedPaths(paths, packages); + const source = classifyChangedPaths(paths, packages); + const contract = contractChanges(root, { base, head, paths, packages }); return { - guarded, - unclassified, - skipped, + guarded: source.guarded, + contract: contract.guarded, + unclassified: [...source.unclassified, ...contract.unclassified], + skipped: [...source.skipped, ...contract.skipped], declarations: addedDeclarations(root, { base, head }), changedFileCount: paths.length, }; @@ -802,10 +1043,28 @@ export const usableDeclarations = (analysis) => analysis.declarations.filter((d) */ export function verdict(analysis) { if (analysis.unclassified.length > 0) return 1; - if (analysis.guarded.length === 0) return 0; + if (analysis.guarded.length + analysis.contract.length === 0) return 0; return usableDeclarations(analysis).length > 0 ? 0 : 1; } +/** + * What changed, as one noun phrase both the red and the green message open with. + * + * Source files and contract changes are counted separately because they are + * answered by looking at different things — a path, and a field's value — and a + * message that merged them would send an author to grep for a file that is + * green. + * + * @param {Analysis} analysis + */ +export function subjectPhrase(analysis) { + const parts = []; + if (analysis.guarded.length > 0) parts.push(`${analysis.guarded.length} source file(s)`); + if (analysis.contract.length > 0) parts.push(`${analysis.contract.length} published-contract change(s)`); + const packages = new Set([...analysis.guarded, ...analysis.contract].map((entry) => entry.pkg)); + return `${parts.join(' and ')} of ${packages.size} released package(s)`; +} + // -- CLI ---------------------------------------------------------------------- const invokedDirectly = isEntrypoint(import.meta.url); @@ -861,17 +1120,18 @@ if (invokedDirectly) { console.log( `Compared ${head ?? 'the working tree'} with ${base.ref.slice(0, 9)} (${base.how}): ` + `${analysis.changedFileCount} file(s) changed, ${analysis.guarded.length} of them published ` + - `source of a package the release covers, ${analysis.skipped.length} under a package changesets ` + + `source of a package the release covers, ${analysis.contract.length} of them a manifest whose ` + + `published contract moved, ${analysis.skipped.length} under a package changesets ` + `ignores, ${analysis.declarations.length} changeset(s) added.`, ); if (analysis.unclassified.length > 0) { const packages = [...new Set(analysis.unclassified.map((entry) => `${entry.pkg} (${entry.directory})`))]; console.error( - `\n❌ ${analysis.unclassified.length} changed source file(s) belong to a package that is in ` + + `\n❌ ${analysis.unclassified.length} changed file(s) belong to a package that is in ` + 'neither the `fixed` group nor `ignore` of .changeset/config.json:\n' + packages.map((name) => ` • ${name}`).join('\n') + - '\n\n So this gate cannot tell whether that source ships, and it will not guess "no" —\n' + + '\n\n So this gate cannot tell whether that package ships, and it will not guess "no" —\n' + ' that would leave the newest package in the repository the one nothing guards.\n' + ' Classify it: `node scripts/check-changeset-fixed.mjs` is the gate that owns this,\n' + ' and its message says where to add the name.', @@ -879,9 +1139,10 @@ if (invokedDirectly) { process.exit(1); } - if (analysis.guarded.length === 0) { + if (analysis.guarded.length + analysis.contract.length === 0) { console.log( - '✅ No source of a released package changed in this range, so no changeset is owed.' + + '✅ No source or published contract of a released package changed in this range, so no ' + + 'changeset is owed.' + (analysis.skipped.length > 0 ? `\n (${analysis.skipped.length} file(s) under a package changesets ignores were skipped.)` : ''), @@ -892,9 +1153,7 @@ if (invokedDirectly) { if (usable.length > 0) { const releaseNothing = usable.every((d) => d.declaration.entries === 0); console.log( - `✅ ${analysis.guarded.length} source file(s) of ${ - new Set(analysis.guarded.map((entry) => entry.pkg)).size - } released package(s) changed, and this change declares ` + + `✅ ${subjectPhrase(analysis)} changed, and this change declares ` + `${usable.length} changeset(s): ${usable.map((d) => d.file).join(', ')}.` + (releaseNothing ? '\n Every one of them has an EMPTY frontmatter — declared as releasing nothing, which\n' + @@ -910,20 +1169,20 @@ if (invokedDirectly) { process.exit(0); } + // A contract change names its FIELDS. Without them the line reads "your + // package.json changed", which is true of a version bump this gate passes — + // an author sent to look for a change the gate did not object to. const byPackage = new Map(); - for (const entry of analysis.guarded) { + for (const entry of [...analysis.guarded, ...analysis.contract]) { if (!byPackage.has(entry.pkg)) byPackage.set(entry.pkg, []); - byPackage.get(entry.pkg).push(entry.file); + byPackage.get(entry.pkg).push(entry.fields ? `${entry.file} — ${entry.fields.join(', ')}` : entry.file); } - console.error( - `\n❌ ${analysis.guarded.length} source file(s) of ${byPackage.size} released package(s) ` + - 'changed, and this change adds no changeset:\n', - ); - for (const [pkg, files] of [...byPackage].sort()) { + console.error(`\n❌ ${subjectPhrase(analysis)} changed, and this change adds no changeset:\n`); + for (const [pkg, lines] of [...byPackage].sort()) { console.error(` ${pkg}`); - for (const file of files.slice(0, 5)) console.error(` ${file}`); - if (files.length > 5) console.error(` … and ${files.length - 5} more`); + for (const line of lines.slice(0, 5)) console.error(` ${line}`); + if (lines.length > 5) console.error(` … and ${lines.length - 5} more`); } if (unreadable.length > 0) { console.error( @@ -947,6 +1206,13 @@ if (invokedDirectly) { Test-only change to the grid column resolver; no published behaviour changes. + A published-contract line above names FIELDS rather than a file: what moved is one + of ${CONTRACT_FIELDS.join(', ')} in that package's + package.json, and each of those is read by consumers of the tarball. \`version\`, + \`dependencies\`, \`devDependencies\` and \`scripts\` are NOT guarded — a + \`changeset version\` bump and a Dependabot devDependency bump go green here by + construction (objectui#6736). + Scored \`minor\` at most, never \`major\`: every package is in one fixed group, so one major carries all of them off the @objectstack major this repo is pinned to (AGENTS.md §版本号策略, enforced by scripts/check-changeset-no-major.mjs).