Upgrade to Storybook v7 with Vite

[jesstelford]

---

See it in action: https://5d559397bae39100201eedc1-otbapvfeve.chromatic.com

Upgrades Storybook to v7 so we can benefit from Vite's improved build times over Webpack, plus be closer aligned to how we build our components for production in @shopify/polaris.

Performance

main

Benchmark 1: yarn workspace @shopify/polaris run storybook:build --quiet
  Time (mean ± σ):     17.833 s ±  0.192 s    [User: 64.729 s, System: 3.628 s]
  Range (min … max):   17.506 s … 18.067 s    10 runs

storybook-v7

Benchmark 1: yarn workspace @shopify/polaris run storybook:build --quiet
  Time (mean ± σ):     17.798 s ±  0.551 s    [User: 30.510 s, System: 2.862 s]
  Range (min … max):   17.225 s … 19.239 s    10 runs

That's a total saving of 35ms! 🎉 ... 🤔

Update: Re-ran the perf test, and this time... it's 1s slower 😓

❯ hyperfine --parameter-list branch main,storybook-v7 --setup='git checkout {branch} && yarn && yarn build --filter=@shopify/polaris' --prepare='rm -rf polaris-react/build-internal/storybook/' --warmup 1 'yarn workspace @shopify/polaris run storybook:build --quiet'
Benchmark 1: yarn workspace @shopify/polaris run storybook:build --quiet (branch = main)
  Time (mean ± σ):     16.509 s ±  0.253 s    [User: 65.154 s, System: 3.320 s]
  Range (min … max):   16.200 s … 17.073 s    10 runs

Benchmark 2: yarn workspace @shopify/polaris run storybook:build --quiet (branch = storybook-v7)
  Time (mean ± σ):     17.011 s ±  0.192 s    [User: 30.488 s, System: 2.655 s]
  Range (min … max):   16.632 s … 17.270 s    10 runs

Summary
  yarn workspace @shopify/polaris run storybook:build --quiet (branch = main) ran
    1.03 ± 0.02 times faster than yarn workspace @shopify/polaris run storybook:build --quiet (branch = storybook-v7)

Tophatting

• Is POLARIS_VERSION being replaced within the JS in dev + prod builds?
• Check all stories render correctly
• Check the STORYBOOK_ env vars are making their way into the bundle in CI (for the GitHub icon/link in the toolbar)

[github-actions]

size-limit report 📦

Path	Size
polaris-react-cjs	240.81 KB (0%)
polaris-react-esm	156.93 KB (0%)
polaris-react-esnext	219.13 KB (0%)
polaris-react-css	47.27 KB (0%)

[BPScott]

With the addition of polaris-patterns stuff, it feels like this would be a good opportunity to pull the storybook stuff into the root of the repo rather than be within polaris-react. Then polaris could have one storybook instance that can serve up content from multiple packages, instead of each package needing to maintain its own storybook config (or even weirder, the storybook in polaris-react loading stories from other packages)

I'd be tempted to pull the .scss -> .module.scss renaming out into a separate PR once this is all ready to keep "boring automations" all in one PR separate to the more interesting shuffling.

[BPScott]

I think there's a world where you don't need this namespaced-classname-module.mjs file. node should be fine with importing commonjs files within a esmodules file using import.

import {generateScopedName} from './config/rollup/namespaced-classname.js'; should work. It it doesn't then you could make that be the default export, or you might need to fiddle with how the export is defined to make it more statically analysable. If you need to do that, I think it's a case of doing module.exports = {generateScopedName} at the bottom of the file instead of module.exports.generateScopedName = BLAH

You might need to fiddle with how the modules are declared

[jesstelford]

I had to revisit this one because I'd forgotten the reason we'd gone this route. Turns out it's unfortunately not as simple as node's file type detection; Vite is baulking on the require('path') within config/rollup/namespaced-classname.js:

ERR! Error: Dynamic require of "node:path" is not supported
ERR!     at file:///Users/jess/dev/polaris/polaris-react/vite.config.mjs.timestamp-1682311621838.mjs:12:9
ERR!     at config/rollup/namespaced-classname.js (/Users/jess/dev/polaris/polaris-react/config/rollup/namespaced-classname.js:1:369)
ERR!     at __require2 (file:///Users/jess/dev/polaris/polaris-react/vite.config.mjs.timestamp-1682311621838.mjs:15:50)
ERR!     at <anonymous> (/Users/jess/dev/polaris/polaris-react/vite.config.mjs:7:34)
ERR!     at ModuleJob.run (node:internal/modules/esm/module_job:193:25)
ERR!     at async Promise.all (index 0)
ERR!     at async ESMLoader.import (node:internal/modules/esm/loader:533:24)
ERR!     at async loadConfigFromBundledFile (file:///Users/jess/dev/polaris/node_modules/vite/dist/node/chunks/dep-d305c21f.js:63967:21)
ERR!     at async Module.loadConfigFromFile (file:///Users/jess/dev/polaris/node_modules/vite/dist/node/chunks/dep-d305c21f.js:63852:28)
ERR!     at commonConfig (/Users/jess/dev/polaris/node_modules/@storybook/builder-vite/dist/index.js:161:5305)

It looks like Storybook is doing some kind of pre-processing on the files (see the .timestamp-XXXXX suffixes) which might be messing with it 🤔

[jesstelford]

An easy fix: The only other place this functionality is requires is in the rollup.config.mjs file which doesn't error when importing namespaced-classname-module.mjs 🎉

I've made that changed and pushed it to this branch.

[BPScott]

Nice! great spot realising that the only other place we use this is an esmodules file. I'd forgotten that polaris moved its rollup files over to be mjs.

[jesstelford]

good opportunity to pull the storybook stuff into the root of the repo rather than be within polaris-react

Agreed! We started exploring this a bit in another branch, but it rapidly got more complex than one PR should contain, hence this first step. Future PRs could even build polaris-react with Vite also (since Vite uses rollup under the hood), which means our Storybook & our npm build would share a config. But that's for the future / another discussion 👍

[jesstelford]

I'd be tempted to pull the .scss -> .module.scss renaming out into a separate PR

This could be possible, but it would mean messing with the webpack config in Storybook v6 to ensure that commit is atomic which I didn't want to bother with given it's immediately going to be deleted by this PR.

It's a good point though about keeping the changes seperate; if you think it's worth it to preserve the history like that, I could be persuaded 👍

[BPScott]

This could be possible, but it would mean messing with the webpack config in Storybook v6 to ensure that commit is atomic which I didn't want to bother with given it's immediately going to be deleted by this PR.

The storybook config on main uses the auto option to control what get passed through css modules. Currently it is set up so every scss file should use modules, with the exception of paths containing global, and it looks like that's only the AppProvider global file. Switching that over to true should result in the same logic as Vite - files with only .module. in the filename get passed through. I think things should continue to work as expected if you only renamed the files and didn't touch the webpack config at all.

Obviously I'm not on polaris anymore so I'm just an interested observer - the final call is very munch in your hands :) but I suspect splitting the renames out, wouldn't require the extra work you're concerned about as I don't think we'd actually need to touch the webpack css-loader config when doing the mass rename.

[jesstelford]

@BPScott With your help I was able to pull out the file renames to a separate PR which is now merged 🎉
#11340

[aaronccasanova]

Amazing! 👏

[jesstelford]

Thanks for the review and suggestions @aaronccasanova!

I realized there was a small bug with replacing the magic string {{POLARIS_VERSION}} in storybook; it wasn't being replace 😓

So I've fixed that up by leveraging https://www.npmjs.com/package/vite-plugin-replace (which ultimately just does code.replace()). Can confirm by looking at the CSS Custom property --polaris-version-number on the :root element:

[jesstelford]

@BPScott @alex-page I've created https://github.com/Shopify/temp-project-mover-Archetypically-20250305131128/issues/176 to track the next logical step of this work.