Vite: Re-export essentials sub-addon preview annotations for pnpm

[IanVS]

Issue: #19176

Closes #19530

What I did

In order to support pnpm from the vite builder, I added re-exports of the sub-addons which have preview annotations, and adjusted the preset handling to leave bare imports alone instead of turning them into absolute paths. This allows pnpm to correctly find the files and avoids users from having to "shamefully hoist" or add the sub-addons to their package.json.

I added typesVersions to the addons, so that we can export * from "@storybook/addon-actions/preview", for instance. The .d.ts file isn't at the root of the package, it's in /dist, so using typesVersions lets us point to the right spot. This is one of the approaches shown in https://github.com/andrewbranch/example-subpath-exports-ts-compat, and has the best overall compatibility.

One problem I hit is that tsup generates empty .d.ts files for manager, so I filed egoist/tsup#762 and needed to ignore errors when re-exporting from them.

How to test

We don't have any good tests for pnpm, unfortunately, so this is a bit manual.

• Build this branch, especially lib/builder-vite, lib/core-common, and addons/essentials
• Somewhere else on your filesystem, create a pnpm vite project (pnpm create vite@latest --template=react)
• Add storybook (pnpx sb@next init --builder=vite)
• Copy over the dist of lib/builder-vite into node_modules/.pnpm/@[email protected]_<hash>/node_modules/@storybook/builder-vite
• Copy over the dist of lib/core-common into node_modules/.pnpm/@[email protected]_<hash>/node_modules/@storybook/core-common
• Copy the dist and package.json of addons/essentials into node_modules/@storybook/addon-essentials/
• Start up storybook, (pnpm run storybook), and it should start successfully.

[IanVS]

@Dschungelabenteuer and I found a few bugs in SSv6 that this now takes care of, and we've confirmed that his project (using pnpm and SSv7) is working correctly without any workarounds with these changes. \o/

[IanVS]

SSv6 with the vite-builder and pnpm has been broken for a while. I figured I might as well fix it in this PR, but I can move it to a different one if preferred. I'd like to make a separate PR soon to create a vite-core that each of the frameworks re-export from, to avoid all the duplication in the frameworks above. I'll do that soon.

[tmeasday]

WFM!

[tmeasday]

Some nitpicking, but one important detail to cover.

[tmeasday]

WFM!

[Dschungelabenteuer]

@Dschungelabenteuer and I found a few bugs in SSv6 that this now takes care of, and we've confirmed that his project (using pnpm and SSv7) is working correctly without any workarounds with these changes. \o/

I can confirm this works perfectly against my pnpm monorepo. This is looking pretty good, especially since my environment has some abstraction around Vite+Storybook configuration, which is shared across different Storybook instances and consumed as a local package. Basically, my setup could easily have raised some edge-case issues, but it did not at all! Well done @IanVS! This lets me get rid of ~7 "explicit" installs from my package.json/.pnpmfile.cjs, it feels like going out the hairdresser (I'm bald, though) 💯

There are still some workarounds needed for MDX (which relies on react dependencies), but this is very likely another issue and IIRC this was mentioned during the monthly meeting. Not a big deal! since I'm exposing homemade React components to enhance my docs (using Storybook with Vue in my project), I still need these deps whatever happens, not a lot I can do here, anyway!

Please let me know if the latest changes require additional testing on my side!

[tmeasday]

Sorry @IanVS I was going to double check with @shilman before merging this yesterday but then it slipped our attention. Will do today!

[shilman]

This LGTM but is all a bit clever/subtle. I hope that we can come up with something simpler in 8.0!!

[IanVS]

I think the main change here is that if an addon wants to include other addons without forcing the user to also install those, it should re-export from them and point to its re-exports, rather than just specify the names of the sub-addons. We'll need to decide if that's a good change to force community addons to make, or if there's another approach like all addons being imported into a preview.js or something. This is "backwards compatible" for now in the sense that webpack will continue working as before (I think that's the "clever" bit you're referring to?), but the vite builder will be broken with pnp(m) for any community addons that have sub-addons. So yeah, I hope we can refine this as we go.

[shilman]

@IanVS I am referring to the whole absolute path / bare thing. And the amount of work that an addon has to do to include other addons. I hope we can streamline this with the preview.js addons proposal.

That said, you've done an incredible job getting this working for 7.0 given the limitations of our current presets setup. Did not intend to piss on your work -- this is really great.

[IanVS]

the amount of work that an addon has to do to include other addons

That should just really be a matter of specifying something like:

export const addons = [
	'super-addon/reexport',
	'super-addon/other-reexport'
];

I got a little fancy with export maps here, but there are other ways to accomplish that as well (but this worked the best with our tooling).

[matsko]

Hey it looks like something in here caused this issue for me:

ERR! TypeError [PLUGIN_ERROR]: Could not load /virtual:/@storybook/builder-vite/vite-app.js (imported by iframe.html): The "path" argument must be of type string. Received an instance of Object

Any idea what caused it? I can confirm the bug shows up at alpha-49.

[IanVS]

@matsko could you open a new issue with all the details, and ideally a reproduction, and ping me on it? My guess is that you might have an old version of @storybook/builder-vite, or some other package, installed.