storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
83.77k stars 9.18k forks source link

[Bug]: Storybook doesn’t work with unplugin icons #20562

Open enyo opened 1 year ago

enyo commented 1 year ago

Describe the bug

When adding unplugin/icons for sveltekit, it fails due to the docgen plugin.

To Reproduce

https://github.com/enyo/storybook-test/tree/main/my-app

System

No response

Additional context

https://discord.com/channels/486522875931656193/1061953675263742042

tony19 commented 1 year ago

I noticed this problem only in my SvelteKit stories. It works correctly with React stories.

tony19 commented 1 year ago

I was able to workaround the issue by monkey-patching svelte-docgen-plugin's transform, excluding any resources whose IDs start with ~icons/:

const workaroundSvelteDocgenPluginConflictWithUnpluginIcons = (config: InlineConfig) => {
  if (!config.plugins) return config

  const [_internalPlugins, ...userPlugins] = config.plugins as Plugin[]
  const docgenPlugin = userPlugins.find(plugin => plugin.name === 'storybook:svelte-docgen-plugin')
  if (docgenPlugin) {
    const origTransform = docgenPlugin.transform
    const newTransform: typeof origTransform = (code, id, options) => {
      // ignore unplugin-icons resources
      if (id.startsWith('~icons/')) {
        return
      }
      return (origTransform as Function)?.call(docgenPlugin, code, id, options)
    }
    docgenPlugin.transform = newTransform
    docgenPlugin.enforce = 'post'
  }
  return config
}
Example TypeScript Storybook config ```typescript // .storybook/main.ts import type { Plugin, InlineConfig } from 'vite' import type { StorybookConfig } from '@storybook/sveltekit' // https://github.com/storybookjs/storybook/issues/20562 const workaroundSvelteDocgenPluginConflictWithUnpluginIcons = (config: InlineConfig) => { if (!config.plugins) return config const [_internalPlugins, ...userPlugins] = config.plugins as Plugin[] const docgenPlugin = userPlugins.find(plugin => plugin.name === 'storybook:svelte-docgen-plugin') if (docgenPlugin) { const origTransform = docgenPlugin.transform const newTransform: typeof origTransform = (code, id, options) => { if (id.startsWith('~icons/')) { return } return (origTransform as Function)?.call(docgenPlugin, code, id, options) } docgenPlugin.transform = newTransform docgenPlugin.enforce = 'post' } return config } const config: StorybookConfig = { stories: ['../src/**/*.mdx', '../src/**/*.stories.@(js|jsx|ts|tsx)'], addons: [ '@storybook/addon-links', '@storybook/addon-essentials', '@storybook/addon-interactions', ], framework: { name: '@storybook/sveltekit', options: {}, }, docs: { autodocs: 'tag', }, viteFinal(config) { return workaroundSvelteDocgenPluginConflictWithUnpluginIcons(config) } } export default config ```
@enyo's JavaScript Storybook config, modified with the workaround ```js // .storybook/main.js import { mergeConfig } from "vite"; import Icons from "unplugin-icons/vite"; import path from "path"; // https://github.com/storybookjs/storybook/issues/20562 /** * @param {import('vite').InlineConfig} config */ const workaroundSvelteDocgenPluginConflictWithUnpluginIcons = (config) => { if (!config.plugins) return config const [_internalPlugins, ...userPlugins] = config.plugins const docgenPlugin = userPlugins.find(plugin => plugin.name === 'storybook:svelte-docgen-plugin') if (docgenPlugin) { const origTransform = docgenPlugin.transform const newTransform = (code, id, options) => { if (id.startsWith('~icons/')) { return } return origTransform?.call(docgenPlugin, code, id, options) } docgenPlugin.transform = newTransform docgenPlugin.enforce = 'post' } return config } /** * @type {import('@storybook/sveltekit').StorybookConfig} */ const config = { stories: ["../src/**/*.mdx", "../src/**/*.stories.@(js|ts|svelte)"], addons: [ "@storybook/addon-links", "@storybook/addon-essentials", "@storybook/addon-interactions", ], framework: { name: "@storybook/sveltekit", options: {}, }, docs: { autodocs: "tag", }, async viteFinal(config) { config = workaroundSvelteDocgenPluginConflictWithUnpluginIcons(config) // Merge custom configuration into the default config return mergeConfig(config, { plugins: [ Icons({ compiler: "svelte", }), ], // Add dependencies to pre-optimization optimizeDeps: { include: ["storybook-dark-mode"], }, }); }, }; export default config; ```
IanVS commented 1 year ago

Thanks for the workaround, @tony19. I'm reluctant to do this in the base plugin, since technically anyone could use ~icons/ for anything, and they may be components that we need to process. I've opened https://github.com/antfu/unplugin-icons/issues/274 to ask whether there's something that should change on unplugin-icons side, in order to prevent other plugins like ours from processing these virtual files.

khari998 commented 1 year ago

I had to add "types": ["unplugin-icons/types/svelte"] to my tsconfig in order to get everything to compile properly. See if this avoids the need for a workaround.

benmccann commented 12 months ago

It looks to me like this would only be hit when a preprocessor is being used:

https://github.com/storybookjs/storybook/blob/03b85f4f5693676cb9092cdf97d3987ce3a326bd/code/frameworks/svelte-vite/src/plugins/svelte-docgen.ts#L61

I'm not quite sure what this code is doing. It looks like the src is passed in, but rather than using it the file is read from the file system to get the src again. I don't know why though. Any ideas @IanVS ?

IanVS commented 12 months ago

No, I think it just needs to be changed not to try to read from disk. I meant to make that change but then forgot about it. Thanks for the reminder. If anyone wants to take a crack at it before I get to it, feel free!

ryu-man commented 6 months ago

Does anyone have a solution for this issue?

IanVS commented 6 months ago

Yes, I believe the solution is to change the svelte-docgen plugin in storybook to read the src instead of reading the contents from disk. Would you be willing to open a PR, @ryu-man?

rChaoz commented 4 months ago

Workaround until this gets fixed: yarn patch @storybook/svelte-vite. Then, edit dist/preset.js and replace:

if(docPreprocessOptions){let rawSource=import_fs.default.readFileSync(resource).toString(),{code:fileContent}=await(0,import_compiler.preprocess)(rawSource,docPreprocessOptions,{filename:resource});docOptions={fileContent}}

with

if(docPreprocessOptions)try{let rawSource=import_fs.default.readFileSync(resource).toString(),{code:fileContent}=await(0,import_compiler.preprocess)(rawSource,docPreprocessOptions,{filename:resource});docOptions={fileContent}}catch(e){} else

Notice right after the if(docPreprocessOptions), add just the keyword try right before the opening bracket, then right after the closing bracket before the else add catch(e){}.

@IanVS I opened a PR for this (#26838)