-
-
Notifications
You must be signed in to change notification settings - Fork 198
upgrade-storybook-v8 #904
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
base: develop
Are you sure you want to change the base?
upgrade-storybook-v8 #904
Conversation
β Deploy Preview for livecodes failed.
|
WalkthroughStorybook upgraded from version 6.5.x to 8.6.x with configuration restructuring. The old main.js file was deleted, preview.js configuration was simplified, a new storybook.config.js was added, and package.json scripts and dependencies were updated to use newer Storybook CLI commands and compatible addon versions. Changes
Estimated code review effortπ― 4 (Complex) | β±οΈ ~45 minutes Multiple heterogeneous changes across configuration files and significant dependency version upgrades (6.5.x β 8.6.x) requiring verification of compatibility, testing environment validation, and confirmation that simplified configuration maintains required functionality. Poem
Pre-merge checks and finishing touchesβ Passed checks (3 passed)
β¨ Finishing touches
π§ͺ Generate unit tests (beta)
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 |
|
upgrade storybook v8 |
|
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: 3
π§Ή Nitpick comments (2)
storybook/.storybook/storybook.config.js (1)
5-5: Minor: Replace Turkish comment with English for consistency.The comment contains "ya da" (Turkish for "or"), which is inconsistent with the English codebase.
Apply this diff:
- name: '@storybook/react-vite', // ya da '@storybook/react-webpack5' + name: '@storybook/react-vite', // or '@storybook/react-webpack5'storybook/package.json (1)
14-29: Consider consistent version format for Storybook packages.Some Storybook packages use exact versions (e.g.,
"8.6.14"on lines 15-20, 23) while others use caret ranges (e.g.,"^8.6.14"on lines 21, 29). For consistency and clearer dependency management, consider using the same versioning approach for all Storybook packages.Consider applying this diff for consistency:
- "@storybook/addon-actions": "8.6.14", - "@storybook/addon-essentials": "8.6.14", - "@storybook/addon-interactions": "8.6.14", - "@storybook/addon-links": "8.6.14", - "@storybook/addon-storysource": "8.6.14", - "@storybook/html": "8.6.14", + "@storybook/addon-actions": "^8.6.14", + "@storybook/addon-essentials": "^8.6.14", + "@storybook/addon-interactions": "^8.6.14", + "@storybook/addon-links": "^8.6.14", + "@storybook/addon-storysource": "^8.6.14", + "@storybook/html": "^8.6.14", "@storybook/html-webpack5": "^8.6.14", "@storybook/test": "^8.6.14", - "@storybook/theming": "8.6.14", + "@storybook/theming": "^8.6.14",
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (4)
storybook/.storybook/main.js(0 hunks)storybook/.storybook/preview.js(1 hunks)storybook/.storybook/storybook.config.js(1 hunks)storybook/package.json(1 hunks)
π€ Files with no reviewable changes (1)
- storybook/.storybook/main.js
β° 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). (1)
- GitHub Check: Codacy Static Code Analysis
π Additional comments (3)
storybook/.storybook/preview.js (1)
2-3: LGTM! Configuration appropriately simplified for Storybook v8.The streamlined parameters configuration aligns well with Storybook v8's best practices.
storybook/package.json (1)
6-7: LGTM! Scripts correctly updated for Storybook v8 CLI.The migration from
start-storybook/build-storybooktostorybook dev/storybook buildaligns with Storybook v8's unified CLI approach.storybook/.storybook/storybook.config.js (1)
4-7: The review comment is based on a false premise and should be disregarded.The original review claims that
package.jsonincludes@storybook/htmland@storybook/html-webpack5dependencies. This is incorrectβthere are no@storybookpackages inpackage.jsonat all, neither independenciesnordevDependencies.While the code does reference
@storybook/htmltypes and the config specifies@storybook/react-vite, the framework mismatch concern is predicated on the non-existent packages. The absence of these dependencies inpackage.jsonis a separate concern that requires its own investigation, not a framework configuration mismatch issue.Likely an incorrect or invalid review comment.
| export default { | ||
| stories: ['../src/**/*.stories.@(js|jsx|ts|tsx|mdx)'], | ||
| addons: ['@storybook/addon-links', '@storybook/addon-essentials'], | ||
| framework: { | ||
| name: '@storybook/react-vite', // ya da '@storybook/react-webpack5' | ||
| options: {}, | ||
| }, | ||
| }; |
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.
π§© Analysis chain
Critical: Verify that Storybook v8 recognizes storybook.config.js as the configuration file.
Storybook v8 conventionally expects the main configuration file to be named main.js (or main.ts) within the .storybook directory. Using storybook.config.js as the filename is non-standard and may cause Storybook to fail to load the configuration.
Please run the following verification to confirm whether Storybook properly loads this configuration:
If this custom filename doesn't work, consider renaming storybook.config.js to main.js:
-// storybook/.storybook/storybook.config.js
+// storybook/.storybook/main.jsπ Script executed:
#!/bin/bash
# Verify if storybook.config.js is recognized by checking Storybook v8 documentation
# and testing if the dev server starts correctly
cd storybook
# Check if main.js exists (it should contain this config instead)
if [ -f .storybook/main.js ]; then
echo "=== main.js exists ==="
cat .storybook/main.js
else
echo "=== main.js does not exist ==="
fi
# Check Storybook documentation for config file naming
echo "=== Checking if storybook.config.js is a valid config filename ==="
fd -t f "main\.(js|ts|mjs|cjs)" .storybookLength of output: 97
Rename storybook.config.js to main.js.
Storybook's main configuration file must be named main.js (or main.ts) and be located in the .storybook folder. Storybook needs a "main.js" file, and using a custom configuration file name is not supportedβthe -c flag is for specifying a custom config directory, not a custom config filename.
Rename storybook/.storybook/storybook.config.js to storybook/.storybook/main.js.
π€ Prompt for AI Agents
In storybook/.storybook/storybook.config.js lines 1-8, Storybook requires its
main configuration file to be named main.js (or main.ts); rename
storybook/.storybook/storybook.config.js to storybook/.storybook/main.js and
leave the exported config as-is, then update any project scripts or references
that pointed to the old filename so they now point to .storybook/main.js.
| "@storybook/html": "8.6.14", | ||
| "@storybook/html-webpack5": "^8.6.14", |
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.
Framework dependencies don't match the configured framework.
The package.json includes @storybook/html and @storybook/html-webpack5, but storybook.config.js specifies @storybook/react-vite as the framework. If you're building a React project with Vite, these HTML and webpack5 dependencies are unnecessary and may cause conflicts.
Ensure the dependencies align with your actual framework choice. If using React with Vite, consider removing these lines:
- "@storybook/html": "8.6.14",
- "@storybook/html-webpack5": "^8.6.14",Alternatively, if you need HTML or webpack5, update the framework configuration in storybook.config.js accordingly.
π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "@storybook/html": "8.6.14", | |
| "@storybook/html-webpack5": "^8.6.14", |
π€ Prompt for AI Agents
In storybook/package.json around lines 20-21, the dependencies list includes
@storybook/html and @storybook/html-webpack5 which conflict with the configured
framework @storybook/react-vite in storybook.config.js; remove those two
HTML/webpack5 packages from package.json if the project uses React with Vite,
then run a clean install (or update lockfile) so only the React/Vite Storybook
packages remain, or alternatively change the framework in storybook.config.js to
an HTML/webpack5-based framework if you actually need those packages.
| "js-base64": "^3.7.8", | ||
| "lz-string": "^1.5.0" |
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.
π§© Analysis chain
Verify that the new runtime dependencies are required.
js-base64 and lz-string were added as runtime dependencies, but the PR description doesn't explain their purpose. Ensure these packages are actually used in the codebase to avoid unnecessary dependencies.
Run the following script to check usage:
π Script executed:
#!/bin/bash
# Check if the new dependencies are imported/used in the codebase
echo "=== Checking usage of js-base64 ==="
rg -t js -t ts "from ['\"]js-base64['\"]|require\(['\"]js-base64['\"]" storybook/
echo ""
echo "=== Checking usage of lz-string ==="
rg -t js -t ts "from ['\"]lz-string['\"]|require\(['\"]lz-string['\"]" storybook/Length of output: 72
π Script executed:
#!/bin/bash
# Broader verification for js-base64 and lz-string usage
echo "=== Checking for any mention of js-base64 in files ==="
rg "js-base64" storybook/ --type-list
echo ""
echo "=== Checking for any mention of lz-string in files ==="
rg "lz-string" storybook/ --type-list
echo ""
echo "=== Listing storybook package.json to understand context ==="
cat storybook/package.json | grep -A 50 "dependencies"
echo ""
echo "=== Checking if there are any story files that might use encoding ==="
find storybook -name "*.stories.*" -o -name "*.story.*" | head -20
echo ""
echo "=== Checking for base64 or compression patterns in JS/TS files ==="
rg -t js -t ts "(base64|btoa|atob|LZString|compress|decompress)" storybook/ --max-count=5Length of output: 1640
π Script executed:
#!/bin/bash
# Corrected search for js-base64 and lz-string usage
echo "=== Direct string search for js-base64 in all files ==="
rg "js-base64" storybook/
echo ""
echo "=== Direct string search for lz-string in all files ==="
rg "lz-string" storybook/
echo ""
echo "=== Check if they're mentioned in any documentation or comments ==="
rg "(Base64|LZString)" storybook/
echo ""
echo "=== Check storybook configuration files ==="
find storybook -name ".storybook" -type d -exec find {} -type f \; | head -20
echo ""
echo "=== List all files in storybook .storybook directory if it exists ==="
ls -la storybook/.storybook/ 2>/dev/null || echo "No .storybook directory found"
echo ""
echo "=== Check if dependencies appear in main/preview config ==="
cat storybook/.storybook/main.ts 2>/dev/null || cat storybook/.storybook/main.js 2>/dev/null || echo "main config not found"Length of output: 1383
π Script executed:
#!/bin/bash
# Final comprehensive check
echo "=== Content of storybook config files ==="
echo "--- manager.js ---"
cat storybook/.storybook/manager.js
echo ""
echo "--- preview.js ---"
cat storybook/.storybook/preview.js
echo ""
echo "--- storybook.config.js ---"
cat storybook/.storybook/storybook.config.js
echo ""
echo "=== Search ALL TypeScript and JavaScript files for imports of these packages ==="
echo "Looking for imports of js-base64 or lz-string..."
find storybook -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" -o -name "*.jsx" \) ! -path "*/node_modules/*" -exec grep -l "js-base64\|lz-string" {} \;
echo ""
echo "=== Verify: check package.json to see if these are intended as peerDependencies or for external use ==="
cat storybook/package.json
echo ""
echo "=== Check if js-base64 appears in lock file as well (to see if it's a transitive dep or direct) ==="
grep -c "js-base64" storybook/package-lock.json || echo "Not found in lock file"Length of output: 2259
Remove unused runtime dependencies from storybook/package.json.
Verification confirms that js-base64 and lz-string are not imported or used anywhere in the storybook codebaseβnot in configuration files (manager.js, preview.js, storybook.config.js) or any source files. These dependencies should be removed from lines 35-36 unless they serve a documented purpose.
π€ Prompt for AI Agents
In storybook/package.json around lines 35-36, the deps "js-base64" and
"lz-string" are unused and should be removed; edit package.json to delete those
two entries, run your package manager (npm install or yarn install) to update
the lockfile, rebuild/test Storybook (npm run build or yarn build) to ensure
nothing breaks, and commit the updated package.json and lockfile changes with a
short message like "chore(storybook): remove unused deps js-base64 and
lz-string".
|
Thank you @FirassHossreh |



What type of PR is this? (check all applicable)
Description
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings
Added tests?
Added to documentations?
[optional] Are there any post-deployment tasks we need to perform?
[optional] What gif best describes this PR or how it makes you feel?
Summary by CodeRabbit