Make bare test modules actually reuse the outer install - #20
Merged
Conversation
BunTypeScriptTests extends upstream Mill's TypeScriptTests, so the unqualified this.tsDeps() inside bunTestPackageJson resolved to the Node toolchain defaults (ts-node, tsconfig-paths, @types/node) that the outer trait deliberately replaced. Those names always survived the outer-name filter, so a bare test module's package.json never equaled the outer's and the reusesOuterInstall branch was unreachable — with bunRequireLockfile on, every bare test module demanded its own lockfile, contradicting the documented contract. The trait now pins tsDeps to the outer module's. The guard test passed anyway because both of its assertions also hold in the broken standalone branch (the suite relaxes the lockfile requirement, and installs never write into the source tree). It now also asserts the returned install path is the outer module's npmInstall.dest — confirmed failing before this fix and passing after. Also: dev() on BunTypeScriptWebModule served straight out of the cached webStage output while its sync thread mirrored live edits (but never deletions) into it, and bundle builds from that same stage — a file created and deleted during a dev session would ship in the next production bundle. dev() now serves from a private copy in its own command dest; copyTree preserves the node_modules symlink, so the copy is cheap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 26, 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 free
to 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.
Stack 7/N. Base is
agent/bun-cache-hardening(#19), notmain.The bug
#16 gave TypeScript test modules their own lockfile identity, with an escape hatch: a bare
object test extends BunTypeScriptTeststhat adds nothing should reuse the outer module'sinstall instead of demanding a second lockfile. That escape hatch was dead code.
BunTypeScriptTestsextends upstream Mill'sTypeScriptTests, so the unqualifiedthis.tsDeps()inside
bunTestPackageJsonresolved to upstream's Node-toolchain defaults —ts-node,tsconfig-paths,@types/node— the exact list the outer trait deliberately replaced ("Bun onlyneeds TypeScript plus target-specific ambient types"). Those names always survived the
outer-name filter, so the test module's package.json never equaled the outer's, and
reusesOuterInstallwas false for every module in existence.This is the same defect class #16 fixed — a reference in the nested trait silently resolving to
the wrong module — one instance of which had survived that fix.
Consequences before this change:
bunRequireLockfile = true, a bare test module failednpmInstallwith "Missing
<module>/test/bun.lock", contradicting the documented contract.test/bun.lockdragged ints-node/tsconfig-paths/@types/node, which Bunnever uses and every test install then downloaded.
The existing guard test passed anyway, because both of its assertions also hold in the broken
non-reuse branch: the suite forces
MILL_BUN_REQUIRE_LOCKFILE=false, so the standalone installsucceeded, and it never writes into the source tree. It now also asserts the returned install
path is the outer's
npmInstall.dest— confirmed failing before the fix (the returned pathwas
out/app/test/npmInstall.dest) and passing after.Also fixed
BunTypeScriptWebModule.dev()served straight out of the cachedwebStageoutput while itssync thread mirrored live source edits (but never deletions) into it — and
bundlebuilds fromthat same stage. A file created and then deleted during a dev session would ship in the next
production bundle, because the source hash is back to its old value and Mill considers the
mutated stage up to date.
dev()now copies the stage into its own command dest (thenode_modulessymlink is preserved, so the copy is cheap) and serves from the copy.Tests
test modules adding nothing reuse the outer install(strengthened) — fails on the parent,passes here.
test modules with extra deps own their lockfilestill passes: modules that genuinely adddependencies keep their standalone install and their own lock.