-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Storybook: Fix dev script
#72487
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
Storybook: Fix dev script
#72487
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
| "start": "npm run dev", | ||
| "prestorybook:build": "npm run build", | ||
| "storybook:build": "storybook build -c ./storybook -o ./storybook/build", | ||
| "prestorybook:dev": "npm run build", |
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.
The pre script is no longer necessary after the new dev script (bin/dev.mjs).
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.
I wondered why the pre-script wasn't enough to prevent the error (i.e. have a single built pass before the Storybook dev command starts), but then I noticed the build script clears existing build artifacts before starting.
What if we just didn't do that?
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.
That's technically possible I guess, but it will still be slower than the marker file approach because we'd need to wait for a full build before initiating the Storybook command.
Not sure why we do a pre-clean in the dev script but not the build script though.
|
Size Change: 0 B Total Size: 2.2 MB ℹ️ View Unchanged
|
|
Flaky tests detected in 5a9b8c0. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/18650837576
|
| "storybook:dev": "concurrently \"npm run dev\" \"storybook dev -c ./storybook -p 50240\"", | ||
| "storybook:e2e:dev": "concurrently \"npm run dev\" \"storybook dev -c test/storybook-playwright/storybook -p 50241\"", | ||
| "storybook:dev": "concurrently \"npm run dev\" \"wait-on .dev-ready && storybook dev -c ./storybook -p 50240\"", | ||
| "storybook:e2e:dev": "concurrently \"npm run dev\" \"wait-on .dev-ready && storybook dev -c test/storybook-playwright/storybook -p 50241\"", |
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.
I don't like that typescript is forcing us to have this marker. I wonder if we can simplify and run things sequentially or something.
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.
Yeah, it could be possible to break down the dev/build scripts into smaller scripts. That itself may have complexity trade-offs though.
Related to #72488
What?
Fixes the
storybook:devcommand so it doesn't fail on race conditions.Why?
The
npm run storybook:devcommand currently fails with these kind of errors:The
npm run devcommand andstorybook devcommand are run concurrently, and it happens that with our new esbuild setup, the Storybook side of the build starts while the packages are still mid-build. We need a way to signal when exactly the initial package builds are ready.How?
Create a temporary marker file when the initial build is ready, and watch for that file before we start the Storybook dev command.
Testing Instructions
npm run storybook:devshould work.