diff --git a/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.i18n.yaml b/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.i18n.yaml index 59b51bbe6b..851f0e9d2d 100644 --- a/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.i18n.yaml +++ b/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/process/2026-08-10-npm-release-sequences.md -2026-08-10-npm-release-sequences.md: e74a4ac8f2aadd8665ec0db198c6a317a0c201bc -2026-08-10-npm-release-sequences.zh.md: e152163976f945224f2524fccd7f831ba98e8161 +2026-08-10-npm-release-sequences.md: d8495f158482d5d6e06a1752a096d1e9200b6070 +2026-08-10-npm-release-sequences.zh.md: 24b466f6b7b10d31ac2e025da6e12ec3c91c7548 diff --git a/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.md b/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.md index e74a4ac8f2..d8495f1584 100644 --- a/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.md +++ b/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.md @@ -80,6 +80,14 @@ Every reference to a workspace member uses `workspace:^`, so `pnpm pack` substit `scripts/check-workspace-constraints.ts` requires the protocol, so a new package cannot reintroduce a hand-written range; the invariant-companion rule requires `workspace:^` for `@deepseek-ai/dsh-invariants` for the same reason. +### An optional dependency is never loaded at module scope + +A dependency in `optionalDependencies`, or a peer carrying `peerDependenciesMeta..optional`, may be absent from an installed tree — that absence is the whole promise of "optional". A static import is evaluated when the importing module loads, so one absent package stops being "this capability is unavailable" and becomes a load failure for everything that reaches the importing module. The failure appears only in an installed tree missing that package, and no test here constructs one: a workspace install always has every package, so the unit tests, the snapshots, and the packed-install probe all pass while the published package is broken for the consumer who declined the optional peer. + +[`verify-optional-dependency-imports`](../../../../scripts/verify-optional-dependency-imports.ts) closes that hole. It reads each package's own manifest for what that package allows to be absent, then scans the files that ship — `packages/*/*/src/` and `apps/*/src/` — across both compiler faces. `vendor/` is out of scope, as pinned upstream source under the [vendoring policy](../../../../vendor/README.md). Value-versus-type is decided against a bound Program rather than the import syntax, because `verbatimModuleSyntax` is off: the compiler already erases an import whose bindings resolve to types, so `import type {}`, `import {}`, an inline `type` specifier, and a named binding that resolves to a type all emit nothing and are allowed, while a bare import, a value binding, and a star re-export are kept and rejected. Only the type phase erases an import: `import defer` still resolves and links its module, deferring evaluation alone, so the gate counts it as a load. + +A violation names the package, the declaration that made it optional, and the way out in order — import it as a type, which is all that declaration merging needs, or restructure so module scope does not need the package. A dynamic `import()` only moves the failure to first use, so it belongs to a caller that genuinely requires the package and handles its absence; reaching for it is a sign the dependency is not optional, and the gate does not offer it as the remedy. + ### Release family objects The entity in this domain is a **release family**: a set of packages sharing one version baseline and tag naming that publishes as a unit. Adding a family means adding a subclass and a workflow lane, not changing the core. @@ -88,9 +96,9 @@ The entity in this domain is a **release family**: a set of packages sharing one |---|---| | `ReleaseFamily` | a family's identity: member discovery, version baseline, tag prefix, packed-payload rule, installed entry | | `ReleaseMember` | one publishable package: directory, name, version, manifest | -| `publishOrder` | topological order over runtime dependencies, ties broken by package name; a cycle is reported rather than resolved arbitrarily | +| `publishOrder` | topological order over the sections npm installs plus peer declarations, ties broken by package name; a cycle among installed dependencies is reported rather than resolved arbitrarily, and a peer edge no order can honour is dropped and named | | `pack` | packs a whole family into one directory and records the upload order | -| `verify` | the family's version baseline, and — when publishing — that the run comes from that family's tag and its members are publishable | +| `verify` | the family's version baseline, the publish order it prints in full, and — when publishing — that the run comes from that family's tag and its members are publishable | | `verify-packed-install` | installs the tarballs of one or more pack directories into a throwaway consumer and drives the installed executable | | `publish` | the three registry states above | | `process` / `tarball` | the one home for spawning commands and for reading a packed tarball, including the entry guard that keeps every script importable | diff --git a/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.zh.md b/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.zh.md index e152163976..24b466f6b7 100644 --- a/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.zh.md +++ b/.agents/notes/implemented/process/2026-08-10-npm-release-sequences.zh.md @@ -80,6 +80,14 @@ registry 的两个行为决定了「怎么尝试一次发布」。写入之间 `scripts/check-workspace-constraints.ts` 要求这个协议，所以新包无法再引入硬写的范围；同理，invariant companion 规则要求 `@deepseek-ai/dsh-invariants` 用 `workspace:^`。 +### optional 依赖绝不在模块作用域被加载 + +`optionalDependencies` 里的依赖，或带 `peerDependenciesMeta..optional` 的 peer，在安装出来的树里可以不存在——这份「可以不存在」正是 optional 的全部承诺。而静态 import 在引入方模块加载时就求值，于是一个缺失的包不再表现为「这个能力不可用」，而是变成所有能走到该模块的代码的加载失败。这种失败只在「缺了该包的安装树」里出现，而本仓没有任何测试构造这种树：workspace 安装总是把每个包都装上，所以单测、快照、打包安装探针全都会过，而那个拒绝了这个 optional peer 的消费者拿到的却是坏的包。 + +[`verify-optional-dependency-imports`](../../../../scripts/verify-optional-dependency-imports.ts) 堵掉这个洞。它从每个包自己的 manifest 读取「这个包允许谁缺失」，再扫描会发布出去的文件——`packages/*/*/src/` 与 `apps/*/src/`——且两个编译门面各扫一遍。`vendor/` 不在范围内，那是[受 vendoring 政策管辖](../../../../vendor/README.md)的固定上游源码。值与类型的判定对着绑定好的 Program 做，而不是看 import 写法，因为 `verbatimModuleSyntax` 是关的：编译器本来就会消除绑定解析为类型的 import，所以 `import type {}`、`import {}`、内联 `type` 说明符、以及解析为类型的具名绑定都不产生产物、一律放行，而裸 import、值绑定、星号 re-export 会被保留、一律报错。只有 type 相位会消除 import：`import defer` 仍然解析并链接它的模块，只推迟求值，所以门禁把它算作一次加载。 + +报错会点名这个包、点名是哪条声明把它标成 optional 的，并按顺序给出出路——把它作为类型引入（声明合并需要的仅此而已），或者调整写法让模块作用域不再需要这个包。动态 `import()` 只是把失败推迟到首次使用，它属于那种确实需要这个包、并且自己处理缺失的调用方；会想到它，往往说明这个依赖并不 optional，所以门禁不把它作为解法给出。 + ### 发布族对象 这个领域里的实体是**发布族**：一组共享版本基线与 tag 命名、可整体发布的包。新增一族等于加一个子类和一条 workflow lane，不改核心。 @@ -88,9 +96,9 @@ registry 的两个行为决定了「怎么尝试一次发布」。写入之间 |---|---| | `ReleaseFamily` | 一族的身份：成员发现、版本基线、tag 前缀、打包 payload 规则、已安装入口 | | `ReleaseMember` | 一个可发布包：目录、包名、版本、manifest | -| `publishOrder` | 按运行时依赖的拓扑序，同层按包名排；遇到环是报错而不是随意定序 | +| `publishOrder` | 按 npm 会安装的依赖段加 peer 声明做拓扑序，同层按包名排；安装依赖成环是报错而不是随意定序，任何排不进去的 peer 边被丢弃并点名 | | `pack` | 把整族打进一个目录并记录上传顺序 | -| `verify` | 族的版本基线；发布时还要求本次运行来自该族的 tag、且成员可发布 | +| `verify` | 族的版本基线、完整打印出来的发布顺序；发布时还要求本次运行来自该族的 tag、且成员可发布 | | `verify-packed-install` | 把一个或多个 pack 目录的 tarball 装进一次性 consumer，并驱动已安装的可执行入口 | | `publish` | 上面那三态 | | `process` / `tarball` | 启动命令、读取打包 tarball 的唯一正家，其中的入口守卫让每个脚本都可被 import | diff --git a/package.json b/package.json index 1fd63bae9a..517d0c56d1 100644 --- a/package.json +++ b/package.json @@ -95,6 +95,7 @@ "website:build": "pnpm run docs:build", "verify-package-readme-limitations": "tsx scripts/verify-package-readme-limitations.ts", "verify-node-next-types": "tsx scripts/verify-node-next-types.ts", + "verify-optional-dependency-imports": "tsx scripts/verify-optional-dependency-imports.ts", "verify-runtime-closure": "tsx scripts/verify-runtime-closure.ts", "verify-vendored-links": "tsx scripts/verify-vendored-links.ts", "verify-cordis-config": "tsx scripts/verify-cordis-config.ts", @@ -125,7 +126,7 @@ "verify-module-graph": "tsx scripts/gen-module-graph.ts --check", "constraints": "tsx scripts/check-workspace-constraints.ts", "doc-sync": "tsx scripts/run-gates.ts doc-sync", - "hygiene": "pnpm run rescope-vendor:check && pnpm run knip && pnpm run publint && pnpm run constraints && pnpm run verify-dsh-package-licenses && pnpm run verify-package-invariants && pnpm run verify-built-package-invariants && pnpm run verify-cordis-config && pnpm run verify-node-next-types && pnpm run verify-runtime-closure && pnpm run verify-vendored-links", + "hygiene": "pnpm run rescope-vendor:check && pnpm run knip && pnpm run publint && pnpm run constraints && pnpm run verify-dsh-package-licenses && pnpm run verify-package-invariants && pnpm run verify-built-package-invariants && pnpm run verify-cordis-config && pnpm run verify-node-next-types && pnpm run verify-optional-dependency-imports && pnpm run verify-runtime-closure && pnpm run verify-vendored-links", "publish:npm-baseline": "tsx scripts/publish-npm-baseline.ts", "release:dsh": "tsx scripts/release/bump.ts --family dsh", "release:vendor": "tsx scripts/release/bump.ts --family vendor", diff --git a/scripts/release/families.spec.ts b/scripts/release/families.spec.ts index 369fc62b16..66c3daf83f 100644 --- a/scripts/release/families.spec.ts +++ b/scripts/release/families.spec.ts @@ -57,7 +57,7 @@ describe('release families', () => { member('packages/a/zebra', '@deepseek-ai/dsh-zebra'), ] - expect(dsh.publishOrder(members).map(entry => entry.name)).toEqual([ + expect(dsh.publishOrder(members).order.map(entry => entry.name)).toEqual([ '@deepseek-ai/dsh-library', '@deepseek-ai/dsh-consumer', '@deepseek-ai/dsh-zebra', @@ -74,6 +74,92 @@ describe('release families', () => { expect(() => { dsh.publishOrder(members) }).toThrow(/dependency cycle/) }) + it('publishes a peer before its consumer', () => { + const dsh = releaseFamily('dsh') + const members = [ + member('packages/a/consumer', '@deepseek-ai/dsh-consumer', { peerDependencies: { '@deepseek-ai/dsh-zebra': 'workspace:^' } }), + member('packages/a/zebra', '@deepseek-ai/dsh-zebra'), + ] + + // Name order alone would place the consumer first; the peer edge moves it. + expect(dsh.publishOrder(members).order.map(entry => entry.name)).toEqual([ + '@deepseek-ai/dsh-zebra', + '@deepseek-ai/dsh-consumer', + ]) + }) + + it('orders around a peer cycle rather than refusing to publish, and reports the edge it dropped', () => { + const dsh = releaseFamily('dsh') + const members = [ + member('packages/a/left', '@deepseek-ai/dsh-left', { peerDependencies: { '@deepseek-ai/dsh-right': 'workspace:^' } }), + member('packages/a/right', '@deepseek-ai/dsh-right', { peerDependencies: { '@deepseek-ai/dsh-left': 'workspace:^' } }), + ] + + // Sibling packages declare each other as peers, and npm treats an unmet peer + // as a warning, so this pair has to publish rather than fail the release. + const plan = dsh.publishOrder(members) + expect(plan.order.map(entry => entry.name)).toEqual([ + '@deepseek-ai/dsh-right', + '@deepseek-ai/dsh-left', + ]) + // One of the two edges has to give, and which one it is belongs in the log. + expect(plan.droppedPeerEdges).toEqual([ + { consumer: '@deepseek-ai/dsh-right', peer: '@deepseek-ai/dsh-left' }, + ]) + }) + + it('honours an install edge even when a peer cycle surrounds it', () => { + const dsh = releaseFamily('dsh') + const members = [ + member('packages/a/base', '@deepseek-ai/dsh-base', { peerDependencies: { '@deepseek-ai/dsh-consumer': 'workspace:^' } }), + member('packages/a/consumer', '@deepseek-ai/dsh-consumer', { + dependencies: { '@deepseek-ai/dsh-base': 'workspace:^' }, + peerDependencies: { '@deepseek-ai/dsh-base': 'workspace:^' }, + }), + ] + + // The install edge is absolute: base publishes first, and the peer edge that + // would reverse it is the one dropped. + const plan = dsh.publishOrder(members) + expect(plan.order.map(entry => entry.name)).toEqual([ + '@deepseek-ai/dsh-base', + '@deepseek-ai/dsh-consumer', + ]) + expect(plan.droppedPeerEdges).toEqual([ + { consumer: '@deepseek-ai/dsh-base', peer: '@deepseek-ai/dsh-consumer' }, + ]) + }) + + it('refuses an order that would publish a consumer before a dependency it installs', () => { + const dsh = releaseFamily('dsh') + const members = [ + member('packages/a/alpha', '@deepseek-ai/dsh-alpha', { peerDependencies: { '@deepseek-ai/dsh-bravo': 'workspace:^' } }), + member('packages/a/bravo', '@deepseek-ai/dsh-bravo', { peerDependencies: { '@deepseek-ai/dsh-charlie': 'workspace:^' } }), + member('packages/a/charlie', '@deepseek-ai/dsh-charlie', { dependencies: { '@deepseek-ai/dsh-alpha': 'workspace:^' } }), + ] + + // A cycle of two peer edges closed by one install edge: dropping a peer edge + // would order this, and the traversal drops the install edge instead. That + // order would publish charlie before the alpha it installs, so it is refused + // here rather than published. + expect(() => { dsh.publishOrder(members) }).toThrow(/no publish order honours @deepseek-ai\/dsh-charlie -> @deepseek-ai\/dsh-alpha/) + }) + + it('ignores devDependencies when ordering', () => { + const dsh = releaseFamily('dsh') + const members = [ + member('packages/a/alpha', '@deepseek-ai/dsh-alpha', { devDependencies: { '@deepseek-ai/dsh-zebra': 'workspace:^' } }), + member('packages/a/zebra', '@deepseek-ai/dsh-zebra'), + ] + + // A dev dependency is absent from the published package, so it must not move + // the consumer behind it. + expect(dsh.publishOrder(members).order.map(entry => entry.name)).toEqual([ + '@deepseek-ai/dsh-alpha', + '@deepseek-ai/dsh-zebra', + ]) + }) + it('applies the harness payload policy to dsh and keeps upstream payloads for vendored packages', () => { const dsh = releaseFamily('dsh') const vendor = releaseFamily('vendor') diff --git a/scripts/release/families.ts b/scripts/release/families.ts index d39552636d..f43939f17d 100644 --- a/scripts/release/families.ts +++ b/scripts/release/families.ts @@ -13,12 +13,47 @@ import { globSync, readFileSync } from 'node:fs' import { resolve } from 'node:path' import { validateTarballPayload } from '../publication-payload.ts' -/** Dependency sections that constrain publish order: a consumer must publish after its dependency. */ -const ORDER_SECTIONS = ['dependencies', 'optionalDependencies'] as const +/** + * Dependency sections a consumer must publish after, because npm resolves them + * when the package is installed: publishing a consumer first would leave a + * window where its own tree cannot be assembled. + */ +const INSTALL_SECTIONS = ['dependencies', 'optionalDependencies'] as const + +/** + * Peer declarations also order the publication, but they cannot constrain it. + * npm never installs a peer on the package's behalf — an unmet peer is a + * warning, not a resolution failure — and sibling packages legitimately declare + * each other as peers, which makes these edges the ones that close cycles. They + * order what they can and are dropped where they would deadlock. + */ +const PEER_SECTIONS = ['peerDependencies'] as const /** The workspace root manifest, which is never a release member. */ const WORKSPACE_ROOT_PACKAGE = '@deepseek-ai/dsh-root' +/** One peer declaration the publish order leaves unordered. */ +interface DroppedPeerEdge { + /** Package declaring the peer. */ + readonly consumer: string + /** The declared peer, which publishes after `consumer` or alongside it in a cycle. */ + readonly peer: string +} + +/** + * A family's publish order together with the ordering it could not honour. + * + * The dropped edges are part of the result rather than a detail of forming it: + * a release drops real ordering constraints, and the operator reading the pack + * log is the only one who can judge whether a newly dropped edge is expected. + */ +export interface PublishPlan { + /** Members in publish order. */ + readonly order: readonly ReleaseMember[] + /** Peer declarations left unordered, in the order the traversal reached them. */ + readonly droppedPeerEdges: readonly DroppedPeerEdge[] +} + /** One publishable package of a release family. */ export interface ReleaseMember { /** Repository-relative package directory, for example `packages/core/session`. */ @@ -107,45 +142,122 @@ export abstract class ReleaseFamily { } /** - * Order members so every package publishes after the family members it depends on. + * Order members so every package publishes after the family members it + * depends on, which is what makes a partial publication self-consistent: an + * interrupted run leaves a prefix whose packages never point at something + * absent from the registry. + * + * Install edges are honoured absolutely — a cycle among them is a defect this + * reports rather than works around. Peer edges order what they can and are + * dropped where honouring one would deadlock: sibling packages declare each + * other as peers, and npm treats an unmet peer as a warning rather than a + * resolution failure ([rationale](../../.agents/notes/implemented/process/2026-08-10-npm-release-sequences.md)). + * Every dropped edge is reported, because dropping one is a decision about a + * real release rather than an implementation detail. * @param members - this family's members. - * @returns The same members in publish order; ties break by name for determinism. + * @returns The order, ties broken by name for determinism, and the peer edges it left unordered. */ - publishOrder(members: readonly ReleaseMember[]): ReleaseMember[] { + publishOrder(members: readonly ReleaseMember[]): PublishPlan { const byName = new Map(members.map(member => [member.name, member])) - const ordered: ReleaseMember[] = [] - const placed = new Set() - const visiting = new Set() + const byNameSorted = [...members].sort((left, right) => left.name.localeCompare(right.name)) + const edges = (member: ReleaseMember, sections: readonly string[]): ReleaseMember[] => + this.orderEdges(member, byName, sections) - const visit = (member: ReleaseMember, path: readonly string[]): void => { - if (placed.has(member.name)) return - if (visiting.has(member.name)) { + // Install edges alone must be acyclic, and that is checked on its own graph: + // a peer edge leading into an install edge would otherwise read as a cycle + // where the install edges are perfectly orderable. + const installVisiting = new Set() + const installDone = new Set() + const checkInstall = (member: ReleaseMember, path: readonly string[]): void => { + if (installDone.has(member.name)) return + if (installVisiting.has(member.name)) { throw new Error(`dependency cycle in release family ${this.id}: ${[...path, member.name].join(' -> ')}`) } - visiting.add(member.name) - for (const dependency of this.orderEdges(member, byName)) { - visit(dependency, [...path, member.name]) + installVisiting.add(member.name) + for (const dependency of edges(member, INSTALL_SECTIONS)) checkInstall(dependency, [...path, member.name]) + installVisiting.delete(member.name) + installDone.add(member.name) + } + for (const member of byNameSorted) checkInstall(member, []) + + // Emit the order over both kinds of edge. A node already on the stack closes + // a cycle, and that cycle carries at least one peer edge because the install + // edges were just proved acyclic — but the back edge that reaches the stacked + // node is not necessarily the peer one, so the post-condition below decides + // whether the emitted order survived. + const ordered: ReleaseMember[] = [] + const droppedPeerEdges: DroppedPeerEdge[] = [] + const placed = new Set() + const onStack = new Set() + // Members reachable from one member through install edges. A peer edge is + // dropped when the peer installs the member declaring it: honouring it would + // emit a package before something it installs, and the install edge wins. + const installClosure = (member: ReleaseMember): Set => { + const reached = new Set() + const walk = (current: ReleaseMember): void => { + for (const dependency of edges(current, INSTALL_SECTIONS)) { + if (reached.has(dependency.name)) continue + reached.add(dependency.name) + walk(dependency) + } } - visiting.delete(member.name) + walk(member) + return reached + } + const visit = (member: ReleaseMember): void => { + if (placed.has(member.name) || onStack.has(member.name)) return + onStack.add(member.name) + for (const dependency of edges(member, INSTALL_SECTIONS)) visit(dependency) + for (const peer of edges(member, PEER_SECTIONS)) { + if (installClosure(peer).has(member.name)) { + droppedPeerEdges.push({ consumer: member.name, peer: peer.name }) + continue + } + // A peer already on the stack is an ancestor, so it publishes after this + // member rather than before it: the edge is dropped, not honoured. + if (onStack.has(peer.name)) droppedPeerEdges.push({ consumer: member.name, peer: peer.name }) + visit(peer) + } + onStack.delete(member.name) placed.add(member.name) ordered.push(member) } + for (const member of byNameSorted) visit(member) - for (const member of [...members].sort((left, right) => left.name.localeCompare(right.name))) { - visit(member, []) + // A cycle mixing both kinds of edge can put an install edge's target on the + // stack, where the traversal skips it like a peer edge and emits a consumer + // before something it installs. Nothing downstream can detect that, and it + // would only surface as an unresolvable install for whoever consumes the + // published packages, so the emitted order is checked against the edges it + // exists to honour. + const position = new Map(ordered.map((entry, index) => [entry.name, index])) + for (const [index, member] of ordered.entries()) { + for (const dependency of edges(member, INSTALL_SECTIONS)) { + const dependencyIndex = position.get(dependency.name) + if (dependencyIndex !== undefined && dependencyIndex < index) continue + throw new Error( + `release family ${this.id}: no publish order honours ${member.name} -> ${dependency.name};` + + ' a cycle mixing peer and dependency declarations reaches this dependency through a peer edge', + ) + } } - return ordered + return { order: ordered, droppedPeerEdges } } /** - * The family members one member depends on at runtime. + * The family members one member declares in the given sections. * @param member - the dependent member. * @param byName - every family member by package name. - * @returns Dependencies inside this family, sorted by name. + * @param sections - manifest sections to read. + * @returns Members of this family named there, sorted by name. */ - private orderEdges(member: ReleaseMember, byName: ReadonlyMap): ReleaseMember[] { + private orderEdges( + member: ReleaseMember, + byName: ReadonlyMap, + sections: readonly string[], + ): ReleaseMember[] { const edges: ReleaseMember[] = [] - for (const section of ORDER_SECTIONS) { + for (const section of sections) { const dependencies = member.manifest[section] if (dependencies === null || typeof dependencies !== 'object' || Array.isArray(dependencies)) continue for (const name of Object.keys(dependencies)) { diff --git a/scripts/release/pack.ts b/scripts/release/pack.ts index 47a33a26ac..5d2b9b4e64 100644 --- a/scripts/release/pack.ts +++ b/scripts/release/pack.ts @@ -45,7 +45,7 @@ function main(): void { const family = releaseFamily(values.family) const root = process.cwd() const destination = resolve(root, values.out ?? DEFAULT_OUTPUT) - const members = family.publishOrder(family.members(root)) + const members = family.publishOrder(family.members(root)).order family.verifyVersions(members) rmSync(destination, { recursive: true, force: true }) diff --git a/scripts/release/process.ts b/scripts/release/process.ts index 746f24ac36..392a6cab66 100644 --- a/scripts/release/process.ts +++ b/scripts/release/process.ts @@ -38,6 +38,40 @@ export function attempt(command: string, args: readonly string[], options: RunOp return { status: result.status, stdout: result.stdout, stderr: result.stderr } } +/** + * Run a command, capture its output, and echo it once the command exits. + * + * A step that both shows what a command said and classifies its own failure + * needs both halves: the output has to reach the workflow log, and the caller has + * to read it to decide whether a failure is worth retrying. + * + * This is not live progress. `spawnSync` returns only after the child exits, so + * nothing appears while the command runs, and the two streams are echoed one + * after the other — all of stdout, then all of stderr — which loses their + * interleaving. For an npm publish that matters in one visible way: `npm notice` + * lines go to stderr while the `+ name@version` confirmation goes to stdout, so + * the log shows the confirmation first. Live progress would need an + * asynchronous spawn with data listeners. + * @param command - executable name. + * @param args - command arguments. + * @param options - working directory and environment. + * @returns The exit status and captured streams. + */ +export function attemptEchoed(command: string, args: readonly string[], options: RunOptions = {}): CommandResult { + const result = spawnSync(command, [...args], { + cwd: options.cwd, + env: options.env, + encoding: 'utf8', + // 'inherit' would leave nothing to capture, so the streams are piped and + // echoed instead. + stdio: ['inherit', 'pipe', 'pipe'], + }) + if (result.error !== undefined) throw result.error + if (result.stdout !== '') process.stdout.write(result.stdout) + if (result.stderr !== '') process.stderr.write(result.stderr) + return { status: result.status, stdout: result.stdout, stderr: result.stderr } +} + /** * Run a command, capture its standard output, and fail on a non-zero exit. * @param command - executable name. diff --git a/scripts/release/publish.ts b/scripts/release/publish.ts index 11ad01173c..2590e96e52 100644 --- a/scripts/release/publish.ts +++ b/scripts/release/publish.ts @@ -18,7 +18,7 @@ import { join, resolve } from 'node:path' import { setTimeout as sleep } from 'node:timers/promises' import { parseArgs } from 'node:util' import { releaseFamily } from './families.ts' -import { attempt, isEntry } from './process.ts' +import { attempt, attemptEchoed, isEntry } from './process.ts' import { packedIdentity, readPublishOrder } from './tarball.ts' /** @@ -102,7 +102,7 @@ async function publishTarball(tarball: string, name: string, version: string): P // command-line flag could not serve both and would override the manifest // that does. Each packed manifest decides, and // check-workspace-constraints holds every manifest to its sequence's level. - const result = attempt('npm', ['publish', tarball, ...tagArgs]) + const result = attemptEchoed('npm', ['publish', tarball, ...tagArgs]) const output = `${result.stdout}${result.stderr}` if (result.status === 0) return @@ -136,9 +136,15 @@ async function main(): Promise { const family = releaseFamily(values.family) const directory = resolve(process.cwd(), values.from) + // Every entry in the order settles as either published or already present, so + // one counter answers "how far along is this run" for whoever is watching a + // release that takes minutes per family. + const order = readPublishOrder(directory) + const total = String(order.length) let published = 0 let skipped = 0 - for (const filename of readPublishOrder(directory)) { + for (const [index, filename] of order.entries()) { + const progress = `[${String(index + 1)}/${total}]` const tarball = join(directory, filename) const { name, version } = packedIdentity(tarball) const state = registryState(name, version) @@ -151,7 +157,7 @@ async function main(): Promise { + '\nBump the version, or investigate why the build is not reproducible.', ) } - console.log(`release publish: ${name}@${version} already published, skipping`) + console.log(`release publish: ${progress} ${name}@${version} already published, skipping`) skipped += 1 continue } @@ -159,11 +165,14 @@ async function main(): Promise { // only skips does not wait at all. if (published > 0) await sleep(PUBLISH_SPACING_MS) await publishTarball(tarball, name, version) - console.log(`release publish: ${name}@${version} published`) + console.log(`release publish: ${progress} ${name}@${version} published`) published += 1 } - console.log(`release publish: family ${family.id}, ${String(published)} published, ${String(skipped)} already present`) + console.log( + `release publish: family ${family.id}, ${total} member(s),` + + ` ${String(published)} published, ${String(skipped)} already present`, + ) } if (isEntry(import.meta.url)) await main() diff --git a/scripts/release/verify.ts b/scripts/release/verify.ts index 1bd74c84d6..5829087f97 100644 --- a/scripts/release/verify.ts +++ b/scripts/release/verify.ts @@ -9,7 +9,33 @@ import { parseArgs } from 'node:util' import { isEntry } from './process.ts' -import { releaseFamily, type ReleaseFamily, type ReleaseMember } from './families.ts' +import { releaseFamily, type PublishPlan, type ReleaseFamily, type ReleaseMember } from './families.ts' + +/** + * Print the publish order the release will follow, and the peer declarations it + * leaves unordered. + * + * The order is the release's own plan: an interrupted publication leaves exactly + * a prefix of it, so reading it is how anyone judges what a partial run left on + * the registry, and printing it on every pull request is what makes a change to + * the order reviewable rather than only observable during a publication. + * @param family - the release family. + * @param plan - the resolved order and its dropped edges. + */ +function reportPublishOrder(family: ReleaseFamily, plan: PublishPlan): void { + console.log(`release verify: publish order for family ${family.id}, ${String(plan.order.length)} member(s):`) + const width = String(plan.order.length).length + for (const [index, member] of plan.order.entries()) { + console.log(` ${String(index + 1).padStart(width, ' ')} ${member.name}@${member.version}`) + } + if (plan.droppedPeerEdges.length === 0) return + console.log( + `release verify: ${String(plan.droppedPeerEdges.length)} peer declaration(s) publish unordered,` + + ' because the peer cannot precede the package declaring it without contradicting a dependency edge' + + ' or its own cycle. npm treats an unmet peer as a warning, so this orders nothing and blocks nothing:', + ) + for (const edge of plan.droppedPeerEdges) console.log(` ${edge.consumer} -> ${edge.peer}`) +} /** * Assert every member may be published: npm refuses a `private` package. @@ -55,6 +81,16 @@ function main(): void { const family = releaseFamily(values.family) const members = family.members(process.cwd()) family.verifyVersions(members) + // Resolve the publish order here, before the build: an install-edge cycle + // makes the order unrepresentable, and that has to surface at the first gate + // rather than when pack is already writing tarballs. + const plan = family.publishOrder(members) + if (plan.order.length !== members.length) { + throw new Error( + `release family ${family.id}: publish order covers ${String(plan.order.length)} of ${String(members.length)} members`, + ) + } + reportPublishOrder(family, plan) const publishing = process.env.RELEASE_PUBLISH === 'true' if (publishing) { @@ -64,7 +100,11 @@ function main(): void { const versions = [...new Set(members.map(member => member.version))] const summary = versions.length === 1 ? versions[0] : `${String(versions.length)} versions` - console.log(`release verify: family ${family.id}, ${String(members.length)} member(s), ${summary}${publishing ? ', publish gates passed' : ''}`) + console.log( + `release verify: family ${family.id}, ${String(members.length)} member(s), ${summary},` + + ` publish order resolved, ${String(plan.droppedPeerEdges.length)} peer declaration(s) unordered` + + (publishing ? ', publish gates passed' : ''), + ) } if (isEntry(import.meta.url)) main() diff --git a/scripts/run-gates.ts b/scripts/run-gates.ts index 27664fab5e..6b775c9bf1 100644 --- a/scripts/run-gates.ts +++ b/scripts/run-gates.ts @@ -249,6 +249,9 @@ function ciSharedStaticGates(): Gate[] { pnpmScript('dsh-package-licenses', 'verify-dsh-package-licenses', { label: 'DSH package licenses' }), pnpmScript('package-invariants', 'verify-package-invariants', { label: 'package invariants' }), pnpmScript('cordis-config', 'verify-cordis-config', { label: 'Cordis config' }), + pnpmScript('optional-dependency-imports', 'verify-optional-dependency-imports', { + label: 'optional dependency imports', + }), pnpmScript('issue-management', 'test:issue-management', { label: 'Issue management policy' }), ] } @@ -565,6 +568,9 @@ function hygieneLeafGates(options: { artifactNeeds?: string[] } = {}): Gate[] { label: 'node-next types', ...artifactOptions, }), + pnpmScript('optional-dependency-imports', 'verify-optional-dependency-imports', { + label: 'optional dependency imports', + }), ] } diff --git a/scripts/ts-project.ts b/scripts/ts-project.ts index 9a0400a39d..53b100ceb0 100644 --- a/scripts/ts-project.ts +++ b/scripts/ts-project.ts @@ -11,6 +11,12 @@ interface ProjectGraph { options: ts.CompilerOptions } +/** + * A compiler face: the two aggregates a repository-wide program may seed from. + * The root solution is never one of them. + */ +export type CompilerFace = 'host' | 'client' + /** TypeScript config host shared by repository scripts. */ export const repositoryConfigHost: ts.ParseConfigFileHost = { useCaseSensitiveFileNames: ts.sys.useCaseSensitiveFileNames, @@ -24,12 +30,12 @@ export const repositoryConfigHost: ts.ParseConfigFileHost = { } /** - * Parse the host aggregate tsconfig and flatten all referenced projects into one + * Parse one face aggregate tsconfig and flatten all referenced projects into one * semantic graph. Never seed the root solution: flattening host+client into one * program collides the cordis Context merges. */ -function loadProjectGraph(projectRoot: string): ProjectGraph { - const rootConfigPath = resolve(projectRoot, 'tsconfig.host.json') +function loadProjectGraph(projectRoot: string, face: CompilerFace): ProjectGraph { + const rootConfigPath = resolve(projectRoot, `tsconfig.${face}.json`) const rootConfig = parseConfig(rootConfigPath) const rootNames = new Set() const visited = new Set() @@ -81,8 +87,12 @@ export class TypeScriptProject { /** The checker shared by every semantic query in this project. */ readonly checker: ts.TypeChecker - constructor(private readonly projectRoot: string) { - const graph = loadProjectGraph(projectRoot) + /** + * @param projectRoot - repository root the program is seeded and reported from. + * @param face - which compiler face aggregate to flatten. + */ + constructor(readonly projectRoot: string, face: CompilerFace = 'host') { + const graph = loadProjectGraph(projectRoot, face) this.program = ts.createProgram(graph.rootNames, semanticCompilerOptions(graph.options)) this.checker = this.program.getTypeChecker() } diff --git a/scripts/verify-optional-dependency-imports.spec.ts b/scripts/verify-optional-dependency-imports.spec.ts new file mode 100644 index 0000000000..3bb857050f --- /dev/null +++ b/scripts/verify-optional-dependency-imports.spec.ts @@ -0,0 +1,130 @@ +/** + * Tests for the optional-dependency load gate: which import and re-export forms + * survive emit, and therefore load a package the installed tree may not carry. + * + * The expectations here match what `tsc` emits with `verbatimModuleSyntax` off: + * `import type`, `import {}`, an inline `type` specifier, and a named binding + * that resolves to a type all disappear; a bare import, a value binding, and a + * star re-export remain. + */ + +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { dirname, join } from 'node:path' +import { afterAll, describe, expect, it } from 'vitest' +import { TypeScriptProject } from './ts-project.ts' +import { collectOptionalImportViolations } from './verify-optional-dependency-imports.ts' + +const FIXTURE: Record = { + 'tsconfig.host.json': JSON.stringify({ + compilerOptions: { + target: 'es2022', + module: 'esnext', + moduleResolution: 'bundler', + noEmit: true, + skipLibCheck: true, + types: [], + paths: { + '@f/opt': ['./packages/f/opt/src/index.ts'], + '@f/hard': ['./packages/f/hard/src/index.ts'], + }, + }, + include: ['packages/**/*.ts'], + }), + + 'packages/f/opt/package.json': JSON.stringify({ name: '@f/opt', version: '0.0.1' }), + 'packages/f/opt/src/index.ts': [ + 'export interface Shape { a: number }', + 'export const runtimeValue = 1', + '', + ].join('\n'), + + 'packages/f/hard/package.json': JSON.stringify({ name: '@f/hard', version: '0.0.1' }), + 'packages/f/hard/src/index.ts': 'export const hardValue = 2\n', + + // The consumer allows @f/opt to be absent and requires @f/hard. + 'packages/f/consumer/package.json': JSON.stringify({ + name: '@f/consumer', + version: '0.0.1', + dependencies: { '@f/hard': '*' }, + peerDependencies: { '@f/opt': '*' }, + peerDependenciesMeta: { '@f/opt': { optional: true } }, + }), + + // Elided by the compiler, so each of these is allowed. + 'packages/f/consumer/src/allowed-type-only.ts': [ + "import type {} from '@f/opt'", + 'export const a = 1', + '', + ].join('\n'), + 'packages/f/consumer/src/allowed-empty.ts': [ + "import {} from '@f/opt'", + 'export const b = 1', + '', + ].join('\n'), + 'packages/f/consumer/src/allowed-inline-type.ts': [ + "import { type Shape } from '@f/opt'", + 'export const c: Shape = { a: 1 }', + '', + ].join('\n'), + 'packages/f/consumer/src/allowed-type-binding.ts': [ + "import { Shape } from '@f/opt'", + 'export const d: Shape = { a: 1 }', + '', + ].join('\n'), + 'packages/f/consumer/src/allowed-type-reexport.ts': [ + "export type { Shape } from '@f/opt'", + '', + ].join('\n'), + // A hard dependency may be loaded at module scope; only optional ones may not. + 'packages/f/consumer/src/allowed-hard-dependency.ts': [ + "import { hardValue } from '@f/hard'", + 'export const e = hardValue', + '', + ].join('\n'), + + // Kept by the compiler, so each of these loads a package that may be absent. + 'packages/f/consumer/src/rejected-bare.ts': [ + "import '@f/opt'", + 'export const f = 1', + '', + ].join('\n'), + 'packages/f/consumer/src/rejected-value.ts': [ + "import { runtimeValue } from '@f/opt'", + 'export const g = runtimeValue', + '', + ].join('\n'), + 'packages/f/consumer/src/rejected-star-reexport.ts': [ + "export * from '@f/opt'", + '', + ].join('\n'), +} + +const root = mkdtempSync(join(tmpdir(), 'optional-imports-')) +for (const [rel, content] of Object.entries(FIXTURE)) { + mkdirSync(dirname(join(root, rel)), { recursive: true }) + writeFileSync(join(root, rel), content) +} +const violations = collectOptionalImportViolations(new TypeScriptProject(root)) + +afterAll(() => { + rmSync(root, { recursive: true, force: true }) +}) + +describe('optional dependency loads', () => { + it('reports every form the compiler keeps, and nothing else', () => { + expect(violations.map(violation => violation.split(' loads ')[0])).toEqual([ + 'packages/f/consumer/src/rejected-bare.ts:1', + 'packages/f/consumer/src/rejected-star-reexport.ts:1', + 'packages/f/consumer/src/rejected-value.ts:1', + ]) + }) + + it('names the package, the declaration that made it optional, and the way out', () => { + expect(violations[0]).toBe( + 'packages/f/consumer/src/rejected-bare.ts:1 loads @f/opt at module scope,' + + ' declared optional in peerDependenciesMeta; import it as a type,' + + ' or restructure so module scope does not need it', + ) + }) +}) diff --git a/scripts/verify-optional-dependency-imports.ts b/scripts/verify-optional-dependency-imports.ts new file mode 100644 index 0000000000..e7e16d7aab --- /dev/null +++ b/scripts/verify-optional-dependency-imports.ts @@ -0,0 +1,214 @@ +/** + * Reject a static value import of an optional dependency. + * + * A dependency declared in `optionalDependencies`, or as a peer carrying + * `peerDependenciesMeta..optional`, may be absent from an installed tree — + * that absence is what "optional" promises a consumer. A static import is + * evaluated when the importing module loads, so one absent package turns + * "this capability is unavailable" into a load failure for everything that + * reaches the importing module. + * + * The way out, in order: import it as a type, which emits nothing and is all + * that declaration merging needs; or restructure so nothing at module scope + * needs the package. A dynamic `import()` only moves the failure to first use, + * so it belongs to a caller that genuinely requires the package and handles its + * absence — it is a last resort, not the default answer, and reaching for it is + * a sign the dependency is not optional. + * + * Value-vs-type is decided against a bound Program rather than the import + * syntax, because `verbatimModuleSyntax` is off: a named import used only in + * type positions is elided and does not load anything. The decision is + * deliberately conservative in one direction — a value binding the compiler + * would elide because nothing references it in a value position is still + * reported, and the fix it asks for (`import type`, or dropping the binding) is + * what the published package wants regardless. Both compiler faces are scanned, + * and only files that ship — a published package's `src` — are subject. + */ + +import { existsSync, readFileSync } from 'node:fs' +import { resolve } from 'node:path' +import ts from 'typescript' +import { TypeScriptProject, type CompilerFace } from './ts-project.ts' + +const root = resolve(import.meta.dirname, '..') + +/** Directories whose `src` ships as a published package. */ +const PUBLISHED_SOURCE = /^(?:packages\/[^/]+\/[^/]+|apps\/[^/]+)\/src\// + +/** How a manifest marked a dependency optional, for the violation message. */ +type OptionalKind = 'optionalDependencies' | 'peerDependenciesMeta' + +/** + * The package name a module specifier resolves to. + * @param specifier - an import specifier, possibly a subpath. + * @returns The bare package name, keeping a leading scope. + */ +function packageOf(specifier: string): string { + const parts = specifier.split('/') + return specifier.startsWith('@') ? parts.slice(0, 2).join('/') : parts[0] ?? specifier +} + +/** + * Read a manifest field as a record. + * @param manifest - parsed manifest. + * @param field - field name. + * @returns The field value, or an empty record. + */ +function record(manifest: Record, field: string): Record { + const value = manifest[field] + if (value === null || typeof value !== 'object' || Array.isArray(value)) return {} + return value as Record +} + +/** + * The dependencies one manifest allows to be absent. + * @param manifest - parsed manifest. + * @returns Each optional package name and how it was marked. + */ +function optionalDependencies(manifest: Record): Map { + const optional = new Map() + for (const name of Object.keys(record(manifest, 'optionalDependencies'))) { + optional.set(name, 'optionalDependencies') + } + const peers = record(manifest, 'peerDependencies') + for (const [name, meta] of Object.entries(record(manifest, 'peerDependenciesMeta'))) { + if (meta === null || typeof meta !== 'object') continue + if ((meta as Record).optional !== true) continue + // A meta entry for an undeclared peer is check-workspace-constraints' business. + if (!(name in peers)) continue + optional.set(name, 'peerDependenciesMeta') + } + return optional +} + +/** One package directory's optional dependencies, resolved once per directory. */ +const optionalByDirectory = new Map>() + +/** + * The optional dependencies of the package owning a source file. + * @param projectRoot - root the relative path is resolved against. + * @param relativePath - repository-relative path of a source file. + * @returns That package's optional dependencies, empty when it declares none. + */ +function optionalFor(projectRoot: string, relativePath: string): Map { + const directory = resolve(projectRoot, relativePath.slice(0, relativePath.indexOf('/src/'))) + const cached = optionalByDirectory.get(directory) + if (cached !== undefined) return cached + const manifestPath = resolve(directory, 'package.json') + const parsed: unknown = existsSync(manifestPath) ? JSON.parse(readFileSync(manifestPath, 'utf8')) : {} + const manifest = parsed !== null && typeof parsed === 'object' && !Array.isArray(parsed) + ? parsed as Record + : {} + const optional = optionalDependencies(manifest) + optionalByDirectory.set(directory, optional) + return optional +} + +/** + * Whether one binding of an import or re-export names a value. + * @param name - the local binding name node. + * @param checker - the program's checker. + * @returns True when the binding carries value meaning, and on an unresolved + * symbol, so an unresolvable binding fails closed. + */ +function bindsValue(name: ts.Identifier | ts.StringLiteral, checker: ts.TypeChecker): boolean { + const symbol = checker.getSymbolAtLocation(name) + if (symbol === undefined) return true + const target = (symbol.flags & ts.SymbolFlags.Alias) === 0 ? symbol : checker.getAliasedSymbol(symbol) + return (target.flags & ts.SymbolFlags.Value) !== 0 +} + +/** + * Whether an import declaration loads its module at run time. + * @param declaration - the import declaration. + * @param checker - the program's checker. + * @returns True when the emitted module keeps the import. + */ +function importLoadsModule(declaration: ts.ImportDeclaration, checker: ts.TypeChecker): boolean { + const clause = declaration.importClause + // A bare `import 'x'` is kept for its side effects. + if (clause === undefined) return true + // Only the type phase erases the import. `import defer` still resolves and + // links the module, deferring evaluation alone, so an absent package fails + // exactly as it would without the modifier. + if (clause.phaseModifier === ts.SyntaxKind.TypeKeyword) return false + if (clause.name !== undefined) return true + const bindings = clause.namedBindings + if (bindings === undefined || ts.isNamespaceImport(bindings)) return true + return bindings.elements.some(element => !element.isTypeOnly && bindsValue(element.name, checker)) +} + +/** + * Whether a re-export loads its module at run time. + * @param declaration - the export declaration, which carries a module specifier. + * @param checker - the program's checker. + * @returns True when the emitted module keeps the re-export. + */ +function exportLoadsModule(declaration: ts.ExportDeclaration, checker: ts.TypeChecker): boolean { + if (declaration.isTypeOnly) return false + const clause = declaration.exportClause + // `export * from 'x'` re-exports whatever values the module has. + if (clause === undefined || ts.isNamespaceExport(clause)) return true + return clause.elements.some(element => !element.isTypeOnly && bindsValue(element.name, checker)) +} + +/** + * Collect every static value import of an optional dependency in one face. + * @param project - a bound repository project. + * @returns One message per violation, sorted by location. + */ +export function collectOptionalImportViolations(project: TypeScriptProject): string[] { + const checker = project.checker + const violations: string[] = [] + for (const sourceFile of project.sourceFiles()) { + if (sourceFile.isDeclarationFile) continue + const relativePath = project.relativePath(sourceFile) + if (!PUBLISHED_SOURCE.test(relativePath)) continue + const optional = optionalFor(project.projectRoot, relativePath) + if (optional.size === 0) continue + + for (const statement of sourceFile.statements) { + const isImport = ts.isImportDeclaration(statement) + if (!isImport && !ts.isExportDeclaration(statement)) continue + const specifierNode = statement.moduleSpecifier + if (specifierNode === undefined || !ts.isStringLiteral(specifierNode)) continue + const kind = optional.get(packageOf(specifierNode.text)) + if (kind === undefined) continue + const loads = isImport + ? importLoadsModule(statement, checker) + : exportLoadsModule(statement, checker) + if (!loads) continue + const { line } = sourceFile.getLineAndCharacterOfPosition(statement.getStart(sourceFile)) + violations.push( + `${relativePath}:${String(line + 1)} loads ${specifierNode.text} at module scope,` + + ` declared optional in ${kind}; import it as a type, or restructure so module scope does not need it`, + ) + } + } + return violations.sort((left, right) => left.localeCompare(right)) +} + +/** CLI entry: list every violation and exit 1, or confirm the invariant holds. */ +function main(): void { + const faces: readonly CompilerFace[] = ['host', 'client'] + const violations = new Set() + for (const face of faces) { + for (const violation of collectOptionalImportViolations(new TypeScriptProject(root, face))) { + violations.add(violation) + } + } + if (violations.size === 0) { + console.log('verify-optional-dependency-imports: no optional dependency is loaded at module scope.') + return + } + console.error(`verify-optional-dependency-imports: ${String(violations.size)} optional dependency load(s) at module scope:`) + for (const violation of [...violations].sort((left, right) => left.localeCompare(right))) { + console.error(` ${violation}`) + } + process.exit(1) +} + +// Run only when invoked as a script, not when imported by a test. +if (process.argv[1] && import.meta.filename === resolve(process.argv[1])) { + main() +}