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: Make vite the default builder #22051

Closed
wants to merge 1 commit into from

Conversation

kasperpeulen
Copy link
Contributor

What I did

New projects without a build setup still default to webpack. However, as vite is less prone to config problems (like a missing babelrc file) and in many cases faster, I think that it would makes sense to make vite the default now.

How to test

Create a new project, and run npx sb init, and see that it will get a vite builder.

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

@ndelangen
Copy link
Member

This needs thorough QA!

@kasperpeulen kasperpeulen added feature request cli patch:yes Bugfix & documentation PR that need to be picked to main branch labels Apr 12, 2023
@joeycozza
Copy link
Contributor

Seems like you'd also want to update documentation around this at the same time.

https://github.com/storybookjs/storybook/blob/next/docs/builders/webpack.md?plain=1#L5

Storybook Webpack builder is the default builder for Storybook.

@jonniebigodes
Copy link
Contributor

@joeycozza it will be. Once this is merged I'll push a pull request adjusting it, thanks for the heads up. Appreciate it 🙏

@shilman shilman added maintenance User-facing maintenance tasks and removed feature request patch:yes Bugfix & documentation PR that need to be picked to main branch labels Apr 15, 2023
@shilman
Copy link
Member

shilman commented Apr 15, 2023

Let's wait to patch this back until it's been thoroughly tested.

default:
case ProjectType.NEXTJS:
case ProjectType.REACT_SCRIPTS:
case ProjectType.WEBPACK_REACT:
Copy link
Member

Choose a reason for hiding this comment

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

I think you'd need to add more stuff here, such as: Angular, Aurelia, Ember, Marionette, Marko, Mithril, Rax, React_project, Riot, Server.

Copy link
Member

@yannbf yannbf left a comment

Choose a reason for hiding this comment

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

Apart from doing thorough QA, it's important to note that differently than builder-webpack, which has webpack as a dependency, the builder-vite has vite as peer dependency. This means projects won't really work unless we made bigger changes to the CLI, to install Vite as well!

@ndelangen
Copy link
Member

Fair point @yannbf
So the CLI should add vite as a dependency for the user, if the framework chosen is vite-based, and no vite dependency is found.

This might not be a really complex change, @kasperpeulen are you planning to make this change?

@yannbf
Copy link
Member

yannbf commented May 11, 2023

Given the behavior was changed here: #22492
I believe we can safely close this PR!

@yannbf yannbf closed this May 11, 2023
@stof stof deleted the kasper/vite-default-builder branch August 2, 2023 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants