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

Consider switching from Webpack to Vite #1403

Open
tylersticka opened this issue Jul 16, 2021 · 6 comments · May be fixed by #2012
Open

Consider switching from Webpack to Vite #1403

tylersticka opened this issue Jul 16, 2021 · 6 comments · May be fixed by #2012
Assignees
Labels
👷 dev 🔌 dependencies Pull requests that update a dependency file 👷 environment Setup, build tools, configuration, etc.

Comments

@tylersticka
Copy link
Member

tylersticka commented Jul 16, 2021

Storybook 6.3 introduces support for bundlers other than Webpack: https://storybook.js.org/blog/storybook-for-vite/

@calebeby pointed out while reviewing a PR by @gerardo-rodriguez that using Vite might simplify the process we've been using for exposing template source code to stories: https://github.com/cloudfour/cloudfour.com-patterns/pull/1402/files#r670882613

Storybook has pluggable bundlers now, so it's not very tied to webpack anymore. But we could make a vite plugin for this syntax if we switched to vite

Unfortunately we haven't upgraded to 6.3 yet because of unresolved regressions related, perhaps ironically, to Webpack. See: #1349

While I don't love the idea of refactoring the project's configuration (which is currently pretty Webpack-centric), if the 6.3 upgrade will require some reconfiguration anyway, we might as well consider switching to a bundler we like better and that might gain us some efficiencies.

@tylersticka tylersticka added 🔌 dependencies Pull requests that update a dependency file 👷 environment Setup, build tools, configuration, etc. size:? labels Jul 16, 2021
@tylersticka tylersticka added this to 🔙 Backlog in cloudfour.com-patterns@next via automation Jul 16, 2021
@tylersticka tylersticka moved this from 🔙 Backlog to ⏭ Up Next in cloudfour.com-patterns@next Jul 16, 2021
@Paul-Hebert
Copy link
Member

I've heard good things about Vite! I also don't love the idea of refactoring the config, but I do love the idea of getting experience with Vite!

@calebeby
Copy link
Member

calebeby commented Jul 16, 2021

I think it would make a lot of sense to switch, here's some reasons:

  • Fast startup time (<500ms)
  • Faster HMR/updates on save (<200ms)
  • Smaller production bundle (because rollup)
  • Easier and more readable configuration/better customizability
  • Smaller dependency graph (no more zillions of dependencies from the webpack ecosystem) storybook still has webpack as a dependency even if we don't use it
  • Better error messages

I'm taking a look to try to size this issue

@calebeby
Copy link
Member

calebeby commented Jul 20, 2021

Here are things I noticed while testing out the swap:

  • Vite isn't able to statically determine the module graph so the pre-bundling dependencies step runs 3-4 times as it builds some stuff, tries to load it, runs into some dependencies, runs the pre-bundling again, and reloads. This is annoying but not too much of a problem since it should cache them.
  • Something is preventing the pre-bundling result from being cached. This is a problem.
  • Any time we use import { ... } from 'path' in the browser, that does not work because path doesn't exist in the browser.
  • Any time we do require() or require.context in the browser, that does not work because require does not exist in the browser and require.context is a webpack-ism. We can get something relatively similar using glob import (a vite-ism): https://vitejs.dev/guide/features.html#glob-import
  • The webpack loader syntax for files does not work (!!raw-loader!, etc) does not work either, but we can make it work easily by adding a rollup plugin.
  • When you run start-storybook, sometimes webpack starts (along with vite). Most of the time only vite though. It is really weird when webpack starts too.
  • I made a sort-of-working twing rollup plugin. I got to the point where one of its dependencies is buffer, and the browser-emulator for that module expects global to exist, and it doesn't. This seems very solve-able.

Conclusion: The lack of caching problem seems like a deal-breaker. I did not look into why it is happening or whether it could be solved. It could be a problem that we have to wait for storybook-builder-vite to solve, or maybe it is a problem that we can fix. Once (if) we can solve that problem, I think the rest of the problems I mentioned seem very solvable.

Giving it a 10. If the lack of caching problem is something that storybook-builder-vite fixes, I think this is a 5.

I pushed my experiments to the vite-experiment branch. The code in there is not very good, when we actually tackle this issue it is probably best to start from scratch, taking ideas (and rollup plugins) from that branch.

@calebeby calebeby added size:10 and removed size:? labels Jul 20, 2021
@tylersticka tylersticka moved this from ⏭ Up Next to 🔙 Backlog in cloudfour.com-patterns@next Jul 20, 2021
@tylersticka tylersticka removed this from 🔙 Backlog in cloudfour.com-patterns@next Jul 22, 2021
@calebeby calebeby self-assigned this Aug 16, 2022
@calebeby
Copy link
Member

I got stuck on something here... There are a couple stories that don't render correctly. I spent most of yesterday trying to debug it and I couldn't figure it out. I'm gonna take a break from this for a while and hopefully when I come back to it with fresh eyes I'll be able to fix it.

@calebeby calebeby linked a pull request Aug 23, 2022 that will close this issue
@calebeby
Copy link
Member

I realized that most of the issues I was having was due to upgrading twing. Newer versions of twing are asynchronous, while the older version we were using was synchronous. I think it is likely that if I switch back to the old version it will fix the issue I was seeing with a couple stories.

Storybook will eventually have first-class support for async stories: storybookjs/storybook#12726, so when that comes around it will probably be easier to use an async version of twing.

@tylersticka
Copy link
Member Author

It looks like Storybook 7 has added a lot of Vite improvements: https://storybook.js.org/blog/first-class-vite-support-in-storybook/

That issue still looks to be open, though, which is disappointing. Also a little confusing, since it's included in 6.x milestones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👷 dev 🔌 dependencies Pull requests that update a dependency file 👷 environment Setup, build tools, configuration, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants