Skip to content

bun: Properly represent workspace submodules as projects - #12367

Draft
fviernau wants to merge 5 commits into
mainfrom
bun-workpsaces-submodules-as-projects
Draft

bun: Properly represent workspace submodules as projects#12367
fviernau wants to merge 5 commits into
mainfrom
bun-workpsaces-submodules-as-projects

Conversation

@fviernau

@fviernau fviernau commented Aug 28, 2026

Copy link
Copy Markdown
Member

See individual commits.

Part of #12366.

@fviernau
fviernau requested a review from a team as a code owner August 28, 2026 08:13
@fviernau
fviernau enabled auto-merge (rebase) August 28, 2026 08:14
@sschuberth sschuberth changed the title bun: Properly represent workpsace submodules as projects bun: Properly represent workspace submodules as projects Aug 28, 2026
sschuberth
sschuberth reviewed Aug 28, 2026
View reviewed changes
Comment thread ...ckage-managers/node/src/funTest/assets/projects/synthetic/bun/workspaces-expected-output.yml
Comment thread plugins/package-managers/node/src/main/kotlin/bun/Bun.kt
.filterInstalled()

val scopes = Scope.entries.filterNotTo(mutableSetOf()) { scope -> scope.isExcluded(excludes, includes) }
requestAllPackageDetails(rootModuleInfo, scopes)

@sschuberth sschuberth Aug 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Should we keep the comment? Or align to the way it's now done for NPM, with // Warm-up the cache. in the same line?

@fviernau fviernau Aug 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an alignment with a preceeding revision of NPM.

I'll have a next PR which then aligns with latest revision, which implements you 2nd proposal.
Are you ok to do it like this?

Comment thread plugins/package-managers/node/src/main/kotlin/bun/Bun.kt Outdated
@fviernau
fviernau force-pushed the bun-workpsaces-submodules-as-projects branch from 8ea6f25 to 978088d Compare August 28, 2026 08:59
@fviernau
fviernau requested a review from sschuberth August 28, 2026 08:59
sschuberth
sschuberth previously approved these changes Aug 28, 2026
View reviewed changes
@fviernau
chore(node): Use / instead of File.resolve()
defeb64 
Signed-off-by: Frank Viernau <frank.viernau@gmail.com>
@fviernau
dep(node): Update Bun to the latest version
ad13fe8 
The version comes with a new lockfile format. So, re-create all
lockfiles.

In upcoming changes an additional use of the `NpmCommand` will be
introduced, which fails with the lockfiles of the currently used Bun
version, but succeeds with Bun 1.4.0. So, update also the version
requirement.

Signed-off-by: Frank Viernau <frank.viernau@gmail.com>
@fviernau
test(node): Align Bun's workspaces test with the one from NPM
e2d400c 
Simplify comparing the results.

Signed-off-by: Frank Viernau <frank.viernau@gmail.com>
@fviernau
test(bun): Improve the test for workspaces
4af9501 
The analysis should return multiple projects, but it returns only one.
Change the test to allow for returning multiple projects, to prepare
for an upcoming change.

Signed-off-by: Frank Viernau <frank.viernau@gmail.com>
@fviernau
feat(node): Add missing projects for Bun workspace modules
6f6cb53 
Previously, the IDs of workspace submodules only appeared as references
in scopes, but not as a `Project` (or `Package`). See also 29a0393.

Signed-off-by: Frank Viernau <frank.viernau@gmail.com>
@fviernau
fviernau force-pushed the bun-workpsaces-submodules-as-projects branch from 978088d to 6f6cb53 Compare August 28, 2026 10:44
@fviernau
fviernau marked this pull request as draft August 28, 2026 10:47
auto-merge was automatically disabled August 28, 2026 10:47

Pull request was converted to draft

@fviernau

fviernau commented Aug 28, 2026

Copy link
Copy Markdown
Member Author

I ran into the issue that Bun.kt uses npm list which seems to be incomopatible with the node_modules layout created by bun. In the test project, it returns errors an misses dependencies.
This needs to be carefully verified still, but if this is true, it could mean that the approach
Bun.kt follows, of using NPM command, does not work out and needs a re-write.

@codecov

codecov Bot commented Aug 28, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 59.17%. Comparing base (f2ece30) to head (6f6cb53).
⚠️ Report is 66 commits behind head on main.

Files with missing lines Patch % Lines
...s/package-managers/node/src/main/kotlin/bun/Bun.kt 95.83% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               main   #12367      +/-   ##
============================================
+ Coverage     59.12%   59.17%   +0.04%     
- Complexity     1880     1885       +5     
============================================
  Files           365      365              
  Lines         13798    13811      +13     
  Branches       1440     1442       +2     
============================================
+ Hits           8158     8172      +14     
- Misses         5111     5113       +2     
+ Partials        529      526       -3
Flag Coverage Δ
funTest-external-tools 16.00% <96.00%> (+0.14%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sschuberth sschuberth sschuberth left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@fviernau @sschuberth