Skip to content

Windows: Fix runtime and build-time errors#32116

Merged
JReinhold merged 12 commits into
sb10/esm-onlyfrom
jeppe/fix-windows-runtime
Jul 24, 2025
Merged

Windows: Fix runtime and build-time errors#32116
JReinhold merged 12 commits into
sb10/esm-onlyfrom
jeppe/fix-windows-runtime

Conversation

@JReinhold
Copy link
Copy Markdown
Contributor

@JReinhold JReinhold commented Jul 24, 2025

Works on #31787

Fixes #32037

What I did

I QA'ed both a regular Storybook and the monorepo on Windows, and fixed some things:

  1. Fixed absolute path imports to always use a File URL, as required by Node on Windows
  2. Fixed a lot of manually built paths that are passed when spawning child processes. If these paths contained spaces (eg. C:\Jeppe Reinhold\dev\storybook), they need to be wrapped in double-quotes
  3. Fixed Core: Remove CJS bundles, only ship ESM #31819 (comment)

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

This PR addresses critical Windows compatibility issues in Storybook's transition to ESM-only architecture. The changes fix two primary categories of problems that prevent Storybook from functioning correctly on Windows systems:

1. ESM Dynamic Import Path Resolution: Multiple files were updated to use pathToFileURL() from Node.js's url module when dynamically importing files with absolute paths. This is required because Node.js on Windows mandates that absolute paths in ESM dynamic imports must be converted to file URLs (e.g., converting C:\path\file.js to file:///C:/path/file.js). Files affected include telemetry modules, package managers (Yarn2Proxy, PNPMProxy), CLI utilities, and build scripts.

2. Shell Command Path Quoting: Several files were modified to wrap file paths in double quotes when constructing shell commands for child processes. This prevents command execution failures when paths contain spaces, which is common on Windows (e.g., C:\Jeppe Reinhold\dev\storybook). Changes include CLI utilities, registry scripts, and sandbox management tools.

Additional Improvements: The PR includes a fallback mechanism in sandbox creation that copies files when symlink operations fail (common on Windows without admin privileges), and updates some import statements to be more ESM-compliant.

These changes are essential for Storybook 10's ESM-only migration to work seamlessly across all platforms, particularly addressing the unique path handling requirements and filesystem limitations of Windows environments.

Confidence score: 3/5

  • This PR contains mostly safe cross-platform compatibility fixes but has one critical issue that will cause runtime failures.
  • The confidence score is lowered due to a serious bug in code/frameworks/angular/src/server/angular-cli-webpack.js where isAbsolute() is used on file URLs instead of file paths, which will always return false and break Tailwind 4 detection.
  • Files needing attention: code/frameworks/angular/src/server/angular-cli-webpack.js requires immediate fix for the isAbsolute() usage with file URLs.

@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Jul 24, 2025

View your CI Pipeline Execution ↗ for commit 4a09205

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

☁️ Nx Cloud last updated this comment at 2025-07-24 11:51:47 UTC

@JReinhold JReinhold self-assigned this Jul 24, 2025
@JReinhold JReinhold added bug core ci:normal Run our default set of CI jobs (choose this for most PRs). labels Jul 24, 2025
@JReinhold JReinhold marked this pull request as ready for review July 24, 2025 09:14
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

15 files reviewed, 1 comment

Edit Code Review Bot Settings | Greptile

Comment thread scripts/knip.config.ts Outdated
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@JReinhold JReinhold merged commit 19dfdec into sb10/esm-only Jul 24, 2025
49 of 53 checks passed
@JReinhold JReinhold deleted the jeppe/fix-windows-runtime branch July 24, 2025 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug ci:normal Run our default set of CI jobs (choose this for most PRs). core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants