Skip to content

Conversation

@JReinhold
Copy link
Contributor

@JReinhold JReinhold commented May 23, 2025

Closes storybookjs/addon-kit#79
🤞 Unblocks igrlk/storybook-addon-test-codegen#29

What I did

In Storybook 9 prereleases, we would always load any found preview.cjs files as preview annotations, even though that is not valid to pass to the browser. There were multiple reasons for this:

  1. We've always done this, at least since 8.6, but probably even before that. The reason for this, is that the lookup mechanism for annotations uses require.resolve() from a CJS file, and Node.js will prefer to return CJS files for that if it finds them. We rarely/never ran into that problem internally, because because the core addons are not "type": "module", they don't have .cjs files but rather .mjs files which we automatically prefer. But addon-kit and derivatives are "type:" "module", they distribute both preview.js and preview.cjs to be compatible with portable stories in Jest. In those situations, we would wrongly prefer the preview.cjs files.
  2. This wasn't a problem until recently, because the resolution mechanism would maintain the package's bare name - eg. storybook-addon-kit/preview, and the Vite builder would use that. However in Vite: Improve handling of preview annotations #28798 we changed that, so that Vite only uses the bare name if the absolute name is empty. But in most cases with addons, Storybook passes both a bare and an absolute path, and so in that situation Vite would now prefer the absolute path to the preview.cjs file which was invalid.

To fix this, we're now improving the "prefer MJS" logic in preset loading, so that it not only prefers .mjs files over .js files, but also .js files over .cjs files. So now, if there is both preview.cjs and preview.js, it will prefer preview.js.

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

We need to test this out in:

  • monorepos, especially those that use the getAbsolutePath helper
  • webpack

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 pull request has been released as version 0.0.0-pr-31556-sha-5f0bee85. Try it out in a new sandbox by running npx [email protected] sandbox or in an existing project with npx [email protected] upgrade.

More information
Published version 0.0.0-pr-31556-sha-5f0bee85
Triggered by @JReinhold
Repository storybookjs/storybook
Branch jeppe/fix-preview-cjs-resolution
Commit 5f0bee85
Datetime Fri May 23 13:23:30 UTC 2025 (1748006610)
Workflow run 15211362158

To request a new release of this pull request, mention the @storybookjs/core team.

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

Copy link
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.

LGTM

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

@nx-cloud
Copy link

nx-cloud bot commented May 23, 2025

View your CI Pipeline Execution ↗ for commit 136c346.

Command Status Duration Result
nx run-many -t check -c production --parallel=7 ✅ Succeeded 1s View ↗
nx run-many -t build -c production --parallel=3 ✅ Succeeded 2s View ↗

☁️ Nx Cloud last updated this comment at 2025-05-26 12:48:04 UTC

@igrlk
Copy link

igrlk commented May 23, 2025

Just tested storybook-addon-test-codegen@next with 0.0.0-pr-31556-sha-5f0bee85 and everything seems to work as expected! Thank you for making the fix, @JReinhold 🔥

@JReinhold JReinhold merged commit a17451e into next May 26, 2025
25 of 31 checks passed
@JReinhold JReinhold deleted the jeppe/fix-preview-cjs-resolution branch May 26, 2025 14:03
@github-actions github-actions bot mentioned this pull request May 26, 2025
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Storybook 9 - Uncaught ReferenceError: require is not defined

4 participants