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

Core: Support webpack5 and webpack4 side by side #13808

Merged
merged 122 commits into from
Feb 20, 2021
Merged

Core: Support webpack5 and webpack4 side by side #13808

merged 122 commits into from
Feb 20, 2021

Conversation

shilman
Copy link
Member

@shilman shilman commented Feb 3, 2021

Issue: #9216

What I did

With @ndelangen:

  • Upgrade to webpack5 + dependencies
  • Fixed webpack build in official example
  • Split up @storybook/core into core-server, core-client, core-common
  • Introduce a builder abstraction and implement builder-webpack4, builder-webpack5
  • Set up snapshot tests for the webpack configuration so that we can test against next
  • Set up an e2e test that runs outside the monorepo to avoid hoisting issues

How to test

See core-presets.test.ts whose snapshots we've manually checked against next (webpack4)

@shilman shilman added the run e2e extended test suite Run the e2e extended test suite in CircleCI workflows label Feb 19, 2021
@shilman
Copy link
Member Author

shilman commented Feb 20, 2021

Early, completely non-optimized perf testing.

Install size up, build size down

This is a CRA benchmark (webpack4) and we are building the Storybook "manager" UI with (webpack5), so that accounts for the extra install time. But build time is faster (20s vs ~30s before) with no optimizations added yet.

Storybook_Benchmark_›_Explorer__Install__Start__Build_

Static storybooks load slightly faster

For static storybooks, time to first render is slightly faster (300ms vs 370ms before):

Storybook_Benchmark_›_Explorer__Browse_

@shilman
Copy link
Member Author

shilman commented Feb 20, 2021

@ndelangen we still need to set up an e2e test for webpack5, but can do it in a separate PR.

@gaetanmaisse I've broken the yarn2 PNP test but am merging anyway because my head will explode if I look at this PR any longer. Apologies for continuing to subject you to this.

@shilman shilman changed the title [WIP] Core: Support webpack5 and webpack4 side by side Core: Support webpack5 and webpack4 side by side Feb 20, 2021
@shilman shilman merged commit 32c6356 into next Feb 20, 2021
@shilman shilman deleted the tech/core-builder branch February 20, 2021 11:56
@phaistonian
Copy link
Contributor

@shilman Somehow running yarn storybook builder-webpack4 is preferred, as opposed to the builder-webpack5 one.

I guess the detection phase needs some extra care.

Ps: any way to force use the webpack5 one?

@shilman
Copy link
Member Author

shilman commented Feb 22, 2021

@phaistonian yes you need to set it in .storybook/main.js

module.exports = {
  core: { builder: 'webpack5' }
}

See the full migration guide here:

https://gist.github.com/shilman/8856ea1786dcd247139b47b270912324

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants