Core: Fix Yarn PnP support #31968

JReinhold · 2025-07-07T08:36:44Z

What I did

This PR makes some changes to the resolution logic to keep supporting Yarn PnP.
It also introduces a Yarn PnP "Test Storybook", that we can use to ensure it is supported. In CI it builds the Storybook and runs the Vitest tests.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

stories
unit tests
integration tests
end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

Add or update documentation reflecting your changes
If you are deprecating/removing a feature, make sure to update
MIGRATION.MD

Checklist for Maintainers

When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts
Make 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/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

Greptile Summary

Improves Yarn Plug'n'Play (PnP) support in Storybook by refactoring module resolution logic and adding comprehensive testing infrastructure.

Modified code/core/src/common/js-package-manager/Yarn2Proxy.ts to properly handle PnP API loading and error handling
Simplified builder resolution in code/core/src/core-server/utils/get-builders.ts by removing custom path handling
Added dedicated Yarn PnP test Storybook with complete Vite+React setup for CI verification
Implemented CircleCI job 'test-yarn-pnp' to ensure PnP compatibility across all workflows
Updated preset loading in code/core/src/common/presets.ts to use more reliable module resolution for PnP environments

Copilot

Pull Request Overview

This PR adds comprehensive Yarn Plug’n’Play support by configuring test sandboxes, streamlining module resolution in core utilities, and integrating a new CI job.

Added a Vitest-based workspace configuration for the Yarn PnP sandbox
Simplified builder lookup by removing configDir dependencies and switching to direct module imports
Updated addon and package resolution to leverage import.meta.resolve and built-in pnpapi
Introduced a dedicated test-yarn-pnp job in CircleCI workflows

Reviewed Changes

Copilot reviewed 34 out of 55 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
test-storybooks/yarn-pnp/vitest.workspace.ts	New Vitest workspace setup using `storybookTest` for PnP sandbox
code/core/src/core-server/utils/get-builders.ts	Removed `configDir` argument from `getPreviewBuilder` and updated callers
code/core/src/common/presets.ts	Replaced `resolvePathSync` with `importMetaResolve` + `fileURLToPath` in addon resolution
code/core/src/common/js-package-manager/Yarn2Proxy.ts	Switched to dynamic import of built-in `pnpapi` and adjusted error code check
.circleci/config.yml	Added `test-yarn-pnp` job to multiple workflows for end-to-end PnP validation

Comments suppressed due to low confidence (1)

code/core/src/core-server/utils/get-builders.ts:1

[nitpick] Public functions getManagerBuilder, getPreviewBuilder, and getBuilders are core parts of the builder resolution API. Adding JSDoc comments would improve clarity on expected inputs, outputs, and error cases.

import { MissingBuilderError } from 'storybook/internal/server-errors';

code/core/src/core-server/utils/get-builders.ts

code/core/src/common/presets.ts

nx-cloud · 2025-07-07T08:44:36Z

View your CI Pipeline Execution ↗ for commit d4ec962

Command	Status	Duration	Result
`nx run-many -t build --parallel=3`	✅ Succeeded	1m 1s	View ↗

☁️ Nx Cloud last updated this comment at 2025-07-08 09:23:59 UTC

mrginglymus · 2025-07-07T08:51:15Z

code/core/src/common/js-package-manager/Yarn2Proxy.ts

-        const pnpApi = await import(pnpapiPath);
+    try {
+      // @ts-expect-error TS doesn't know about the built-in Yarn PnP API
+      const { default: pnpApi } = await import('pnpapi');


This will break many storybook cli usages, which respawns commands using npx.

I thought that this was the site of the issues I found in automigrations, before I realised you weren't importing the pnpapi module - but it now will be.

ah that's a good point, thank you! I reverted the changes and added a comment explaining this to the next person.

I'd say that the proper fix would be to find all those places where we spawn sub processes of Storybook, and ensure that they do it with the right package manager command, but that's not my concern here.

This feels like a fragile solution, surely loading the pnp loader manually is not officially supported and will break at some point.

I think all of the implementations would be a lot simpler if you could guarantee they were running under the original runtime; you could just then import the package.json directly, or at the very least require.resolve it and read it using native fs (which would be already patched in yarn berry).

True. I don't have the overview of all the places where we spin up a new process, but I know that in the dispatcher code in the core CLI, it's a trade off with performance and size. Essentially by making npm run storybook automigrate run npx @storybook/cli automigrate, it means that the core storybook package only have to contain the workflows that are used daily - dev, build - instead of installing and parsing the workflows that are executed very rarely like upgrade and automigrate. I think that is the right trade off.

I agree to a point - one pain point is it makes debugging a bit more tricky as you have to ensure you bypass all the respawns.

My point is that if we respawn using the correct runtime, then we can drop some of the complexity here without sacrificing those advantages.

I'd be happy to attempt a PR to that end, if you think it's worth it.

My point is that if we respawn using the correct runtime

Can you elaborate on what you mean by this? What would the change be?

Replace this line:

storybook/code/core/src/bin/index.ts

Line 46 in 3ac2fec

command = ['npx', '--yes', `@storybook/cli@${versions.storybook}`, ...args];

with something like

command = JsPackageManagerFactory.getPackageManager({}).getRemoteRunCommand(`@storybook/cli@${versions.storybook}`, args)'

though, now I come to think about it, I'm not sure whether a yarn dlx process would have access to the pnp runtime of the cwd...

ah yes then I agree with you, on both counts. 😅

Feel free to open a PR, but probably use sb10/esm-only as a base, as that has changed quite a bit.

fyi early draft - #31989 some degree of yak shaving happened (:

storybook-pr-benchmarking · 2025-07-07T08:51:25Z

Package Benchmarks

^{Commit: d4ec962, ran on 8 July 2025 at 09:12:53 UTC}

No significant changes detected, all good. 👏

greptile-apps

_{37 files reviewed, 28 comments}
_{Edit PR Review Bot Settings | Greptile}

test-storybooks/yarn-pnp/src/stories/header.css

test-storybooks/yarn-pnp/tsconfig.json

test-storybooks/yarn-pnp/src/App.css

test-storybooks/yarn-pnp/src/main.tsx

test-storybooks/yarn-pnp/src/stories/Configure.mdx

code/core/src/common/presets.ts

test-storybooks/yarn-pnp/src/stories/Configure.mdx

test-storybooks/yarn-pnp/src/stories/Page.tsx

test-storybooks/yarn-pnp/eslint.config.js

JReinhold added 5 commits July 4, 2025 12:03

simplify Yarn PnP API usage

6f5403c

dont convert preview builder paths before importing

d67278b

use yarn pnp compatible resolutions

6f9b0ae

add Yarn PnP test storybook

f19343b

add Yarn PnP test to Circle config

a457a43

JReinhold added bug core ci:normal labels Jul 7, 2025

JReinhold self-assigned this Jul 7, 2025

JReinhold requested a review from Copilot July 7, 2025 08:37

Copilot AI reviewed Jul 7, 2025

View reviewed changes

code/core/src/core-server/utils/get-builders.ts Show resolved Hide resolved

code/core/src/common/presets.ts Outdated Show resolved Hide resolved

mrginglymus reviewed Jul 7, 2025

View reviewed changes

JReinhold added 7 commits July 7, 2025 11:05

revert simplifed yarn pnp api usage.

9590730

upgrade to latest node 22 in ci

b29ae7f

use node orb

3220636

fix playwright versions

7058a6f

remove node version step

8bb19ec

fix safe resolution in resolveAddonName

4759589

add playwright dep to pnp project

d4ec962

JReinhold marked this pull request as ready for review July 8, 2025 10:23

JReinhold requested a review from ndelangen July 8, 2025 10:23

greptile-apps bot reviewed Jul 8, 2025

View reviewed changes

ndelangen approved these changes Jul 8, 2025

View reviewed changes

JReinhold merged commit 4889efd into sb10/esm-only Jul 8, 2025
52 of 55 checks passed

JReinhold deleted the jeppe/fix-yarn-pnp branch July 8, 2025 10:41

Uh oh!

Core: Fix Yarn PnP support #31968

Core: Fix Yarn PnP support #31968

Uh oh!

Conversation

JReinhold commented Jul 7, 2025 • edited by greptile-apps bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What I did

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

Manual testing

Documentation

Checklist for Maintainers

🦋 Canary release

Greptile Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

nx-cloud bot commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrginglymus Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

JReinhold Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

mrginglymus Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

JReinhold Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

mrginglymus Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

JReinhold Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrginglymus Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JReinhold Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

mrginglymus Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

storybook-pr-benchmarking bot commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Package Benchmarks

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JReinhold commented Jul 7, 2025 •

edited by greptile-apps bot

Loading

nx-cloud bot commented Jul 7, 2025 •

edited

Loading

JReinhold Jul 8, 2025 •

edited

Loading

mrginglymus Jul 8, 2025 •

edited

Loading

storybook-pr-benchmarking bot commented Jul 7, 2025 •

edited

Loading