Uh oh!
There was an error while loading. Please reload this page.
fix(scripts): fail fast when the spec-dist override's own deps don't resolve - #5995
Merged
Merged
Conversation
…resolve
resolveSpecDistInjection validated that the override's exports-map targets
resolve, but not that the override's own package.json `dependencies` do.
A spec dist built without a reachable node_modules passed validation
cleanly and only failed deep inside the console build, in an error naming
a dependency (zod, observed) but never OBJECTSTACK_SPEC_DIST.
Adds assertSpecDependenciesResolve, called right after the package
directory is located: it reads the override's own `dependencies` (not
peerDependencies, not transitive deps) and confirms each resolves via a
manual ancestor node_modules walk, failing loudly with both the override
and every unresolved dependency named.
Measured and worth recording: a require.resolve(name, { paths: [packageDir] })
implementation of this same check passed silently every time it ran
through the real `vite` CLI, because pnpm's own `apps/console/node_modules/
.bin/vite` shim exports NODE_PATH pointing at the workspace's flat
`.pnpm/node_modules` hoist directory, and Node's `paths` option does not
suppress NODE_PATH. The manual directory walk consults nothing global and
is immune to it; a regression test pins the failure mode directly.
This was referenced Aug 24, 2026
yinlianghui-tw
marked this pull request as ready for review
August 24, 2026 13:44
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#5391
What changed
resolveSpecDistInjection(scripts/vite-objectstack-spec-dist.ts) validated that the override'sexportstargets resolve, but not that the override's ownpackage.jsondependenciesdo. This addsassertSpecDependenciesResolve, called right after the package directory is located and before the exports map is read: it walks eachdependenciesentry up frompackageDir's ancestornode_modulesdirectories and fails loudly, naming both the override and every unresolved dependency, if any are missing.Deliberately narrow, per the card: only the manifest's own
dependencies(notpeerDependencies— those are supplied by the consuming app by design, not the override's own tree — and not transitive dependencies, which fail the same way one frame further into their own resolution, still at build time).Measurement note — the reproduction differs from the issue's description
Reproduced with a real dep-less copy of the installed
@objectstack/spec(itsdist/, no reachablenode_modules) injected into a realapps/consolevite build. On this repo's current toolchain (vite 8.2.1 + rolldown 1.2.3,d8b48f495), the late failure was not the hardRolldown failed to resolve import "zod"error the issue describes. Instead the build ran to completion of the transform phase (8613 modules) with zero warning or error for the missingzod— it was silently kept as an unresolved bareimport ... from "zod"in the output chunk — and only crashed downstream, and only because ofpg-connection-string(the other missing dependency), as an unrelated plugin'sENOENTreading a chunk file that was never written:So today the late failure is arguably worse than described — silent for
zodspecifically, and only incidentally loud (in a confusing, unrelated plugin) forpg-connection-string. This does not change the fix; it strengthens the case for it. Full before/after logs are in the dev report.A second, load-bearing finding along the way
The first implementation used
require.resolve(name, { paths: [packageDir] }), which is the obvious tool for this job and is what the file's own header suggests ("the same walk Node itself performs"). It passed the dep-less fixture cleanly — but only when invoked directly withnode. Run through the realviteCLI it never caught anything, because pnpm's ownapps/console/node_modules/.bin/viteshim exportsNODE_PATHpointing at pnpm's flat.pnpm/node_moduleshoist directory (which holds a copy of nearly every package the workspace has ever installed,zodincluded), and Node'spathsoption does not suppressNODE_PATH—Module.globalPathsis consulted regardless. The shipped implementation instead does a manual ancestornode_moduleswalk (mirroringfindSpecPackageDir's own idiom), which consults nothing global. A regression test (is not fooled by a dependency name that resolves globally but not from the package directory) pins this directly, usingtypescript— a name genuinely reachable elsewhere in the repo — as the stand-in.Tests
scripts/__tests__/vite-objectstack-spec-dist.test.ts— 8 new cases: throws naming the override + dependency; names every unresolved dependency, not just the first; passes once the dependency is reachable (ownnode_modulesand an ancestor's); ignorespeerDependencies; inert whendependenciesis absent; the real installed spec (which does declarezod+pg-connection-string) passes; and theNODE_PATH-blindness regression test above. All 33 tests in the file pass, including the 25 pre-existing ones.apps/consolevite buildexercising the injection path three ways: unset (baseline, unaffected,EXIT=0), override at a dep-less fixture (now fails in ~1s at config-load time instead of ~80s–2min into the build), override at the same fixture with its dependencies symlinked in (EXIT=0, injection still works end-to-end).Gates run
pnpm type-check:scripts— passscripts/__tests__/vite-objectstack-spec-dist.test.ts— 33/33 passscripts/__tests__/scripts-type-check.test.ts— 9/9 pass (relevant: tsconfig.scripts.json claims to match apps/console/tsconfig.node.json's option set but omits allowImportingTsExtensions — the shared vite-*.ts files are green in one program, red in the other #4926 touchedtsconfig.scripts.jsontoday)eslint scripts/vite-objectstack-spec-dist.ts scripts/__tests__/vite-objectstack-spec-dist.test.ts— 0 errors (5 pre-existingno-explicit-anywarnings elsewhere in the test file, none in added lines)node scripts/check-changeset-presence.mjs— 0 files under a released package'ssrc/, no changeset owednode scripts/check-control-bytes.mjs— cleanapps/consolevite build, three configurations (see above)Generated by Claude Code
Generated by Claude Code