Skip to content

Conversation

@yannbf
Copy link
Member

@yannbf yannbf commented Oct 16, 2025

Closes #32626

What I did

The csf factories syncing mechanism was working locally but not when running npx storybook automigrate csf-factories, because import.meta.resolve doesn't account for the origin path. This PR uses createRequire instead, which respects the configDir as origin path and therefore fixes the detection of addon annotations

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 pull request has been released as version 0.0.0-pr-32741-sha-cc844cd8. 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-32741-sha-cc844cd8
Triggered by @yannbf
Repository storybookjs/storybook
Branch yann/fix-csf-codemod-getting-annotations
Commit cc844cd8
Datetime Thu Oct 16 11:14:26 UTC 2025 (1760613266)
Workflow run 18559393358

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 publish.yml --field pr=32741

Summary by CodeRabbit

  • Bug Fixes
    • Improved addon preview resolution handling to enhance reliability.

@yannbf yannbf requested a review from ndelangen October 16, 2025 10:35
@yannbf yannbf self-assigned this Oct 16, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 16, 2025

📝 Walkthrough

Walkthrough

The addon preview resolution mechanism in a utility function is updated to switch from ESM's import.meta.resolve() to CommonJS's require.resolve() via createRequire, while maintaining the same return structure and API contract.

Changes

Cohort / File(s) Summary
Module resolution migration
code/core/src/common/utils/get-addon-annotations.ts
Replaced ESM-based import.meta.resolve() with CommonJS-based require.resolve() via createRequire(import.meta.url). Added import of createRequire from node:module. Updated resolution call to use { paths: [configDir] } option instead of inline directory parameter.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch yann/fix-csf-codemod-getting-annotations

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
code/core/src/common/utils/get-addon-annotations.ts (1)

42-43: Correctly fixes addon resolution in npx context.

The switch from import.meta.resolve() to createRequire() + require.resolve() with { paths: [configDir] } ensures that addon preview modules are resolved relative to the project's configuration directory. This fixes the issue where addons were not detected when running via npx storybook automigrate csf-factories.

Consider adding a brief comment explaining why createRequire is used instead of import.meta.resolve for future maintainers:

  // If the preview endpoint doesn't exist, we don't need to add the addon to definePreview
  try {
+   // Use createRequire to resolve addon preview relative to configDir,
+   // ensuring correct resolution in npx context
    const require = createRequire(import.meta.url);
    require.resolve(`${addon}/preview`, { paths: [configDir] });
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bbff0e5 and cc844cd.

📒 Files selected for processing (1)
  • code/core/src/common/utils/get-addon-annotations.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (use yarn lint:js:cmd <file>)

Files:

  • code/core/src/common/utils/get-addon-annotations.ts
**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Export functions from modules when they need to be unit-tested

Files:

  • code/core/src/common/utils/get-addon-annotations.ts
code/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

In application code, use Storybook loggers instead of console.* (client code: storybook/internal/client-logger; server code: storybook/internal/node-logger)

Files:

  • code/core/src/common/utils/get-addon-annotations.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

  • code/core/src/common/utils/get-addon-annotations.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: get-parameters
  • GitHub Check: get-branch
  • GitHub Check: Danger JS
  • GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
code/core/src/common/utils/get-addon-annotations.ts (1)

1-1: LGTM!

The import of createRequire from the built-in node:module is necessary for the fix and introduces no external dependencies.

@nx-cloud
Copy link

nx-cloud bot commented Oct 16, 2025

View your CI Pipeline Execution ↗ for commit cc844cd

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

☁️ Nx Cloud last updated this comment at 2025-10-16 10:58:32 UTC

Comment on lines +42 to +43
const require = createRequire(import.meta.url);
require.resolve(`${addon}/preview`, { paths: [configDir] });
Copy link
Member

Choose a reason for hiding this comment

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

We do this, because import.meta.resolve does not natively support the "from path" functionality yet.
It will in the future; but not yet.

@yannbf yannbf merged commit e9c4b29 into next Oct 16, 2025
62 of 67 checks passed
@yannbf yannbf deleted the yann/fix-csf-codemod-getting-annotations branch October 16, 2025 12:59
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]: SB10 CSF Next - migration not updating addons

3 participants