install: skip workspace members whose package.json has no name - #38781
install: skip workspace members whose package.json has no name#38781robobun wants to merge 4 commits into
Conversation
A package.json matched by a workspaces pattern (or listed explicitly) that
has no usable "name" failed the whole install with
'Missing "name" from package.json', and a pnpm-lock.yaml importer without
one failed the migration. pnpm and npm accept such members; monorepos
commonly match test fixtures like {"type":"module"} (vite has 23).
Workspaces are linked and resolved by name, so a nameless member is not a
workspace package: the scan and the pnpm migration now skip it, the same
way the scan already skipped "name": "". When the skipped package.json
declares dependencies, the root package.json parse warns once that they
are not installed. Because the scan itself stays quiet, the lockfile
migrations, --filter and the walk that locates the root from a member
directory (which previously also broke on a nameless sibling) do not
repeat the warning.
|
Warning Review limit reached
Next review available in: 6 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
Comment |
|
Status: ready for review; the diff is green on every CI lane that ran. Build 97050 (head d3b85cb) is red only because both Reproduced on the released binary with the minimal tree from the report (root with This is also what #6317 reports (nameless package.json files under On vite v8.2.1, migration now gets past its 23 nameless importers and stops at the relative |
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
|
Checked #6317: it is the same failure (nameless |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
I re-reviewed after 9e3b0cb/5e20ae5a/d3b85cbd — the duplicate-warning nit is fixed (the overlapping test variant asserts exactly one warning) and the comment-cop items are addressed; no new issues found. Since this is a user-facing bun install behavior change with a deliberate design choice (skip + warn rather than derive a name like npm, and unlike pnpm/npm the skipped member's dependencies are not installed), a maintainer sign-off on that direction would still be worthwhile.
Checked: warn_skipped is called from Package::parse after process_names_array returns, so the warnings don't trip the orig_msgs_len != log.msgs.len() error gate inside the scan; the pnpm migrator's continue on a nameless importer leaves it out of workspace_paths so later passes never see it; the removed MissingPackageName/WorkspaceNameMissing variants have no remaining references.
Extended reasoning...
Overview
The PR changes bun install to skip workspace members whose package.json has no "name" (or an empty/non-string one) instead of failing the whole install with Missing "name" from package.json. It touches src/install/lockfile/Package/WorkspaceMap.rs (the scan now returns a Scanned enum and records nameless-with-deps members for a deferred warning), src/install/lockfile/Package.rs (calls warn_skipped from the root parse), src/install/pnpm.rs and src/install/migration.rs (pnpm importers with no name are skipped; the WorkspaceNameMissing error and its message arm are removed), src/install/error.rs (MissingPackageName removed), docs/pm/workspaces.mdx, and adds tests to bun-workspaces.test.ts, pnpm-lock-v9.test.ts, and updates the arborist workspaces-need-update snapshot/notes.
Security risks
None identified. The change relaxes an error into a skip on locally-controlled package.json files; nothing new is read, resolved, linked into node_modules, or executed for the skipped directory. No untrusted-input parsing paths are widened.
Level of scrutiny
High. This is a user-facing behavior change to the package manager — critical-path code — and encodes a design decision that intentionally diverges from npm (which derives a name from the directory) and from pnpm/npm on one axis (they install the nameless member's dependencies; bun does not, and warns instead). The PR description argues the tradeoff well (derived names collide — five of vite's fixtures share the directory name nested; pnpm doesn't link them either), but a maintainer should confirm the direction.
Other factors
My earlier inline finding (duplicate warnings under overlapping workspaces patterns) was fixed in 9e3b0cb with a linear dedup and covered by the new overlapping test variant asserting exactly one warning. Comment-cop feedback was addressed in 5e20ae5/d3b85cbd. Test coverage is thorough: glob/listed/overlapping patterns × with/without dependencies, empty-string name, install from a member directory (root-finding walk), pnpm-lock.yaml migration + frozen install agreeing with the scan, and the arborist workspaces-need-update fixture now migrating. I verified the warning emission point sits after process_names_array returns, so the added log warnings don't hit that function's orig_msgs_len error gate; and that the pnpm migrator's continue keeps the nameless importer out of lockfile.workspace_paths so downstream passes are unaffected. Given the scope, deferring to a human for the design sign-off rather than approving.
|
Agreed that the direction is the thing to sign off on. The two choices are spelled out in the Fix section: skipping instead of deriving a name (derived names would be linked into node_modules and collide, e.g. five of vite's fixtures live in a directory called |
2 participants
Problem
bun installfails witherror: Missing "name" from package.json in pkgs/fixture/package.jsonas soon as one directory matched byworkspaces(glob or listed path) has a package.json without aname, and installs nothing. pnpm 11 and npm 11 install the same tree. Fixtures like{"type":"module"}under a workspaces glob are common: vite v8.2.1 has 23 of them underpackages/**/__tests__/**andplayground/**, none declaring dependencies. Ignore package.json files without "name" property #6317 reports the same failure for nameless package.json files used as folder metadata underpackages/features/*.warn: pnpm-lock.yaml migration failed due to missing workspace name., thenInvalidLockfile), and package-lock.json migration fails withInstallFailed(arborist fixtureworkspaces-need-update).bun installrun inside a named member whose sibling is nameless installed the member as a standalone project: the walk that looks for the workspace root (src/install/PackageManager.rs) scans the workspaces with the same code and gives up on the error.process_workspace_nameinsrc/install/lockfile/Package/WorkspaceMap.rsreturnedError::MissingPackageName, which both the listed-path and the glob branch ofprocess_names_arraylogged as an error;migrate_pnpm_lockfileinsrc/install/pnpm.rsreturnedWorkspaceNameMissingfor the importer. The scan already skipped"name": ""silently.Fix
process_workspace_namereturnsScanned::Namelessfor a missing, non-string or empty name and both branches skip the directory (the old empty-name check is folded into this path). The pnpm migrator applies the same rule and skips the importer; the later passes iteratelockfile.workspace_paths, so they never see it.Error::MissingPackageName,WorkspaceNameMissingand their message arms are removed.node_modules/<name>link, so derived names would link fixtures into node_modules under whatever the directory is called and collide with each other (five of vite's fixtures are in a directory namednested). pnpm does not link them either.Package::parsewarns once:warn: Skipping workspace "packages/fixture": its package.json has no "name", so its dependencies will not be installed. The warning is emitted from the root parse instead of the scan because the migrations,--filterand the root-finding walk also scan during the same command; emitting it in the scan printed it twice during a package-lock.json migration. Fixtures without dependencies (vite's) print nothing.Missing "name"errors, the migration warning, the member installing standalone, and no warning for"name": ""):test/cli/install/bun-workspaces.test.ts, "workspace member package.json without a name": glob, listed path and overlapping patterns that match the fixture three times, each with and without dependencies (the fixture declares a dependency that does not exist, so a successful install proves it was never resolved; the warning is asserted to appear exactly once), empty name, andbun installfrom a member directory finding the root and warning exactly once.test/cli/install/migration/pnpm-lock-v9.test.ts, "importer whose package.json has no name": workspaces defined only in pnpm-workspace.yaml with a__tests__glob, as in vite; the migration writes root +packages/aonly, and the followingbun install --frozen-lockfilepasses without rewriting bun.lock, i.e. the scan agrees with the migration. The second case checks the importer's recorded dependency is not migrated and that the install warns.test/cli/install/migration/migrate.test.ts: the arboristworkspaces-need-updatesnapshot now records a successful migration (fixture notes updated).bun pm migratenow gets past the 23 importers and stops at vite's relativelink:./fixtures/...dependencies (bun install: support non-existent workspaces and relative link dependencies #23026), whichbun installwithout a lockfile rejects too, so that repo additionally needsbun install: support non-existent workspaces and relative link dependencies #23026.docs/pm/workspaces.mdxdocuments the behavior.Background
Package::parsecallsWorkspaceMap::process_names_array, which resolves everyworkspacesentry (plain paths directly, globs through a directory walk) to a package.json and keys the map by the member's relative directory. Each entry then becomes aworkspace:dependency of the root named after the member, is linked atnode_modules/<name>, and is whatworkspace:specifiers elsewhere resolve to throughlockfile.workspace_paths, a map keyed by name hash. A member without a name therefore has nothing to be stored under.--filtertarget selection, and the startup walk that checks whether the cwd belongs to a workspace further up, each into a different log (the startup walk discards its log).workspace_pathsand finally copies pnpm-workspace.yaml'spackagesinto the root package.jsonworkspaces, so the root parse that follows scans the same globs and must reach the same conclusion about these directories.Fixes #6317
[review] gate passed · iteration 0 · 11 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file