search

etchteam / storybook-addon-css-variables-theme

Change the CSS variable based design tokens in your storybook on the fly
MIT License
28 stars 12 forks source link

2.1.0 is not using dependencies anymore which breaks functionality #62

Closed pkyeck closed 5 months ago

pkyeck commented 5 months ago

hi i updated from 2.0.6 -> 2.1.0 and the plugin is not working anymore b/c you got rid of the dependencies and moved everything to the devDependencies which does not install the required packages :(

[storybook] ✘ [ERROR] Could not resolve "@storybook/icons"
[storybook]
[storybook]     node_modules/@etchteam/storybook-addon-css-variables-theme/dist/manager.js:4:21:
[storybook]       4 │ var _icons = require("@storybook/icons");
[storybook]         ╵                      ~~~~~~~~~~~~~~~~~~
[storybook]
[storybook]   You can mark the path "@storybook/icons" as external to exclude it from the bundle, which will remove this error. You can also surround this "require" call with a try/catch block to handle this failure at run-time instead of bundle-time.
[storybook]
[storybook] Error: Build failed with 1 error:
[storybook] node_modules/@etchteam/storybook-addon-css-variables-theme/dist/manager.js:4:21: ERROR: Could not resolve "@storybook/icons"
DanWebb commented 5 months ago

Since v7 storybook are advising addon authors to move to devDependencies, are you using v7 yet?

I haven't seen this error in testing yet but the storybook docs don't make it clear specifically which packages are "Globalized" so it's also possible @storybook/icons isn't one of them and it'll need to go back into the dependencies like you suggest.

pkyeck commented 5 months ago

yes, I'm using v7

"@etchteam/storybook-addon-css-variables-theme": "2.1.0",
"@storybook/addon-essentials": "7.6.18",
"@storybook/addon-links": "7.6.18",
"@storybook/addon-queryparams": "7.0.1",
"@storybook/addon-viewport": "7.6.18",
"@storybook/blocks": "7.6.18",
"@storybook/html": "7.6.18",
"@storybook/html-webpack5": "7.6.18",
"storybook": "7.6.18"

I got most of the storybook modules in my package.json as well but not the icons and that's where the error comes from

pkyeck commented 5 months ago

I haven't seen this error in testing yet but the storybook docs don't make it clear specifically which packages are "Globalized" so it's also possible @storybook/icons isn't one of them and it'll need to go back into the dependencies like you suggest.

do you refer to this part in the docs? https://storybook.js.org/docs/addons/addon-migration-guide#react-peer-dependency-is-no-longer-required

it also says:

This assumes your addon uses tsup for bundling. If your addon was built with an older version of the Addon Kit that uses Babel for bundling, you must first switch to tsup. For guidance, explore these older changes implemented in the Addon Kit repository.

so maybe it's because you are still using babel atm

DanWebb commented 5 months ago

It was this globalized packages text that I linked https://github.com/storybookjs/addon-kit?tab=readme-ov-file#globalized-packages

Storybook provides a predefined set of packages that are available in the manager UI and the preview UI. In the final bundle of your addon, these packages should not be included. Instead, the imports should stay in place, allowing Storybook to replace those imports with the actual packages during the Storybook build process.

I did manage to uncover the specific list of globalized packages, there's a separate one for manager and preview:

"@storybook/icons" is included there in the manager/globals.

Investigating more it probably is down to a difference between how the two bundlers work – tsup externals won't try to bundle the code into the end package but Babel probably will(?), might explain why it works in some cases and not others.

Ideally, we don't want to be requiring separate versions of global dependencies that Storybook now expects to provide itself. So, I will have a go at moving to the same tsup config as the addon kit now uses to accomplish that.

Thanks for the feedback and sorry for messing up the installation 🙈

pkyeck commented 5 months ago

@DanWebb Are the globalized packages a storybook v8 feature? B/c I just tried to install #65 but get the same error with v7.

DanWebb commented 5 months ago

Damn, I got a reproduction up using v7 and can see the same. It looks like Storybook added this package in v8 so it wouldn't be part of the globals until v8.

I think we'll need to bring icons back as a dependency in the v2 line and then release v3 with it removed.

pkyeck commented 5 months ago

I think releasing the v8 stuff as your v3 would make sense so others don't get the breaking changes via auto-update b/c they have ^2.0.9 in their package.json