fix(create): clean pnpm scaffolds + package-manager-aware next steps#235
Conversation
…er-aware next steps
🦋 Changeset detectedLatest commit: fa76a42 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughScaffolding now retries failed pnpm installations after approving esbuild build scripts. The CLI generates next-step commands using the detected package manager and the available 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/create/src/create.ts`:
- Around line 199-214: Update the retry logic around the pnpm fallback and the
error produced by exec so the catch can inspect preserved diagnostic output,
proceed only when the failure is ERR_PNPM_IGNORED_BUILDS and the report names
esbuild, and otherwise return false immediately. Keep the existing
approve-builds esbuild and install retry unchanged for the qualifying case,
while retaining the original error output for diagnostics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8a50d764-c77f-4a8a-bc97-46a748bbb675
📒 Files selected for processing (3)
.changeset/scaffold-pnpm-polish.mdpackages/create/src/cli.tspackages/create/src/create.ts
| // pnpm exits non-zero when it blocks a dependency's build scripts. The | ||
| // vite-based templates rely on esbuild's, so approve just esbuild — pnpm | ||
| // writes nothing and no-ops if esbuild wasn't actually installed — then | ||
| // re-install to confirm that was the only problem. Any other package | ||
| // manager (or a genuine pnpm failure) is a real error. | ||
| if (installer !== "pnpm") return false; | ||
|
|
||
| try { | ||
| await exec(cwd, installer, ["approve-builds", "esbuild"], { | ||
| shell: true, | ||
| }); | ||
| await exec(cwd, installer, ["install"], { shell: true }); | ||
| return true; | ||
| } catch { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Gate the retry on the actual esbuild ignored-build error.
packages/create/src/exec.ts rejects with only a generic exit-code error, so this catch cannot distinguish ERR_PNPM_IGNORED_BUILDS from network, authentication, lockfile, or unrelated dependency failures. Every pnpm failure therefore runs approve-builds esbuild, potentially persisting build configuration in templates without esbuild, before retrying. Preserve the diagnostic output and only approve/retry when the ignored-build report names esbuild; otherwise return false.
Suggested gating shape
- } catch {
- if (installer !== "pnpm") return false;
+ } catch (err) {
+ if (
+ installer !== "pnpm" ||
+ !isEsbuildIgnoredBuildError(err)
+ ) {
+ return false;
+ }🧰 Tools
🪛 OpenGrep (1.25.0)
[ERROR] 207-209: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
[ERROR] 210-210: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/create/src/create.ts` around lines 199 - 214, Update the retry logic
around the pnpm fallback and the error produced by exec so the catch can inspect
preserved diagnostic output, proceed only when the failure is
ERR_PNPM_IGNORED_BUILDS and the report names esbuild, and otherwise return false
immediately. Keep the existing approve-builds esbuild and install retry
unchanged for the qualifying case, while retaining the original error output for
diagnostics.
1 participant
Polish for
pnpm create markooutput:ERR_PNPM_IGNORED_BUILDSand left esbuild unbuilt.createnow runspnpm approve-builds esbuild(only esbuild, and only persists theallowBuildsconfig when esbuild was actually installed) and re-installs to confirm.Downloading → Installing dependencies with pnpm → Setting up git repository → Project created. The noisy internals (the transient ignored-builds error, esbuild postinstall, the confirm re-install, husky's.git can't be found, and git probes likefatal: not a git repository) are hidden. Install output is surfaced only if it genuinely fails.pnpm run dev) instead of alwaysnpm run dev.Verified end-to-end: pnpm
appscaffold shows clean spinner output, still approves + builds esbuild (allowBuilds: { esbuild: true }), inits git, and the app builds.