Skip to content
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

CLI: Default new vite projects to storyStoreV7 #17859

Merged
merged 1 commit into from
Apr 3, 2022

Conversation

IanVS
Copy link
Member

@IanVS IanVS commented Apr 2, 2022

Issue:

What I did

The vite builder benefits greatly from the on-demand story store, especially in dev-mode where modules are all sent to the browser unbundled (for better HMR and faster server startup). In large vite storybook apps, this can mean thousands of network requests, which can take some time for the browser to process, causing a longer time-to-interactive. (Note, this isn't a problem in built storybooks, since vite bundles them with rollup to avoid this problem).

Furthermore, the new store is much more reliable and less likely to encounter bugs and subtle differences between the vite and webpack builders. A PR, storybookjs/builder-vite#303, is currently open which would allow the use of the new store while not lazy-loading, in case users prefer or require that approach.

How to test

  • Is this testable with Jest or Chromatic screenshots?
  • Does this need a new example in the kitchen sink apps?
  • Does this need an update to the documentation?

I'm not sure how the cli is currently tested, other than in e2e tests, which do not use the vite builder (maybe we should add a few?)

But, I did build my changes, create a new vite project, and run the changed cli against it, and verified that features: {storyStoreV7: true} was in the generated config.

@nx-cloud
Copy link

nx-cloud bot commented Apr 2, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 9630843. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@IanVS IanVS requested a review from shilman April 2, 2022 19:53
@shilman shilman changed the title Default new vite projects to storyStoreV7 CLI: Default new vite projects to storyStoreV7 Apr 2, 2022
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@shilman shilman merged commit 60c5f5c into next Apr 3, 2022
@shilman shilman deleted the default-storyStoreV7-for-vite branch April 3, 2022 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants