fix(types): recognise an aliased package root by the name its declaration promises - #15563
Conversation
…tion promises `createHostImporter`'s #13330 condition re-decision recognised the package root by walking up from the CJS resolver's answer until it found a manifest named after the DECLARATION KEY. An aliased install — `{"foo": "npm:bar@1"}` — puts a manifest named `bar` there, so the walk never matched, `esmEntryForDeclared` returned `undefined`, and `?? resolved` handed back the entry CommonJS resolution had answered: the `require` condition. For an aliased dual publish that is the two-instances-of-one-package split #13330 exists to remove. The expectation now comes from `declaredManifestName` — the reading #14278 gave the ESM-only fallback finder — so both legs expect one name from one place. The helper moves above both rather than staying inside the fallback section. Only the EXPECTATION moves, never the strictness: an alias naming one package still does not license a directory holding another, and a non-aliased declaration is still verified against its key. The exports SUBPATH stays cut from the KEY, which is what the specifier is spelled with. `packageRootOf` is module-private, called at one site, and named by neither subpath export of `@objectstack/types`, so the signature change is unreachable by any consumer. Part of #15044 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
📓 Docs Drift Check5 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 2 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 05362b555824f655d7780496e06bacaec514f081 && git checkout 05362b555824f655d7780496e06bacaec514f081
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin cf6b67164e37b92cf1fb6b172aa8f44e1fdce666 eb613ef6366ee8d24e90d958ed600791120af0c5 && git checkout -B drift-repro cf6b67164e37b92cf1fb6b172aa8f44e1fdce666 && git merge --no-ff eb613ef6366ee8d24e90d958ed600791120af0c5
node scripts/docs-audit/affected-docs.mjs --json cf6b67164e37b92cf1fb6b172aa8f44e1fdce666 |
Fixes #15044
createHostImporter's declared leg re-decides the condition for a package the CommonJS resolver has already located (the #13330 contract): it asks that package which entry animport()gets, so the caller's ESM chain and this load share one instance. That re-decision recognised the package root by walking up from the resolved entry until it found apackage.jsonwhosenameequals the declaration key.An aliased install —
{"dependencies": {"foo": "npm:bar@1"}}— puts a manifest namedbarunder the keyfoo. The walk therefore never matched,esmEntryForDeclaredreturnedundefined, and the?? resolvedat the call site handed back the entry CommonJS resolution had answered: therequirecondition. For an aliased dual-published package the process was left holding two live copies of one package — the CommonJS build behind the host importer, theimportbuild in the caller's own chain — which is exactly the split #13330 exists to remove.The population measurement, first
Triage fenced this card: if the affected population is not narrow — ordinary installs reach this path too, or an in-tree package is dual-published and aliased — stop and report. Measured before the fix, on
origin/main. The fence holds.npm:, aliasedworkspace:NAME@RANGE) across 81 workspace manifests / 875 dependency declarationsworkspace:rangepnpm-lock.yamlnpm:hits are allengines:maps and one dependency literally namednpm; positive control: 855version:hits)importand arequirecondition)node_modules/KEY/package.jsonname equals the key — i.e. the walk matches, build-independentpackageRootOfPositive control for the zero: the same sweep pointed at a synthetic host declaring
{"foo": "npm:bar@1"}reports 1 mismatch of 2 declarations, flagged dual-published. The instrument can report a mismatch; the repo has none.⇒ No ordinary, non-aliased install reaches this path. The
p3grading and the "restores a contract, does not widen a surface" call both stand.The change
esmEntryForDeclarednow takes the wholeHostDeclaration, exactly as the #14041 fallback leg has since #14278, and recognises the package root bydeclaredManifestName(declaration)— the name the host's own declaration promises. That helper moved above both legs rather than staying inside the fallback section, because both now read it.Two names, two questions. The package root is recognised by the name the declaration promises; the exports subpath is still cut from the declaration key, because the key is what the specifier is spelled with —
aliased/pluginaddresses./pluginof whateveraliasedaliases. They coincide for every ordinary dependency, which is why one name served both until an alias pulled them apart. A regression test pins it.Only the expectation moves, never the strictness. An alias naming one package still does not license a directory holding another; a non-aliased declaration is still verified against its key;
link:/file:name a location rather than a package, so no name can be parsed out of them and they keep today's behaviour — pinned as a declared residue rather than left silent.Nothing is exported.
packageRootOfis module-private, called at one site, and named by neither subpath export of@objectstack/types, so the signature change is unreachable by any consumer.Before / after, shown rather than described
Ablation on the committed tree at
eb613ef6366:packageRootOf's argument reverted to the declaration key, mutation confirmed on disk by counting both the removed text (1 to 0) and an injected marker (0 to 1), plus a worktree blob-hash change.Predicted direction, stated before the run: red — exactly the 5 aliased cases, with all 7 non-aliased / tightness / boundary controls staying green. Observed:
Tests 5 failed | 65 passed (70), and the 5 are precisely the predicted ones.Restore proved by blob-hash equality with the HEAD blob (
97d373138e422e5e011596a33655eeb7314dd619), an emptygit diff HEAD, an emptygit status --porcelain, and an absent-marker count of 0.Which resolution path the suite is on: source.
src/node.test.tsimports./node.jsrelatively and the package's vitest config declares no alias — and the ablation proves it rather than asserting it:packages/types/dist/node.jsanddist/node.mjsexisted at the time of the run (the whole workspace closure was built for the ratchet gates) and were never touched, yet a source-only mutation reddened the suite. A dist-resolving harness would have stayed green.The controls that matter more than the fix
The risk here is not the aliased case being corrected — it is a regression in the ordinary case that already works. Four controls, all in the same host app as the aliased fixtures, all green before and after:
importbuild;requirecondition still loads;link:target whose manifest names something else keeps today'srequire-condition load.Plus the reader control the #13330 suite established: an empty registry is proved to be a reading, not an instrument that can only ever answer empty.
No ADR-0112 envelope is asserted because none is produced: this path never throws, before or after — the change moves which entry a working load returns, and can turn no working load into a failure.
Verification
All run on
eb613ef6366, the head of this branch; exit codes captured before any pipe.pnpm --filter @objectstack/types test— 19 files, 573 tests passed (src/node.test.tsalone: 70 passed, 12 of them new)pnpm --filter @objectstack/types typecheck— exit 0;tsc --noEmit --listFilesconfirms bothsrc/node.tsandsrc/node.test.tsare in the program, so the green covers the new testsscripts/pm/dispatch-gates.mjsat this commit (36 by path + 7 by change kind + 7 declared whole-tree, 2 reached twice). 46 green on the first pass;check:dual-build-cjs-loadsandcheck:type-check-debtansweredPREREQUISITE NOT MET(exit 3, "this is NOT a pass") until the workspace closure was built, then both green —check:dual-build-cjs-loads: 103 published require entry points across 66 packages load;check:type-check-debt --re-measure: 13 ledger entries re-measured, 143 raw errors, none above its recorded numberpnpm lint(repo-wideeslint . --no-inline-config) — exit 0, no narrowing claimedChangeset
patch, for@objectstack/types. The Check Changeset rule reservesminorfor a purely additive widening of a published package's public surface (a new exported symbol on an index, a new accepted key or value). This exports no new symbol, accepts no new key and no new value; it changes which entry a workingimport()returns for a population measured at zero in this workspace, restoring the condition contract the declared leg already promises. That is a bug fix in a released package, which takespatch. Not breaking — nothing an author can write is removed or renamed — so no ADR-0087 disposition marker is owed.🤖 Generated with Claude Code
Generated by Claude Code
Generated by Claude Code