search

whitespace-se / storybook-addon-html

A Storybook addon that extracts and displays compiled syntax-highlighted HTML
Other
95 stars 44 forks source link

No longer compatible with @storybook/html #91

Closed Bilge closed 7 months ago

Bilge commented 1 year ago

Upgrading "@storybook/html": "^6.3.2" -> ^6.5.15 (and "@whitespace/storybook-addon-html": "^5.0.0" -> ^5.1.1) causes the following errors at build time:

ERROR in ./node_modules/@whitespace/storybook-addon-html/dist/esm/components/SyntaxHighlighter.js Module not found: Error: Can't resolve 'react-syntax-highlighter' in 'C:\Users\Bilge\Documents\PhpstormProjects\Steam 250\node_modules\@whitespace\storybook-addon-html\dist\esm\components' @ ./node_modules/@whitespace/storybook-addon-html/dist/esm/components/SyntaxHighlighter.js 20:0-62 82:38-60 @ ./node_modules/@whitespace/storybook-addon-html/dist/esm/components/PanelContent.js @ ./node_modules/@whitespace/storybook-addon-html/dist/esm/Panel.js @ ./node_modules/@whitespace/storybook-addon-html/dist/esm/preset/manager.js @ multi ./node_modules/@storybook/manager-webpack4/node_modules/@storybook/core-client/dist/esm/globals/polyfills.js ./node_modules/@storybook/manager-webpack4/node_modules/@storybook/core-client/dist/esm/manager/index.js ./node_modules/@storybook/addon-links/manager .js ./node_modules/@storybook/addon-docs/manager.js ./node_modules/@storybook/addon-controls/manager.js ./node_modules/@storybook/addon-actions/manager.js ./node_modules/@storybook/addon-backgrounds/manager.js ./node_modules/@storybook/addon-viewport/manager.js ./node _modules/@storybook/addon-toolbars/manager.js ./node_modules/@storybook/addon-measure/manager.js ./node_modules/@storybook/addon-outline/manager.js ./node_modules/@whitespace/storybook-addon-html/dist/esm/preset/manager.js

ERROR in ./node_modules/@whitespace/storybook-addon-html/dist/esm/components/PanelContent.js Module not found: Error: Can't resolve 'react-syntax-highlighter/dist/esm/styles/hljs/github-gist' in 'C:\Users\Bilge\Documents\PhpstormProjects\Steam 250\node_modules\@whitespace\storybook-addon-html\dist\esm\components' @ ./node_modules/@whitespace/storybook-addon-html/dist/esm/components/PanelContent.js 3:0-78 16:11-16 @ ./node_modules/@whitespace/storybook-addon-html/dist/esm/Panel.js @ ./node_modules/@whitespace/storybook-addon-html/dist/esm/preset/manager.js @ multi ./node_modules/@storybook/manager-webpack4/node_modules/@storybook/core-client/dist/esm/globals/polyfills.js ./node_modules/@storybook/manager-webpack4/node_modules/@storybook/core-client/dist/esm/manager/index.js ./node_modules/@storybook/addon-links/manager .js ./node_modules/@storybook/addon-docs/manager.js ./node_modules/@storybook/addon-controls/manager.js ./node_modules/@storybook/addon-actions/manager.js ./node_modules/@storybook/addon-backgrounds/manager.js ./node_modules/@storybook/addon-viewport/manager.js ./node _modules/@storybook/addon-toolbars/manager.js ./node_modules/@storybook/addon-measure/manager.js ./node_modules/@storybook/addon-outline/manager.js ./node_modules/@whitespace/storybook-addon-html/dist/esm/preset/manager.js

It now seems to assume that react-syntax-highlighter would be available, even though it certainly wouldn't, in an HTML-only project that does not use React whatsoever.

Edit: Note this error just occurs due to upgrading this addon; upgrading Storybook itself is immaterial. Downgrading to 5.0.0 "fixes" the issue.

mylmz10 commented 1 year ago

Hi, same issue

jeanfredrik commented 1 year ago

Since version 5 you have to manually add react-syntax-highlighter as a dependency. From the readme:

Install the addon and its dependencies.

With NPM:

npm i --save-dev @whitespace/storybook-addon-html prettier react-syntax-highlighter

With Yarn:

yarn add -D @whitespace/storybook-addon-html prettier react-syntax-highlighter

Bilge commented 1 year ago

But why? I don't use React. In any case, if this isn't an optional dependency then it should not have to be added manually at all.

jeanfredrik commented 1 year ago

The Storybook application uses React, including all the UI and addons, regardless of what framework you use for your components. react-syntax-highlighter is used in the panel to highlight the HTML syntax. It was added as a peer dependency (that you need to install manually) to allow you to choose which version to use, which was required to solve issue #38.

Did adding react-syntax-highlighter to you package.json solve the problem for you?

Bilge commented 1 year ago

Maybe my understanding of the Node dependency ecosystem is poor, but according to the issue you linked it seems you can (and wish to) declare the library is compatible with versions 13 through 15 of the dependency, react-syntax-highlighter. But rather than do that, you just removed the dependency altogether, despite the fact that you still have such a dependency, and instead just documented that it's the end-user's responsibility to include the dependency.

Now, again, maybe my understanding of NPM and such is poor, but it is my understanding of dependencies and dependency solvers in general that libraries should declare all their dependencies and, if they are compatible with a range of dependencies, to declare that range. It is not, in any scenario, correct to simply yank the dependency and push that burden of knowledge out to the consumer of your library.

stof commented 1 year ago

For a dependency, npm always uses the highest version matching the range. There is no config that will allow a project to install version 13 of the dependency (if you depend on it at the root, you will end up with 2 different versions in your node_modules folder)