Repository navigation
CLI: Allow esbuild builds for Storybook-owned pnpm dlx - #35742
Conversation
Avoids the interactive approve-builds hang on pnpm 11+ for dlx without persisting allowBuilds into the user's project via pnpm add. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the pnpm package-manager proxy in Storybook’s core CLI utilities to avoid pnpm 11+ interactive “approve builds” prompts (and related ERR_PNPM_IGNORED_BUILDS) during Storybook-owned pnpm dlx executions by conditionally adding --allow-build=esbuild when supported.
Changes:
- Add pnpm version detection (cached) and version-gated
--allow-build=esbuildargs for Storybook-ownedpnpm dlxinvocations (pnpm >= 10.2.0). - Ensure
--allow-buildis not applied topnpm install/pnpm addto avoid persistingallowBuildsinto user project config. - Add unit tests covering the flag placement/gating and ensuring installs remain “plain” (no
--allow-build).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| code/core/src/common/js-package-manager/PNPMProxy.ts | Adds cached pnpm version detection and conditionally prefixes Storybook-owned pnpm dlx calls with --allow-build=esbuild (pnpm >= 10.2.0). |
| code/core/src/common/js-package-manager/PNPMProxy.test.ts | Adds unit tests asserting the new dlx flag behavior and confirming install/add remain unchanged. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough
ChangesPNPM build approval handling
Sequence Diagram(s)sequenceDiagram
participant PNPMProxy
participant pnpm
participant semver
PNPMProxy->>pnpm: Read pnpm --version
PNPMProxy->>semver: Check version against 10.2.0
semver-->>PNPMProxy: Supported or unsupported
PNPMProxy->>pnpm: Run remote dlx with conditional build approval
Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
code/core/src/common/js-package-manager/PNPMProxy.test.ts (1)
28-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove mock behavior into
beforeEach.Lines 130, 132, 148, and 150 change mock behavior inside test cases. Set the command result and a mutable pnpm-version value in
beforeEach. In each boundary test, change only the version value before creatingPNPMProxy. Use a typed command-result fixture instead ofas any.As per coding guidelines, “Implement mock behaviors in
beforeEachblocks in Vitest tests” and “Avoid inline mock implementations within test cases in Vitest tests.”Also applies to: 129-132, 147-150
🤖 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 `@code/core/src/common/js-package-manager/PNPMProxy.test.ts` around lines 28 - 34, Move the mockedExecuteCommandSync implementation from mockPnpmVersion and individual tests into beforeEach, using a typed command-result fixture and a mutable pnpm-version value. Update each boundary test to change only that version before constructing PNPMProxy, while preserving the existing command matching and returned-result behavior.Source: Coding guidelines
🤖 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 `@code/core/src/common/js-package-manager/PNPMProxy.ts`:
- Line 14: Update the version parsing and gate in PNPMProxy to preserve
prerelease identifiers instead of using coerce, so versions such as 10.2.0-rc.1
are compared as prereleases and use plain dlx. Retain the existing allow-build
behavior for supported stable versions, and add coverage verifying the
10.2.0-rc.1 case.
---
Nitpick comments:
In `@code/core/src/common/js-package-manager/PNPMProxy.test.ts`:
- Around line 28-34: Move the mockedExecuteCommandSync implementation from
mockPnpmVersion and individual tests into beforeEach, using a typed
command-result fixture and a mutable pnpm-version value. Update each boundary
test to change only that version before constructing PNPMProxy, while preserving
the existing command matching and returned-result behavior.
🪄 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: 8fe6fd5a-3b04-4e5d-986e-a56c001834d6
📒 Files selected for processing (2)
code/core/src/common/js-package-manager/PNPMProxy.test.tscode/core/src/common/js-package-manager/PNPMProxy.ts
Inline the version gate, drop unused helpers, and keep only the dlx version-branch unit tests. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Check the diff here: storybookjs/storybook@bb5ab56...29a0e16 List of included PRs since previous version: - storybookjs/storybook#35660 (Insik-Han/fix/tanstack-standalone-index-clone-id) - storybookjs/storybook#35742 (fix/pnpm-dlx-allow-build-esbuild) - storybookjs/storybook#35713 (jeppe-cursor/setup-dev-environment-1230)
Closes #
What I did
Related to #35668. On pnpm 11+, Storybook-owned
pnpm dlxcan hang on an interactive approve-builds prompt (or fail withERR_PNPM_IGNORED_BUILDS) when esbuild needs a build script.I pass version-gated
--allow-build=esbuildonly for Storybook-ownedpnpm dlx(runPackageCommandwithuseRemotePkg: true) when local pnpm is >= 10.2.0.I deliberately do not pass
--allow-buildonpnpm addorpnpm install. Empirically on pnpm 11.18,pnpm add --allow-build=esbuildpersistsallowBuilds.esbuild: trueinto the user'spnpm-workspace.yaml;pnpm --allow-build=esbuild dlxdoes not. I also do not seed/writeallowBuildsinto the user's project.This supersedes the broader approach in #35671 with a narrower scope (dlx only; no add/install flags, no project config seeding, no tasks/verdaccio changes). It does not fully fix the TanStack Start outer install hang described in #35668 (that remains upstream).
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
pnpm dlx(e.g.pnpm dlx storybook@next init/ create-storybook via the Storybook-owned remote package command), in a TTY, without pre-approving esbuild builds.dlxstep.allowBuilds/pnpm.onlyBuiltDependencies(or similar) intopackage.json/pnpm-workspace.yamlsolely because of this change.--allow-build(unit tests cover this).Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsDeclare whether manual QA will be needed for this PR during the next release, through
qa:neededorqa:skipMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Made with Cursor