feat(cli): run app dev candidates through execa#117
Open
AmanVarshney01 wants to merge 2 commits into
Open
Conversation
Migrates the local dev-server launcher ladder from raw node:child_process spawn to execa (already a CLI dependency). execa spawns through cross-spawn, so on Windows the npx rung resolves its .cmd shim via PATHEXT instead of always failing with a false ENOENT, which made the Next.js ladder bunx-or-bust there. The hand-rolled win32 next.cmd special case goes away for the same reason. Behavior preserved: inherited stdio, ENOENT falls through to the next candidate, exit code and termination signal are reported (SIGTERM on abort still maps to the controller's cancel path), and the child env is passed exactly (extendEnv: false). Spawn failures other than ENOENT now surface with execa's message instead of a raw errno throw. Drops the unused spawnImpl injection seam; tests mock runLocalApp itself. Verified live: bun --watch runs and terminates cleanly on abort, and an all-ENOENT ladder still produces the friendly install message.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughLocal development command execution was standardized on 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Companion to project-compute#106 (approved): the SDK's spawn layer moved to execa there; this migrates the one remaining Windows-broken spawn site in the CLI, the
app devlauncher ladder inlib/app/local-dev.ts.Why
The ladder spawned candidates with raw
spawnand no shell. On Windows,npxis a.cmdshim, which rawspawncannot launch (only.exe/.comresolve), so the npx rung always failed with a false ENOENT and Next.js dev degraded to bunx-or-bust. There was even a hand-rollednext.cmdspecial case for the local-bin rung; execa (via cross-spawn, PATHEXT) makes that unnecessary, so it's gone.execa is already a CLI dependency (
controllers/init.ts,controllers/agent.ts), so no new ecosystem choice.Survey of remaining spawn sites (checked all of
packages/)lib/app/local-dev.ts: migrated here.shell/update-check.ts: spawnsprocess.execPath(an absolute path to the running Node binary) detached and unref'd. No PATH or shim resolution involved, no output handling, and execa adds nothing but risk aroundunrefsemantics. Left as is, deliberately.adapters/git.ts,lib/git/local-status.ts:execFile("git", ...);git.exeresolves fine on Windows. Left as is.Behavior preserved
AbortError) is unchanged.extendEnv: false).spawnImplinjection seam; nothing consumed it (tests mockrunLocalAppitself).Testing
bun --watchapp spawns, streams output, and terminates cleanly on abort (SIGTERMreported); an all-ENOENT ladder (empty PATH) produces the friendly message.