Conversation
Bun apps with no dependencies never produce a lockfile, so the existing detection (which required bun.lock/bun.lockb) would miss them entirely. Added three new signals: bunfig.toml presence, the packageManager field in package.json, and bun commands appearing in package.json scripts. Also moved BunStack ahead of NodeStack in detection order so these Bun-specific signals get checked first — safe because NodeStack still requires its own lockfiles.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds Bun-specific detection to the stack builder: recognizes bun.lock / bun.lockb and bunfig.toml, detects Bun via package.json packageManager starting with "bun@", and detects Bun usage in npm scripts via a precompiled regex. New internal helpers parse package.json and inspect scripts; detection emits corresponding events. The overall stack detection order was changed by moving NodeStack to run after BunStack. Tests were added to cover bun lock files, bunfig.toml, packageManager, scripts invoking bun, Procfile cases, and negative scenarios (tests appear duplicated). Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pkg/stackbuild/bun.go (2)
58-82: Optional: Consider caching the package.json read.Both
detectPackageManagerBunanddetectBunInScripts(viagetPackageScripts) read and parsepackage.jsonseparately. For a small config file read once during detection this is fine, but consolidating into a single read could be cleaner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/stackbuild/bun.go` around lines 58 - 82, Both detectPackageManagerBun and detectBunInScripts re-read/parse package.json independently; change this by caching the package.json contents/parsed result on the BunStack instance so both detection helpers reuse it. Add a single loader method (e.g., loadPackageJSON or ensurePackageJSONLoaded) that uses s.readFile("package.json") and json.Unmarshal into a cached field on BunStack (e.g., pkgJSONData or parsedPackage struct) and return any error; then update detectPackageManagerBun to read from the cached parsedPackage.PackageManager and detectBunInScripts (and getPackageScripts) to use the cached scripts map/list instead of calling readFile again. Ensure the loader is idempotent (no duplicate parsing) and called lazily by both detectPackageManagerBun and detectBunInScripts.
72-82: Consider detectingbunxcommands in scripts as well.The regex correctly matches standalone
bunwhile avoiding false positives likebundle. However,bunxis Bun's equivalent ofnpxand also indicates Bun usage. Scripts like"test": "bunx vitest"would not be detected.If this is intentional for conservative detection, a brief comment explaining the choice would help maintainability.
Potential enhancement to include bunx
-var bunCommandRe = regexp.MustCompile(`(?:^|\s)bun(?:\s|$)`) +// Matches standalone "bun" or "bunx" commands in scripts. +// Uses whitespace/line anchors to avoid false positives like "bundle". +var bunCommandRe = regexp.MustCompile(`(?:^|\s)bunx?(?:\s|$)`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/stackbuild/bun.go` around lines 72 - 82, The script detection currently only matches standalone "bun" via bunCommandRe used in detectBunInScripts on BunStack (which calls getPackageScripts); update the regex to also match the "bunx" command (e.g., change bunCommandRe to match either "bun" or "bunx" as whole tokens) so scripts like "bunx vitest" are detected, and run/update any tests or usages relying on bunCommandRe; if the exclusion of bunx was intentional instead add a brief comment above bunCommandRe explaining why only "bun" is matched for future maintainability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/stackbuild/bun.go`:
- Around line 58-82: Both detectPackageManagerBun and detectBunInScripts
re-read/parse package.json independently; change this by caching the
package.json contents/parsed result on the BunStack instance so both detection
helpers reuse it. Add a single loader method (e.g., loadPackageJSON or
ensurePackageJSONLoaded) that uses s.readFile("package.json") and json.Unmarshal
into a cached field on BunStack (e.g., pkgJSONData or parsedPackage struct) and
return any error; then update detectPackageManagerBun to read from the cached
parsedPackage.PackageManager and detectBunInScripts (and getPackageScripts) to
use the cached scripts map/list instead of calling readFile again. Ensure the
loader is idempotent (no duplicate parsing) and called lazily by both
detectPackageManagerBun and detectBunInScripts.
- Around line 72-82: The script detection currently only matches standalone
"bun" via bunCommandRe used in detectBunInScripts on BunStack (which calls
getPackageScripts); update the regex to also match the "bunx" command (e.g.,
change bunCommandRe to match either "bun" or "bunx" as whole tokens) so scripts
like "bunx vitest" are detected, and run/update any tests or usages relying on
bunCommandRe; if the exclusion of bunx was intentional instead add a brief
comment above bunCommandRe explaining why only "bun" is matched for future
maintainability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 86e4734b-4973-4800-b0f5-f04974a7ffb2
📒 Files selected for processing (3)
pkg/stackbuild/bun.gopkg/stackbuild/stackbuild.gopkg/stackbuild/stackbuild_test.go
bunx is Bun's equivalent of npx, so scripts like "bunx vitest" are a valid Bun signal. Tweak the regex from `bun` to `bunx?` to catch both.
phinze
deleted the
phinze/mir-773-detect-bun-runtime-when-no-bunlock-file-is-present
branch
2 participants
Stack detection was gated on finding a
bun.lockorbun.lockbfile, which works great when there are dependencies — but a Bun app with no deps never generates a lockfile. That meant deploys would either fail or fall back to the wrong runtime, which isn't a great experience for someone just trying to ship a simple Bun server.We now pick up on three additional signals: a
bunfig.tomlconfig file, the"packageManager": "bun@..."field in package.json, and any package.json script that invokesbunas a command (using a word-boundary regex so things likebundledon't false-positive). The lock file checks are still there and still checked first, so existing projects are unaffected.Also swapped the detection order so Bun runs before Node. Both stacks look for package.json, but since all of Bun's new signals are unambiguous and Node still requires its own lock files, there's no risk of a Node project accidentally matching as Bun.
Closes MIR-773