-
Notifications
You must be signed in to change notification settings - Fork 2k
Update to Storybook v8 #101924
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
Update to Storybook v8 #101924
Conversation
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
|
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~2018 bytes removed 📉 [gzipped]) DetailsCommon code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~17793 bytes removed 📉 [gzipped]) DetailsSections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Async-loaded Components (~13901 bytes removed 📉 [gzipped]) DetailsReact components that are loaded lazily, when a certain part of UI is displayed for the first time. Moment.js Locales (~115 bytes added 📈 [gzipped]) DetailsLocale data for moment.js. Unless you are upgrading the moment.js library, changes in these chunks are suspicious. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
| "@automattic/calypso-build": "workspace:^", | ||
| "@automattic/calypso-color-schemes": "workspace:^", | ||
| "@automattic/calypso-typescript-config": "workspace:^", | ||
| "@swc/core": "^1.11.13", |
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.
Not sure why exactly, but seems like we need this explicit dependency now.
| "devDependencies": { | ||
| "@automattic/calypso-eslint-overrides": "workspace:^", | ||
| "@sentry/webpack-plugin": "^1.21.0", | ||
| "@storybook/addon-actions": "^7.6.20", |
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.
Removing some of these explicit dependencies, as they are already included through the @automattic/calypso-storybook config.
(Same with the other package.jsons)
| "react-router": "^6.23.1", | ||
| "react-test-renderer": "^18.3.1", | ||
| "redux-mock-store": "^1.5.5", | ||
| "storybook": "^8.6.9", |
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.
We now need this as an explicit dependency.
(Same with the other package.jsons)
| "@storybook/addon-actions": "^8.6.9", | ||
| "@storybook/addon-controls": "^8.6.9", | ||
| "@storybook/addon-docs": "^8.6.9", | ||
| "@storybook/addon-themes": "^8.6.9", | ||
| "@storybook/addon-toolbars": "^8.6.9", | ||
| "@storybook/addon-viewport": "^8.6.9", | ||
| "@storybook/addon-webpack5-compiler-babel": "^3.0.5", | ||
| "@storybook/react-webpack5": "^8.6.9", | ||
| "@storybook/test": "^8.6.9", |
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.
Just consolidating all these to the devDependencies.
| '@storybook/addon-toolbars', | ||
| '@storybook/addon-viewport', | ||
| '@storybook/addon-themes', | ||
| '@storybook/addon-webpack5-compiler-babel', |
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.
Newly required for setup.
|
This PR modifies the release build for the following Calypso Apps: For info about this notification, see here: PCYsg-OT6-p2
To test WordPress.com changes, run |
oswian
left a comment
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.
LGTM. All storybooks started with no errors.
| const testWorkspaces = { | ||
| name: 'yarn', | ||
| args: 'workspaces foreach -A --verbose --parallel run storybook --ci --smoke-test', | ||
| args: 'workspaces foreach -A --verbose --parallel run storybook:start --ci --smoke-test', |
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.
Prior to this, the CI check was running storybook on all workspaces that had a storybook script defined in its package.json. From v8, because the Storybook CLI command was renamed from sb to storybook, the same CI check also runs the storybook command (not package.json script) directly wherever possible.
This is not desirable, as it runs the raw storybook command in workspaces like wp-calypso (where the boot script is called storybook:start rather than storybook) and @automattic/calypso-storybook (which doesn't have a Storybook instance to run).
In this PR, we address it by renaming all the storybook boot scripts to storybook:start, so they can be correctly targeted by a yarn workspaces foreach command like in the CI check.
As a bonus, the main wp-calypso Storybook is now also smoke tested, where previously it was not.
|
Nice work getting this one to the finish line @mirka 👏 |
Closes ARC-34
Proposed Changes
Updates all the Storybook instances to v8.6.9.
Also renames the Storybook boot scripts in all the workspaces to
storybook:start. See #101924 (comment) for why.Why are these changes being made?
For modern features and faster launch times.
Testing Instructions
All Storybooks should start as before, without errors.
Pre-merge Checklist