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 builds twice on launch #28089

Open pciarach opened 2 months ago

pciarach commented 2 months ago

Describe the bug

When starting Storybook (React latest + webpack 5 template) in a dev mode, build process runs twice before one can freely browse stories. After some debugging it turned out that the second build is triggered by the file system watcher because of 'removal' of storybook-config-entry.js and storybook-stories.js files. As I understood from the source code, these are only virtual modules that should not really exist as a separate physical files, so maybe they should be somehow excluded from the watch? I'm not quite sure how though 😅

image

This second build basically doubles Storybook launch time which for larger repos can be quite significant.

As a workaround, we created these files without any content but this is just a temporary solution.

Reproduction link

It happens on the clean sandbox reproduction repo

Reproduction steps

  1. Create clean reproduction repo with React latest + webpack 5 template (I assume that webpack part might be mandatory in order to reproduce the issue, while the React is optional and the same could happen with other libraries)
  2. Install packages and start storybook using standard commands
  3. After seeing 'welcome box' (with Storybook version, timings and URLs), notice that the progress plugin prints some output once again (on the clean reproduction repo it's quite fast but as I mentioned before it is dependent on the size of the repo)

System

System:
    OS: Windows 10 10.0.19045
    CPU: (12) x64 Intel(R) Core(TM) i7-10750H CPU @ 2.60GHz
  Binaries:
    Node: 20.11.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.22 - C:\Program Files\nodejs\yarn.CMD <----- active
    npm: 10.2.4 - C:\Program Files\nodejs\npm.CMD
    pnpm: 9.1.2 - C:\Program Files\nodejs\pnpm.CMD
  Browsers:
    Edge: Chromium (125.0.2535.67)
  npmPackages:
    @storybook/addon-essentials: ^8.2.0-alpha.5 => 8.2.0-alpha.5
    @storybook/addon-interactions: ^8.2.0-alpha.5 => 8.2.0-alpha.5
    @storybook/addon-links: ^8.2.0-alpha.5 => 8.2.0-alpha.5
    @storybook/addon-onboarding: ^8.2.0-alpha.5 => 8.2.0-alpha.5
    @storybook/addon-webpack5-compiler-swc: ^1.0.3 => 1.0.3
    @storybook/blocks: ^8.2.0-alpha.5 => 8.2.0-alpha.5
    @storybook/react: ^8.2.0-alpha.5 => 8.2.0-alpha.5
    @storybook/react-webpack5: ^8.2.0-alpha.5 => 8.2.0-alpha.5
    @storybook/test: ^8.2.0-alpha.5 => 8.2.0-alpha.5
    storybook: ^8.2.0-alpha.5 => 8.2.0-alpha.5

Additional context

No response

shilman commented 2 months ago

@pciarach Can you try in 8.0.x? I have a theory about what's going wrong

pciarach commented 2 months ago

@shilman Unfortunately, the same happens on 8.0.10 :(

shilman commented 2 months ago

@pciarach well i guess the good news is that fixing this bug might be a low-hanging optimization opportunity.

Did you encounter this problem during an upgrade from an old version that did not have this problem?

pciarach commented 2 months ago

@shilman Well, sort of. We did two upgrades recently (7.0.7 => 7.16.7 => 8.1.5) but I've just gone back in the git history and it seems that both previous versions we had were affected as well so it seems that we didn't notice that before.

shilman commented 2 months ago

@pciarach Thanks for the context. This might be a known problem with no easy fix, but I'll ask our local Webpack experts and we'll see what we can figure out. @ndelangen @valentinpalkovic @tmeasday ?

tmeasday commented 2 months ago

@shilman do you have a reproduction repo of this behaviour?

pciarach commented 2 months ago

@tmeasday This happens even on the clean sandbox repository (npx storybook@next sandbox with React latest + webpack 5 template). Please note that this second build I'm referring to is quite fast on this repo but [my assumption is that it] increases in proportion to number of stories.

tmeasday commented 2 months ago

@pciarach I'm not sure what I am looking for. I did:

  1. Create a sandbox with react-webpack/18-ts
  2. Ran yarn storybook
  3. It opened a browser with the welcome screen. I was watching the terminal and it didn't print anything else / there was no further activity in the browser.
pciarach commented 2 months ago

@tmeasday This is what I see in the output and in the browser:

image

It's less than one second there (as there are only 3 stories :D) but on the bigger repo it is almost as long as the first build: image

tmeasday commented 2 months ago

@pciarach OK I see what you mean. Does adding this to main.js help with the problem for you?

  webpackFinal: (config) => ({
    ...config,
    watchOptions: {
      ignored: /(node_modules|storybook-config-entry|storybook-stories)/,
    },
  }),

It feels like this is a bug in https://github.com/sysgears/webpack-virtual-modules - @ndelangen any ideas?

pciarach commented 2 months ago

@tmeasday Yes, it also helps (and it's definitely better workaround than my physical files ^^)

ndelangen commented 2 months ago

@tmeasday I've been thinking of removing the need to have https://github.com/sysgears/webpack-virtual-modules for some time.

I think if we performed code injection into the html template we could likely perform the same task, but without the "magic". Not sure how much this comment helps us, right now. But for reference, we do not have anything like virtual modules on the manager's side, because we do this: https://github.com/storybookjs/storybook/blob/94cb5fca85adc5579986c8447d8c681f862f0fc6/code/builders/builder-manager/templates/template.ejs#L77-L85 that + prebundling + globalization

To solve the immediately issue, we could easily add the following to the default webpack config:

  webpackFinal: (config) => ({
    ...config,
    watchOptions: {
      ignored: /(node_modules|storybook-config-entry|storybook-stories)/,
    },
  }),

That feels rather safe to me.

tmeasday commented 2 months ago

@ndelangen the idea of getting rid of virtual modules seems good. But I'm not sure how it would work with Webpack's HMR?

ndelangen commented 2 weeks ago

The virtual modules never HMR, AFAIK?

But we should handle those changes manually via the serverChannel?

tmeasday commented 2 weeks ago

They do because the storybook-stories file (or the list of files in your code snippet above) can change. Although TBH I don't actually remember how it works in webpack right now (how do we tell it that the virtual module needs to HMR?).

But we should handle those changes manually via the serverChannel?

In this case we do send a signal that there's a new CSF file over the serverChannel, which triggers the client to re-download the index.json. Ultimately though if the user browses to the story we are going to import('./path/to/csf/file.ts') -- which isn't going to work unless the list of files is updated to include it.

I guess what you are suggesting is that rather than import()-ing via webpack's mechanism, we would import '...' it via native HTML and a script tag?