fix(init): detect Bun and invoking package manager reliably

[liuxiaopai-ai]

Summary

Fixes package manager detection for vinext init / deploy dependency installs when lock files are missing or Bun is used.

What changed

• Extend lockfile detection to support both Bun lockfile names:
  • bun.lockb (legacy)
  • bun.lock (current)
• Detect package manager from package.json#packageManager when no lockfile exists.
• Detect invoking package manager from npm_config_user_agent as a final hint (e.g. bun x vinext init, pnpm dlx vinext init).
• Keep deterministic fallback to npm when no hints are available.
• Reuse detectPackageManagerName() in detectPackageManager() to keep behavior consistent.

Why

Issue #109 reports that running bun x vinext init still installs deps via npm. This happened because detection only checked lock files and only recognized bun.lockb.

Fixes #109

Test plan

• pnpm -s test tests/init.test.ts
• pnpm -s test tests/deploy.test.ts

New coverage (init tests)

• uses Bun when bun.lock exists
• uses packageManager from package.json when lockfiles are missing
• uses invoking package manager from npm_config_user_agent
• falls back to npm when no PM hints exist

[southpolesteve]

/bigbonk review

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/vinext@139

commit: a422274

[ask-bonk]

Good fix for #109. The detection priority (lockfile > package.json#packageManager > npm_config_user_agent > npm fallback) is well-chosen and matches how tools like Corepack and @antfu/ni handle this. Tests pass, deploy tests are unaffected.

A few observations:

Dual-purpose parsePackageManagerName — This function silently handles two different string formats (the name@version packageManager field and the name/version ... user-agent). It works correctly today, but the implicit format detection could be a source of subtle bugs if a new format is added later. Consider either splitting into two functions or adding a comment that the dual-format behavior is intentional. Not blocking.

Deterministic npm fallback test — The old "calls exec with npm when no lock file exists" test was implicitly relying on whatever npm_config_user_agent happened to be set in the environment. The new version explicitly clears it via withUserAgent(undefined, ...), making it deterministic. Good improvement.

withUserAgent and parallelism — The helper mutates process.env globally. Tests within a describe block run sequentially by default in Vitest, so this is fine today. If the test runner config ever enables concurrent execution within a file, this would need isolated workers or a different approach. Worth a brief comment in the helper.

Overall this is a clean, well-tested fix. The code is straightforward and the test coverage is solid.

[ask-bonk]

Nit: the Bun lockfile check on line 121 returns inline (return "bun") while the npm lockfile check on lines 122-124 uses a block with braces. Consider using the block form for both for consistency:

Suggested change

	}
	if (
	fs.existsSync(path.join(root, "bun.lockb")) ||
	fs.existsSync(path.join(root, "bun.lock"))
	) {
	return "bun";
	}

(This comment is on the wrong line due to GitHub API constraints — it refers to line 121.)

[ask-bonk]

Reading process.env.npm_config_user_agent directly makes this function impure — the same root can return different results depending on the environment. This is the correct behavior for the use case (detecting the invoking PM), but it makes the function harder to test in isolation. If you ever need to unit-test detectPackageManagerName directly (rather than through init()), consider accepting an optional env parameter with process.env as the default.

[ask-bonk]

Review submitted successfully. Here's a summary of what I reviewed:

PR #139: fix(init): detect Bun and invoking package manager reliably

The PR fixes issue #109 where bun x vinext init was falling through to npm for dependency installation. The changes are clean and well-structured:

What's good:

• Detection priority (lockfile > package.json#packageManager > npm_config_user_agent > npm fallback) is the industry-standard approach
• detectPackageManager() now delegates to detectPackageManagerName(), eliminating duplicated logic
• Both bun.lockb (legacy) and bun.lock (current) are recognized
• Explicit npm lockfile detection (package-lock.json, npm-shrinkwrap.json) ensures lockfiles always win over packageManager field
• The old npm fallback test was made deterministic by explicitly clearing npm_config_user_agent
• All 48 init tests and 138 deploy tests pass

Feedback posted:

1. Minor formatting suggestion — Bun lockfile check uses inline return while npm uses a block; suggest consistency
2. Note about process.env access making detectPackageManagerName impure — suggest optional env parameter for testability
3. Summary notes about the dual-purpose parsePackageManagerName function and the withUserAgent helper's global state mutation

github run