CLI: Add automigrate check for 'storybook' package name conflict#34290
Conversation
When a project's package.json name is 'storybook', it shadows the real storybook package in node_modules within npm/pnpm/yarn workspaces, causing 'Cannot find module storybook/internal/...' errors. This adds a notification-type automigrate fix that warns users and suggests renaming their package. Related to storybookjs#28725 Made-with: Cursor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds detection and notifications for projects whose package.json "name" is exactly "storybook": a new automigration fix is introduced and registered, and the Create Storybook preflight now warns during setup when this conflict is detected. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Preflight as PreflightCheckCommand
participant PM as JsPackageManager
participant Logger
participant Automigrate as storybookPackageNameConflict
User->>Preflight: run create-storybook
Preflight->>PM: initialize package manager
PM-->>Preflight: primaryPackageJson (contains name)
Preflight->>Preflight: checkPackageNameConflict(name)
alt name == "storybook"
Preflight->>Logger: warn about package name conflict
Logger-->>User: display warning
end
User->>Automigrate: run automigrate check
Automigrate->>PM: read primaryPackageJson.name
alt name == "storybook"
Automigrate-->>User: return fix { packageName: "storybook" }
User->>Automigrate: prompt shown (notification)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/configure/integration/eslint-plugin.mdx (1)
43-46: Optional consistency pass in.eslintrcsnippet.Most snippets now use double quotes, but the
.eslintrcexample still has single-quoted strings. Consider normalizing that block too for consistency.Also applies to: 95-100, 108-115, 123-135
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/configure/integration/eslint-plugin.mdx` around lines 43 - 46, The snippet uses single quotes in the `.eslintrc` example while other snippets use double quotes; update the string literals to double quotes for consistency (e.g., in the defineConfig/globalIgnores example and the `.eslintrc` blocks referenced around lines 95-100, 108-115, 123-135) so all examples use double-quoted strings consistently; check and replace any remaining single-quoted keys/values in those code blocks (including occurrences inside defineConfig, globalIgnores, and the .eslintrc example) to double quotes.code/lib/eslint-plugin/docs/rules/no-stories-of.md (1)
16-20: Optional: align quote style in the “correct” snippets too.You updated the incorrect example to double quotes; consider doing the same in the correct examples for full-file consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/eslint-plugin/docs/rules/no-stories-of.md` around lines 16 - 20, The "correct" example snippets should use double quotes to match the updated incorrect example; update string literals in the examples (the import path string and the storiesOf arguments) to use double quotes consistently — specifically adjust occurrences tied to the import of Button, the storiesOf call, and the "primary" story label so all quote usage is double quotes for full-file consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@code/lib/eslint-plugin/docs/rules/no-stories-of.md`:
- Around line 16-20: The "correct" example snippets should use double quotes to
match the updated incorrect example; update string literals in the examples (the
import path string and the storiesOf arguments) to use double quotes
consistently — specifically adjust occurrences tied to the import of Button, the
storiesOf call, and the "primary" story label so all quote usage is double
quotes for full-file consistency.
In `@docs/configure/integration/eslint-plugin.mdx`:
- Around line 43-46: The snippet uses single quotes in the `.eslintrc` example
while other snippets use double quotes; update the string literals to double
quotes for consistency (e.g., in the defineConfig/globalIgnores example and the
`.eslintrc` blocks referenced around lines 95-100, 108-115, 123-135) so all
examples use double-quoted strings consistently; check and replace any remaining
single-quoted keys/values in those code blocks (including occurrences inside
defineConfig, globalIgnores, and the .eslintrc example) to double quotes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dbbea48a-816c-4f60-a127-6b3c65ff4a1e
📒 Files selected for processing (18)
code/lib/cli-storybook/src/automigrate/fixes/index.tscode/lib/cli-storybook/src/automigrate/fixes/storybook-package-name-conflict.test.tscode/lib/cli-storybook/src/automigrate/fixes/storybook-package-name-conflict.tscode/lib/eslint-plugin/docs/rules/await-interactions.mdcode/lib/eslint-plugin/docs/rules/csf-component.mdcode/lib/eslint-plugin/docs/rules/default-exports.mdcode/lib/eslint-plugin/docs/rules/hierarchy-separator.mdcode/lib/eslint-plugin/docs/rules/meta-inline-properties.mdcode/lib/eslint-plugin/docs/rules/meta-satisfies-type.mdcode/lib/eslint-plugin/docs/rules/no-redundant-story-name.mdcode/lib/eslint-plugin/docs/rules/no-renderer-packages.mdcode/lib/eslint-plugin/docs/rules/no-stories-of.mdcode/lib/eslint-plugin/docs/rules/no-title-property-in-meta.mdcode/lib/eslint-plugin/docs/rules/no-uninstalled-addons.mdcode/lib/eslint-plugin/docs/rules/story-exports.mdcode/lib/eslint-plugin/docs/rules/use-storybook-expect.mdcode/lib/eslint-plugin/docs/rules/use-storybook-testing-library.mddocs/configure/integration/eslint-plugin.mdx
Update .eslintrc and no-stories-of rule documentation to use double quotes consistently across all code snippets. Made-with: Cursor
whdjh
force-pushed
the
fix/28725-storybook-package-name-check
branch
from
c5f438e to
aac7aff
Compare
valentinpalkovic
moved this from Empathy Queue (prioritized)
to On Hold
in Core Team Projects
|
Hi @whdjh, Thank you for your contribution! It is valuable to have an automated migration in place that notifies users of the problem when they attempt to upgrade via the upgrade CLI. What are your thoughts on covering new users who initialize Storybook in a fresh project and therefore don't run the automigration? |
|
"That's a great point. I focused on the upgrade path for this PR since it addresses #28725. Should I open a separate issue/PR for the init case, or would you prefer I extend this one?" |
|
I would prefer to extend this one instead! |
|
Sounds good! I'll extend the check to the |
|
Hi, @valentinpalkovic Changes:
This covers both paths now:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/lib/create-storybook/src/commands/PreflightCheckCommand.test.ts`:
- Around line 116-129: The test uses direct access to logger.warn in assertions
which is not type-safe; update the two assertions to use vi.mocked(logger.warn)
so they follow the project's Vitest mocking convention and allow proper mock
typing — locate the expectations in PreflightCheckCommand.test.ts that reference
logger.warn (around the tests for package.json name) and replace those
expect(logger.warn)... and expect(logger.warn).not... calls with
expect(vi.mocked(logger.warn))... while keeping the same expect.stringContaining
checks; no other behavior changes are needed (other related mocks like
scaffoldModule.currentDirectoryIsEmpty and mockPackageManager.primaryPackageJson
remain as-is).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5b3a339f-a4c3-45f5-bc8b-a0d49de15c33
📒 Files selected for processing (2)
code/lib/create-storybook/src/commands/PreflightCheckCommand.test.tscode/lib/create-storybook/src/commands/PreflightCheckCommand.ts
code/lib/create-storybook/src/commands/PreflightCheckCommand.test.ts
Outdated
Show resolved
Hide resolved
|
It's about time someone fixed this, good work @whdjh !! 🥳🥳 |
|
Thanks! 😀 |
|
I've verified the upgrade and init flow. LGTM! |
github-project-automation
bot
moved this from In Progress
to Done
in Core Team Projects
4 participants
When a project's package.json name is 'storybook', it shadows the real
storybook package in node_modules within npm/pnpm/yarn workspaces,
causing 'Cannot find module storybook/internal/...' errors.
This adds a notification-type automigrate fix that warns users and
suggests renaming their package.
Related to #28725
Closes #28725
What I did
Added a new
notification-type automigrate fix (storybookPackageNameConflict) that detects when a project'spackage.json"name"field is set to"storybook".In npm/pnpm/yarn workspaces, this creates a symlink at
node_modules/storybookthat shadows the real Storybook package, causingCannot find module 'storybook/internal/...'errors. This was identified by @daaashley in #28725 and @shilman suggested adding a CLI check.The fix warns the user and suggests renaming their package, but does not make automatic changes.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
No manual testing is strictly necessary — the check reads the
namefield frompackage.jsonviapackageManager.primaryPackageJsonand returns a notification prompt. The logic is fully covered by unit tests (4 test cases: name is"storybook", name is something else, name is undefined, and prompt content verification).Documentation
MIGRATION.MD
N/A — this is a CLI diagnostic check, no user-facing docs needed.
Summary by CodeRabbit
New Features
Tests
Documentation