install: remove node_modules entries that left the lockfile

Closed
RO
2mo ago
+844-56

Fixes #16176. Fixes #8662.

What does this PR do?

bun install now removes root node_modules entries that are no longer reachable from the lockfile, for both the hoisted and the isolated linker, so a dependency removed from package.json stops being importable instead of lingering on disk forever. Details below.

Problem

Removing a dependency from package.json and running bun install prunes it from bun.lock but never from node_modules. The removed package stays importable on every machine that had installed it (a phantom dependency) and breaks on a fresh clone or in CI. Repeated bun install keeps reporting "no changes" and never cleans it up. Both the hoisted and the isolated linker are affected, and the same thing happens when an upgrade shrinks a package's transitive closure (the #16176 repro).

mkdir repro && cd repro
echo '{"name":"t","dependencies":{"alpha":"^1","gamma":"^1"}}' > package.json
bun install
echo '{"name":"t","dependencies":{"alpha":"^1"}}' > package.json
bun install
grep -c gamma bun.lock            # 0, gone from the lockfile
ls node_modules/gamma             # still there, still require()-able

npm, pnpm, and yarn all remove node_modules/gamma here.

Cause

Both linkers drive purely off the lockfile tree and only add or update the entries it contains. Nothing ever walks node_modules on disk to compare against the new graph. The lockfile diff even notices (src/install/lockfile/Package.rs counts summary.remove with a comment saying "It will be cleaned up later") but nothing does the cleanup. The only place a stale top-level directory is deleted is the bun remove <pkg> subcommand's hardcoded cleanup, which a plain bun install never reaches.

Fix

Add prune_extraneous_node_modules (src/install/PackageInstall.rs), called by both linkers after Lockfile::clean_with_logger has rebuilt the lockfile. It walks the top level of the root node_modules (descending one level into @scope/ directories, and removing a scope directory that ends up empty) and deletes any entry whose folder name is not a dependency alias in the lockfile. Dot-prefixed entries (.bin, .bun, .cache, ...) are never touched, and the pass is skipped for the security scanner's narrowed pre-install (packages_to_install); the full install that follows performs it.

Each linker builds its own kept set as "every folder name the lockfile can legitimately place at the root", independent of install flags: the root package's declared dependency aliases (all behaviors, so --production, --omit, --filter, and --cpu/--os overrides never turn a reinstall into a delete of packages the lockfile still contains) unioned with the linker's own root placements and with the names of packages that have a patchedDependencies entry. For the hoisted linker the root placements are the unfiltered root tree (snapshotted before filter() runs), because hoisted transitives legitimately live at the root; for the isolated linker they are every lockfile alias matching publicHoistPattern (the store's own root entry cannot be used because it is narrowed by --filter). Patched package names are included because bun patch materializes the package being patched at the root node_modules/<name> even when no tree places it there; without them, bun patch --commit removed the folder it had just told the user about (caught by the existing bun patch workspace test in bun-patch.test.ts, which passes again). A direct dependency that is removed but survives as someone else's transitive is therefore kept by the hoisted linker (it is still correctly hoisted there) and pruned by the isolated linker, whose root must only contain direct dependencies.

Two places the prune deliberately never runs or reaches:

  • Global installs. bun add -g/bun install -g operate inside the global install directory, which is also the bun link registry: bun link records a package there as a bare symlink with no entry in the global package.json or lockfile, so pruning it would destroy every link registration. The global directory is a tool registry, not a project.
  • Symlinks, under the hoisted linker. bun link <pkg> in a consumer is --no-save by default, so the local node_modules/<pkg> symlink is also recorded in no manifest. Hoisted extraneous entries are always real extracted directories; a root symlink there is a workspace link, a file: folder dependency, or a bun link, none of which the prune should touch (declining to delete them is exactly main's behavior today). The isolated linker's own root entries are symlinks into node_modules/.bun, so it keeps pruning them; isolated plus an unsaved bun link is a pre-existing gap (#30289).

Two details worth calling out:

  • The set is keyed by the alias (Dependency.name), because that is the node_modules/<name> folder both installers write. Dependency.name_hash is the hash of the resolved package name, so for "moo": "npm:@barn/moo" it would miss node_modules/moo and delete it.
  • A pruned package's node_modules/.bin/<cmd> symlink would otherwise be left dangling (turning a stale-but-working bin into an ENOENT), so the prune finishes with the dangling-symlink sweep bun remove already performs. That sweep is extracted into a shared prune_dangling_bin_links and the bun remove path now calls it. The prune itself is best effort: a stale directory the filesystem refuses to delete (locked, read-only) is left behind rather than failing the install, matching bun remove and npm.

The existing add trusted, delete, then add again test in bun-install-lifecycle-scripts.test.ts was written with a comment saying to update it "when we change bun install to delete dependencies from node_modules for both cases"; updated as instructed (the manual-edit branch now matches the bun rm branch, and the re-add installs the pruned transitive dependency as well).

Unrelated to the prune but required to run these suites at all: the test harness now starts Verdaccio bound to 127.0.0.1 explicitly. With only a port it listens on whatever getaddrinfo("localhost") returns first, and on hosts where that is ::1 every registry-backed install test fails with ConnectionRefused.

How did you verify your code works?

12 new tests in test/cli/install/bun-install-registry.test.ts under prunes stale node_modules entries (<linker>), run for both the hoisted and the isolated linker:

  • removing a direct dependency removes it from node_modules
  • removing a scoped dependency removes it and cleans up the empty @scope directory
  • removing a dependency with a transitive dep removes both (the #16176 shape; #8662 is the same mechanism via bun remove, covered by the updated add trusted, delete, then add again lifecycle test, whose hoisted transitive what-bin is now pruned too)
  • unrelated user directories and dotfiles are not removed
  • removing the last dependency removes it
  • a file: folder dependency survives the prune
  • a dependency demoted to transitive-only is kept by the hoisted linker and pruned from the root by the isolated linker
  • removing a dependency also removes its now-dangling .bin symlink
  • a publicHoistPattern hoist from a --filtered-out workspace survives a filtered install

plus two standalone tests: a bun link registration in the global directory survives bun add -g, and an unsaved bun link <pkg> in a consumer survives bun install.

16 of them fail on the current release build and pass with this change, as do the 4 updated lifecycle-script tests. The --filter, global-install, and unsaved-link tests additionally fail on intermediate revisions of this branch that lacked their respective guards, which is how those three cases were pinned.

Beyond the suites, both linkers were checked against a workspace symlink at the root (preserved), --production with dev dependencies on disk (preserved), an npm: aliased scoped package (preserved, while an unrelated removed package alongside it is pruned), and a publicHoistPattern-hoisted transitive at the isolated root (preserved).

The per-workspace node_modules directories are out of scope here; the hoisted side of that is #29793 / #29794.