Skip to content

Core: resolve builder preset path correctly in pnpm strict mode#33750

Closed
braedenfoster wants to merge 5 commits into
storybookjs:nextfrom
braedenfoster:fix/pnpm-preset-resolution
Closed

Core: resolve builder preset path correctly in pnpm strict mode#33750
braedenfoster wants to merge 5 commits into
storybookjs:nextfrom
braedenfoster:fix/pnpm-preset-resolution

Conversation

@braedenfoster
Copy link
Copy Markdown

@braedenfoster braedenfoster commented Feb 2, 2026

What I did

Fixed builder preset path resolution in pnpm monorepos with strict module resolution (shamefully-hoist=false).

Problem

The current code uses dirname(builderName) to construct the path to the builder's preset file. When builderName is @storybook/builder-vite, dirname() returns @storybook, which is not a valid package path and causes ERR_MODULE_NOT_FOUND errors in pnpm's strict resolution mode.

Solution

Changed dirname(builderName) to resolvePackageDir(builderName) on line 66 of code/core/src/core-server/load.ts. This properly resolves the full package directory path, allowing the preset.js file to be found.

The resolvePackageDir function is already imported and used elsewhere in the same file (line 73), so this change maintains consistency with existing code patterns.

Testing

Closes #33748

Summary by CodeRabbit

  • Refactor
    • Improved how builder preset files are located and resolved, enhancing reliability when determining preset locations during project setup.

Use resolvePackageDir instead of dirname when resolving builder preset paths.
This fixes preset resolution in pnpm monorepos with shamefully-hoist=false.

The issue occurs because dirname('@storybook/builder-vite') returns '@storybook',
which is not a valid package path. Using resolvePackageDir properly resolves
the full package directory path, allowing the preset.js file to be found.

Fixes storybookjs#33748
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 2, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 080d3e0 and 4364064.

📒 Files selected for processing (1)
  • code/core/src/core-server/load.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • code/core/src/core-server/load.ts

📝 Walkthrough

Walkthrough

Replaced use of dirname(builderName) with resolvePackageDir(builderName) when computing the absolute path to a builder's preset.js in the core server loader.

Changes

Cohort / File(s) Summary
Builder Preset Path Resolution
code/core/src/core-server/load.ts
Changed first-pass builder preset resolution to call resolvePackageDir(builderName) and then join 'preset.js', instead of using dirname(builderName) to compute the preset path.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@valentinpalkovic valentinpalkovic changed the title fix(core): resolve builder preset path correctly in pnpm strict mode Core: resolve builder preset path correctly in pnpm strict mode Feb 4, 2026
@valentinpalkovic valentinpalkovic moved this to In Progress in Core Team Projects Feb 4, 2026
@valentinpalkovic valentinpalkovic self-assigned this Feb 4, 2026
@storybook-app-bot
Copy link
Copy Markdown

storybook-app-bot Bot commented Feb 4, 2026

Package Benchmarks

Commit: 4364064, ran on 24 February 2026 at 10:26:27 UTC

No significant changes detected, all good. 👏

@valentinpalkovic
Copy link
Copy Markdown
Contributor

Hi @braedenfoster

Thank you for your contribution!

Some of our mocking-related tests seem to fail due to that change. Would you mind taking a look?

@braedenfoster
Copy link
Copy Markdown
Author

Hi @braedenfoster

Thank you for your contribution!

Some of our mocking-related tests seem to fail due to that change. Would you mind taking a look?

Sure, I'll take a look 👍

@valentinpalkovic valentinpalkovic moved this from In Progress to On Hold in Core Team Projects Feb 6, 2026
@valentinpalkovic
Copy link
Copy Markdown
Contributor

@braedenfoster Let me know if you need any help.

@github-actions github-actions Bot added the Stale label Feb 24, 2026
@valentinpalkovic
Copy link
Copy Markdown
Contributor

Closing due to inactivity. Just open a new PR if you plan to bring it over the finish line :)

@github-project-automation github-project-automation Bot moved this from On Hold to Done in Core Team Projects Mar 2, 2026
@braedenfoster
Copy link
Copy Markdown
Author

Closing due to inactivity. Just open a new PR if you plan to bring it over the finish line :)

Cheers. My apologies, I am struggling to get the time to work on this. For now, I have a local patch that fixes this issue for me. I'll aim to open a new PR when I have some time if it's still unresolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Bug]: Preset resolution uses dirname() on package name instead of resolving path, breaks pnpm monorepos

3 participants