Skip to content

Commit e9855a0

Browse files
authored
🐛 Build npm packages from their local release closure (#847)
* 🐛 Build npm packages from their local release closure A tagged publish waited on npm indexing a sibling the previous job had just published, because the builder rewrote every `workspace:*` dependency to `^<version>` before calling dnt. `publish-one.yml` bounded that wait with four attempts and 15-second sleeps; the observed lag exceeded it on 0.12.0, 0.12.1 and 0.13.1, and 0.13.1 reached npm only after four rounds of rerunning failed jobs. `scripts/build-npm.ts` now builds in two phases. Phase 1 builds the requested package's internal dependencies depth-first from the same checkout, each at most once, and hands dnt absolute `file:` ranges naming those artifacts, so the install and the type check never reach the registry for a package from this release. Phase 2 finalizes every manifest in the closure together, once the last dnt call has returned, replacing each internal `file:` range with the sibling's `^<version>`. Together, not per package: in a chain A → B → C, rewriting B when its own build finishes puts C's registry version back in front of A's install. Before reporting success the builder refuses any dependency range still starting with `workspace:` or `file:`, and any string naming the checkout's path — a release gate, so an unexpected local dependency fails the build instead of being normalized into something npm would accept. An internal dependency no workspace member declares is refused by name, with no registry fallback. `DNT_LOCAL_SIBLINGS` is gone: the default path is what it used to provide, and the artifact a developer builds is now the artifact a release publishes. `DNT_SKIP_INSTALL` keeps its leaf-only contract. `publish-one.yml` builds once and keeps its already-published guard; `publish-packages.yml` keeps its dependency-ordered `needs:`, which is now a publication guarantee rather than a build one — a failed upstream still withholds its dependents, so whatever npm holds is dependency-closed. The builder is importable, so the regression drives the real path over a three-member closure whose versions npm has never seen. * 🧪 Prove closure finalization with packed local dependencies N4 was returned as unprovable, and that was wrong about the mechanism rather than about the contract. npm symlinks a directory `file:` dependency by default, so a dependent reaches whatever the sibling's own build left in its `node_modules` and an early-finalized sibling costs nothing. npm's supported `install-links=true` packs and installs it as an ordinary dependency instead, which leaves the sibling's own manifest as the only statement of where its dependencies come from — and that is the discriminator. N4 now runs the real builder twice under an invocation-private npm configuration: a positive control, so packed local artifacts are known to work before a failure means anything, then the same closure with `b` rewritten to the registry range the moment its build completes. `a`'s install fails, no manifest is finalized, and `a` never completes. The registry it would have to reach is unreachable and retries are off, so the failure is hermetic and immediate rather than whatever npmjs.org happens to answer. The environment is restored however the case ends; every other build in the file is the ordinary one. Setting `install-links=false` makes the negative control pass, which is what makes the scoped configuration load-bearing rather than decoration. The three places that explained phase 2 by claiming an early rewrite puts a registry version in front of the dependent's install now say what is actually true: the cost of finalizing early depends on how npm installs a local directory and on the sibling's own build residue, and finalizing the closure at the end is what makes the build independent of both. * 🧪 Hold N4 to the failure it is about `expect(caught).toBeInstanceOf(Error)` accepted any throw, including one from the observer that applies the mutation — so a negative control that never reached `a` would have passed while proving nothing. The build-start events must now be exactly `c`, `b`, `a`, which is what says the mutation was applied and `a` entered dnt; an observer that threw leaves that list one short. The caught error must be dnt's `npm install failed with exit code 1`, which is what says it failed where a registry range has to be resolved rather than anywhere else in the run. The proof that only `c` and `b` complete and that finalization never starts is unchanged. Making the observer throw before its rewrite now fails the case.
1 parent 3951a69 commit e9855a0

7 files changed

Lines changed: 716 additions & 106 deletions

File tree

‎.github/workflows/publish-one.yml‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,18 +56,12 @@ jobs:
5656
- name: Build the browser bundle
5757
run: deno task build:web
5858

59+
# One attempt. The builder constructs this package's internal closure from
60+
# this checkout, so nothing here waits on npm indexing a sibling the
61+
# previous job published — the wait the retry loop used to bound, and
62+
# never reliably did (#843).
5963
- name: Build with dnt
60-
# Retry to absorb npm propagation of a just-published sibling dependency.
61-
run: |
62-
set -eu
63-
for attempt in 1 2 3 4; do
64-
if deno run -A scripts/build-npm.ts "${{ inputs.package }}" "${{ inputs.version }}"; then
65-
exit 0
66-
fi
67-
[ "$attempt" -lt 4 ] && sleep 15 || true
68-
done
69-
echo "build failed after 4 attempts" >&2
70-
exit 1
64+
run: deno run -A scripts/build-npm.ts "${{ inputs.package }}" "${{ inputs.version }}"
7165

7266
# Idempotent: skip a version npm already has, so tag re-runs succeed.
7367
- name: Publish to npm

‎scripts/build-npm.ts‎

Lines changed: 250 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,30 @@
1111
* Everything published is derived from the member's own deno.json (name,
1212
* exports) and package.json (dependencies, description, bin) — those manifests
1313
* are the single source of truth. Internal @executablemd siblings are declared
14-
* as external npm dependencies (resolved to the sibling's own version), never
15-
* inlined, so each published package resolves them from npm.
14+
* as external npm dependencies, never inlined.
1615
*
17-
* `DNT_LOCAL_SIBLINGS=1` builds each internal sibling first and depends on those
18-
* artifacts by path instead of by published version, so a branch can build and
19-
* type-check against its own workspace sources. The resulting package.json names
20-
* local directories and is therefore unpublishable; release workflows never set
21-
* the variable.
16+
* A build happens in two phases, and the split is what keeps a release off the
17+
* registry's clock.
18+
*
19+
* **Phase 1 builds the local closure.** The requested package's internal
20+
* dependencies are built first, depth-first, each at most once, and handed to
21+
* dnt as absolute `file:` ranges pointing at the artifacts this same invocation
22+
* produced. Nothing asks npm for a package from the same release.
23+
*
24+
* **Phase 2 finalizes every manifest together**, once the last dnt call has
25+
* returned, replacing each internal `file:` range with the sibling's
26+
* `^<version>`. It has to be every manifest at once: rewriting B when its own
27+
* dnt call returns leaves it describing a dependency only the registry could
28+
* supply, and whether A survives that then rests on how npm installs a local
29+
* directory and on what B's own build left in its `node_modules` — no cost
30+
* under the default symlink layout, a failed install under
31+
* `install-links=true`. Finalizing the closure at the end depends on neither.
32+
* No install or typecheck runs after finalization begins, and a surviving local
33+
* reference fails the build rather than reaching `npm publish`.
34+
*
35+
* `DNT_SKIP_INSTALL=1` still skips the install and typecheck for a leaf package
36+
* that declares no `workspace:*` dependency, for exercising the tooling. It
37+
* refuses anything else, and release workflows never set it.
2238
*/
2339

2440
import { ensure, exit, main, scoped, until } from "effection";
@@ -126,36 +142,69 @@ interface WorkspaceMember {
126142
version: string;
127143
}
128144

145+
/**
146+
* What one invocation reports as it runs. Diagnostic observation only: an
147+
* observer chooses no build, no dependency resolution and no finalization
148+
* policy, which is what keeps the regression watching the real path instead of
149+
* a second one.
150+
*/
151+
export type BuildEvent =
152+
| {
153+
readonly type: "package-build-started";
154+
readonly package: string;
155+
/** Exactly what dnt was handed, so a test can see the local ranges. */
156+
readonly dependencies: Readonly<Record<string, string>>;
157+
}
158+
| { readonly type: "package-build-completed"; readonly package: string }
159+
| { readonly type: "closure-finalization-started" }
160+
| { readonly type: "package-manifest-finalized"; readonly package: string };
161+
162+
export interface BuildNpmOptions {
163+
/** The workspace root the closure is built from. */
164+
repoRoot: URL;
165+
/** The requested member's directory, e.g. `packages/cli`. */
166+
package: string;
167+
/** The npm version for the requested artifact; siblings use their own. */
168+
version: string;
169+
observe?: (event: BuildEvent) => Operation<void>;
170+
}
171+
172+
/** One package this invocation built, awaiting closure-wide finalization. */
173+
interface BuiltArtifact {
174+
name: string;
175+
dir: string;
176+
/** Internal dependency name -> the workspace version to finalize it to. */
177+
internal: Record<string, string>;
178+
}
179+
129180
interface BuildContext {
130181
repoRoot: URL;
131182
rootDeno: z.infer<typeof RootDenoSchema>;
132183
members: Record<string, WorkspaceMember>;
133-
/** Depend on locally built sibling artifacts instead of published versions. */
134-
localSiblings: boolean;
135184
skipInstall: boolean;
136-
/** Package names already built in this process, so a diamond builds once. */
137-
built: Set<string>;
185+
/** Artifacts already built in this process, so a diamond builds once. */
186+
built: Map<string, BuiltArtifact>;
187+
observe: (event: BuildEvent) => Operation<void>;
138188
}
139189

140-
await main(function* (args) {
141-
const pkgArg = args[0];
142-
const version = args[1] ?? "0.0.0-dev";
143-
144-
if (!pkgArg) {
145-
console.error("usage: build-npm.ts <package-dir> [version]");
146-
yield* exit(1);
147-
return;
148-
}
190+
/**
191+
* npm resolves a relative `file:` against the dependent's own location, which
192+
* differs for a sibling installed under another package's node_modules, so the
193+
* range this invocation hands dnt is absolute.
194+
*/
195+
function localRange(repoRoot: URL, dir: string): string {
196+
return `file:${fromFileUrl(new URL(`${dir}/npm`, repoRoot))}`;
197+
}
149198

150-
// Before anything is emitted. Copying the documentation assets is not the
151-
// same as validating them: a package built from a set that has drifted from
152-
// the components it documents would install cleanly and refuse the first time
153-
// somebody asked it for documentation. The same assembly the run profile uses
154-
// runs here, so a missing, unknown or duplicated section fails the build for
155-
// exactly the reason it would fail a run.
156-
yield* validateDocumentation();
199+
function* observeNothing(): Operation<void> {}
157200

158-
const repoRoot = new URL("../", import.meta.url);
201+
/**
202+
* Build `options.package` and its internal closure, and leave every generated
203+
* manifest publishable. This is the whole builder; the command below is an
204+
* adapter over it, so the regression exercises the release path itself.
205+
*/
206+
export function* buildNpmPackage(options: BuildNpmOptions): Operation<void> {
207+
const { repoRoot } = options;
159208

160209
const rootDeno = RootDenoSchema.parse(
161210
JSON.parse(yield* readTextFile(new URL("deno.json", repoRoot))),
@@ -175,15 +224,18 @@ await main(function* (args) {
175224
}
176225
}
177226

178-
yield* buildPackage(pkgArg, version, {
227+
const ctx: BuildContext = {
179228
repoRoot,
180229
rootDeno,
181230
members,
182-
localSiblings: Deno.env.get("DNT_LOCAL_SIBLINGS") === "1",
183231
skipInstall: Deno.env.get("DNT_SKIP_INSTALL") === "1",
184-
built: new Set(),
185-
});
186-
});
232+
built: new Map(),
233+
observe: options.observe ?? observeNothing,
234+
};
235+
236+
yield* buildPackage(options.package, options.version, ctx);
237+
yield* finalizeClosure(ctx);
238+
}
187239

188240
function* buildPackage(pkgArg: string, version: string, ctx: BuildContext): Operation<void> {
189241
const { repoRoot, rootDeno, skipInstall } = ctx;
@@ -200,29 +252,28 @@ function* buildPackage(pkgArg: string, version: string, ctx: BuildContext): Oper
200252
);
201253

202254
// Dependencies come from package.json verbatim, except internal siblings
203-
// (workspace:* protocol) which resolve to the sibling's own version range —
204-
// or, with local siblings, to the artifact this process just built for it.
255+
// (workspace:* protocol), which are built first and named by the artifact
256+
// this invocation just produced. Phase 2 turns those into version ranges.
205257
const dependencies: Record<string, string> = {};
258+
const internal: Record<string, string> = {};
206259
for (const [name, range] of Object.entries(packageJson.dependencies ?? {})) {
207260
if (!name.startsWith(INTERNAL_SCOPE)) {
208261
dependencies[name] = range;
209262
continue;
210263
}
211264
const member = ctx.members[name];
212265
if (!member) {
213-
throw new Error(`no workspace version found for internal dependency "${name}"`);
214-
}
215-
if (!ctx.localSiblings) {
216-
dependencies[name] = `^${member.version}`;
217-
continue;
266+
// No registry fallback: reaching npm for an unmapped internal name is how
267+
// a misspelled or removed member would silently restore the release race.
268+
throw new Error(
269+
`"${denoJson.name}" depends on internal package "${name}", which is not a workspace member`,
270+
);
218271
}
219272
if (!ctx.built.has(name)) {
220273
yield* buildPackage(member.dir, member.version, ctx);
221274
}
222-
// An absolute path: npm resolves a relative `file:` against the dependent's
223-
// own location, which differs for a sibling installed under another
224-
// package's node_modules.
225-
dependencies[name] = `file:${fromFileUrl(new URL(`${member.dir}/npm`, repoRoot))}`;
275+
internal[name] = member.version;
276+
dependencies[name] = localRange(repoRoot, member.dir);
226277
}
227278

228279
// Library entry points come from deno.json exports. An executable comes from
@@ -311,6 +362,8 @@ function* buildPackage(pkgArg: string, version: string, ctx: BuildContext): Oper
311362
),
312363
);
313364

365+
yield* ctx.observe({ type: "package-build-started", package: denoJson.name, dependencies });
366+
314367
// The build tree is removed when this scope closes, before the finished
315368
// package is completed below.
316369
yield* scoped(function* () {
@@ -323,10 +376,10 @@ function* buildPackage(pkgArg: string, version: string, ctx: BuildContext): Oper
323376
importMap: join(srcCopy, "deno.json"),
324377
shims: { deno: false },
325378
test: false,
326-
// Internal @executablemd deps are published tier-by-tier, so a downstream
327-
// package's siblings are already on npm when it builds in CI. For local
328-
// builds (before siblings are published) set DNT_SKIP_INSTALL=1 to skip
329-
// the npm install + type check that would otherwise 404 on them.
379+
// The install resolves internal siblings from the artifacts phase 1
380+
// already built, so it never reaches npm for a package from this
381+
// release. DNT_SKIP_INSTALL=1 drops the install and typecheck entirely,
382+
// which only a leaf package may ask for (refused above).
330383
skipNpmInstall: skipInstall,
331384
typeCheck: skipInstall ? false : "single",
332385
declaration: "separate",
@@ -383,8 +436,153 @@ function* buildPackage(pkgArg: string, version: string, ctx: BuildContext): Oper
383436
yield* copyFile(new URL(asset, pkgDir), target);
384437
}
385438

386-
ctx.built.add(denoJson.name);
387-
const provenance =
388-
ctx.localSiblings && workspaceDeps.length > 0 ? " (local siblings — not publishable)" : "";
389-
console.log(`built ${denoJson.name}@${version} -> ${pkgArg}/npm${provenance}`);
439+
ctx.built.set(denoJson.name, { name: denoJson.name, dir: pkgArg, internal });
440+
yield* ctx.observe({ type: "package-build-completed", package: denoJson.name });
441+
console.log(`built ${denoJson.name}@${version} -> ${pkgArg}/npm`);
442+
}
443+
444+
/** The dependency maps npm reads, so validation misses none of them. */
445+
const DEPENDENCY_FIELDS = [
446+
"dependencies",
447+
"devDependencies",
448+
"peerDependencies",
449+
"optionalDependencies",
450+
];
451+
452+
function isRecord(value: unknown): value is Record<string, unknown> {
453+
return typeof value === "object" && value !== null && !Array.isArray(value);
454+
}
455+
456+
/** Every string anywhere in `value`, so a workspace path cannot hide in a field nobody checks. */
457+
function* strings(value: unknown): Generator<string> {
458+
if (typeof value === "string") {
459+
yield value;
460+
return;
461+
}
462+
if (Array.isArray(value)) {
463+
for (const item of value) {
464+
yield* strings(item);
465+
}
466+
return;
467+
}
468+
if (isRecord(value)) {
469+
for (const item of Object.values(value)) {
470+
yield* strings(item);
471+
}
472+
}
473+
}
474+
475+
/**
476+
* Why `manifest` cannot be published, or `undefined` when it can.
477+
*
478+
* A release gate rather than cleanup. A known internal local range is rewritten
479+
* by the caller before this runs; anything local still standing is an
480+
* unexpected dependency, and normalizing it away would be the one edit that
481+
* makes an unpublishable artifact look fine.
482+
*/
483+
function unpublishable(manifest: unknown, repoRoot: URL): string | undefined {
484+
if (!isRecord(manifest)) {
485+
return "it is not an object";
486+
}
487+
488+
for (const field of DEPENDENCY_FIELDS) {
489+
const map = manifest[field];
490+
if (!isRecord(map)) {
491+
continue;
492+
}
493+
for (const [name, range] of Object.entries(map)) {
494+
if (
495+
typeof range === "string" &&
496+
(range.startsWith("workspace:") || range.startsWith("file:"))
497+
) {
498+
return `${field}["${name}"] is the local range ${range}`;
499+
}
500+
}
501+
}
502+
503+
const nativeRoot = fromFileUrl(repoRoot).replace(new RegExp(`${sep}$`), "");
504+
const urlRoot = repoRoot.href.replace(/\/$/, "");
505+
for (const value of strings(manifest)) {
506+
if (value.includes(nativeRoot)) {
507+
return `it names the workspace path ${nativeRoot}`;
508+
}
509+
if (value.includes(urlRoot)) {
510+
return `it names the workspace URL ${urlRoot}`;
511+
}
512+
}
513+
514+
return undefined;
515+
}
516+
517+
/**
518+
* Phase 2. Every artifact this invocation produced becomes publishable at once,
519+
* after the last dnt call returned — see the two-phase note at the top of this
520+
* file for why it cannot happen package by package.
521+
*/
522+
function* finalizeClosure(ctx: BuildContext): Operation<void> {
523+
yield* ctx.observe({ type: "closure-finalization-started" });
524+
525+
const candidates: Array<{ name: string; url: URL; manifest: Record<string, unknown> }> = [];
526+
527+
for (const artifact of ctx.built.values()) {
528+
const url = new URL(`${artifact.dir}/npm/package.json`, ctx.repoRoot);
529+
const manifest: unknown = JSON.parse(yield* readTextFile(url));
530+
if (!isRecord(manifest)) {
531+
throw new Error(`${artifact.name}'s generated package.json is not an object`);
532+
}
533+
for (const field of DEPENDENCY_FIELDS) {
534+
const map = manifest[field];
535+
if (!isRecord(map)) {
536+
continue;
537+
}
538+
for (const [name, version] of Object.entries(artifact.internal)) {
539+
const member = ctx.members[name];
540+
if (member && map[name] === localRange(ctx.repoRoot, member.dir)) {
541+
map[name] = `^${version}`;
542+
}
543+
}
544+
}
545+
candidates.push({ name: artifact.name, url, manifest });
546+
}
547+
548+
// Every manifest is judged before any is written, so a closure that cannot be
549+
// published in full is never half-published.
550+
for (const candidate of candidates) {
551+
const refusal = unpublishable(candidate.manifest, ctx.repoRoot);
552+
if (refusal !== undefined) {
553+
throw new Error(`${candidate.name} cannot be published: ${refusal}`);
554+
}
555+
}
556+
557+
for (const candidate of candidates) {
558+
yield* writeTextFile(candidate.url, `${JSON.stringify(candidate.manifest, null, 2)}\n`);
559+
yield* ctx.observe({ type: "package-manifest-finalized", package: candidate.name });
560+
}
561+
}
562+
563+
if (import.meta.main) {
564+
await main(function* (args) {
565+
const pkgArg = args[0];
566+
const version = args[1] ?? "0.0.0-dev";
567+
568+
if (!pkgArg) {
569+
console.error("usage: build-npm.ts <package-dir> [version]");
570+
yield* exit(1);
571+
return;
572+
}
573+
574+
// Before anything is emitted. Copying the documentation assets is not the
575+
// same as validating them: a package built from a set that has drifted from
576+
// the components it documents would install cleanly and refuse the first
577+
// time somebody asked it for documentation. The same assembly the run
578+
// profile uses runs here, so a missing, unknown or duplicated section fails
579+
// the build for exactly the reason it would fail a run.
580+
yield* validateDocumentation();
581+
582+
yield* buildNpmPackage({
583+
repoRoot: new URL("../", import.meta.url),
584+
package: pkgArg,
585+
version,
586+
});
587+
});
390588
}

0 commit comments

Comments
 (0)