-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: upgrade storybook 8 #4124
base: canary
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
WalkthroughThis pull request introduces several updates primarily focused on upgrading the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
packages/storybook/.storybook/addons/react-strict-mode/index.tsx (1)
7-8
: Consider more robust boolean parsingThe direct string comparison
=== "true"
could be made more robust to handle various truthy values.- const [isStrict, setStrict] = useState(strictQuery === "true"); + const [isStrict, setStrict] = useState(strictQuery?.toLowerCase() === "true");packages/storybook/.storybook/main.ts (3)
19-28
: Consider adding configuration documentationThe new @storybook/addon-docs configuration with remarkGfm is well-structured, but it would be helpful to add a comment explaining the purpose of these specific options and why remarkGfm is needed.
+ // Enable GitHub Flavored Markdown support in documentation { name: '@storybook/addon-docs', options: {
33-35
: Consider removing empty options objectIf there are no specific framework options needed, you could remove the empty options object to keep the configuration concise.
45-47
: Consider documentation strategy with autodocsEnabling
autodocs: true
will automatically generate documentation for all stories. This can significantly increase the documentation surface area. Consider:
- The impact on build time and bundle size
- Whether all components should have auto-generated docs
- Setting up documentation standards for consistency
packages/storybook/.storybook/welcome.mdx (1)
16-18
: Consider semantic HTML structureWhile the change from
<p>
to<div>
works, consider keeping the<p>
tag for better semantic meaning since this is a paragraph of text. The styling can still be achieved with the same Tailwind classes.-<div className="!text-base !text-foreground max-w-xl [&>p]:text-foreground [&>p]:text-base"> +<p className="!text-base !text-foreground max-w-xl"> Here you can find the guidelines, components APIs and examples to help you build your next project with NextUI. -</div> +</p>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
package.json
(1 hunks)packages/storybook/.storybook/addons/react-strict-mode/index.tsx
(1 hunks)packages/storybook/.storybook/main.ts
(1 hunks)packages/storybook/.storybook/welcome.mdx
(2 hunks)packages/storybook/package.json
(1 hunks)packages/storybook/tailwind.config.js
(1 hunks)
🔇 Additional comments (12)
packages/storybook/.storybook/addons/react-strict-mode/index.tsx (3)
1-4
: LGTM! Clean and well-organized imports
The imports are properly structured with specific named imports and appropriate type imports.
10-18
: LGTM! Clean effect implementation
The effect properly sets up and cleans up the channel subscription.
22-29
: LGTM! Well-structured decorator implementation
The decorator follows Storybook's best practices for decorator implementation.
packages/storybook/.storybook/main.ts (3)
2-5
: LGTM! Good type safety improvements
The addition of proper TypeScript types and necessary imports enhances code reliability and maintainability.
6-10
: Verify the welcome.mdx file path
The path has been updated from welcome.stories.mdx
to welcome.mdx
. Please ensure that this file exists at the new location and that all story paths are correctly configured.
54-54
: LGTM! Good modernization
The switch to ES module syntax with export default
aligns with modern JavaScript practices.
packages/storybook/package.json (3)
40-50
: Major version upgrade requires migration steps
This is a major version upgrade from Storybook 7 to 8, which likely includes breaking changes. Please document the migration steps and any breaking changes that NextUI users need to be aware of.
Consider:
- Adding migration notes to the PR description
- Documenting any breaking changes
- Testing all existing stories for compatibility
52-53
: Verify dark mode compatibility
The upgrade of storybook-dark-mode
from 3.x to 4.x may require changes to the dark mode configuration. Please ensure that the dark mode functionality continues to work as expected.
54-54
: Document the purpose of remark-gfm
The addition of remark-gfm
suggests enhanced markdown support. This is likely required for MDX documentation in Storybook 8, but it would be helpful to document why this dependency was added.
packages/storybook/tailwind.config.js (1)
6-6
: LGTM! Please verify the welcome.mdx file path.
The path update aligns with Storybook v8 conventions by simplifying the extension from .stories.mdx
to .mdx
. Please ensure that the file exists at ./.storybook/welcome.mdx
to prevent any build failures.
packages/storybook/.storybook/welcome.mdx (1)
129-130
: LGTM! JSX compliance improvement
Good update to change datetime
to dateTime
for proper JSX attribute naming convention. The HTML structure and Tailwind classes look appropriate for this timestamp section.
package.json (1)
74-74
: Document Storybook v8 migration steps
Upgrading from Storybook 7 to 8 is a major version change that includes breaking changes. Please:
- Document the migration steps in the PR description
- List any breaking changes that affect the NextUI project
- Provide guidance for NextUI users who need to upgrade
Still have some hot reload issues |
📝 Description
Currently, there are many issues and incorrect usages with version 7 of Storybook.
Some typical problems include:
To resolve the issues mentioned above we have upgraded to the latest Storybook 8.
The main changes include:
main.js
tomain.ts
for better type hinting.xxx.storybook.mdx
toxxx.mdx
.Summary by CodeRabbit
New Features
remark-gfm
to enhance documentation capabilities.Bug Fixes
Documentation
Chores