[Bug]: Dynamic import trigger infinite preview reload in production mode

yoriiis commented 2 months ago

Describe the bug

Hello the Storybook community!

We use Storybook server at our company to display Twig component from a Symfony application in a Storybook.

It appears since the v7 that dynamic import in a component break the Storybook preview. The preview is infinitely reloading in production mode only. The following video capture present the infinite preview reload.

https://github.com/storybookjs/storybook/assets/2563298/af0b8d35-0f2d-45bb-93b1-e4fcdc4a6b1a

In development mode, the DevTools (console tabs) with the JavaScript context set to the iframe (component preview) display the following error. The dynamic import is not executed.

The content of the component itself is working correctly (even the dynamic import) in standalone mode, without the Storybook.

Are these two errors related? Errors in development mode would trigger an infinite reload in production mode? How can we fix this error?

Note: I've tested a dynamic import in a other Storybook on a React application (without Storybook server/Twig/Symfony) and the dynamic import works correctly.

The reproductible repository is linked to the issue and has a README.md with instructions on how to reproduces errors describe in this issue.

Thanks!

Reproduction link

https://github.com/yoriiis/storybook-server-dynamic-import

Reproduction steps

Clone the reproductible repository
Use the Node.js version from the .nvmrc file
Install dependencies with npm install

To reproduce infinite preview reload in production mode

Start terminal 1 with the command: npm run build:webpack
Start terminal 2 with the command: npm run build:storybook
Start terminal 3 with the command: npm run start:server:app
Start terminal 4 with the command: npm run start:server:storybook
Open http://127.0.0.1:3001 to view the Storybook public build
The Storybook preview is infinitely reloading like the video above

To reproduce error in development mode

Start terminal 1 with the command: npm run start:webpack
Start terminal 2 with the command: npm run start:server:app
Start terminal 3 with the command: npm run start:storybook
Open http://localhost:6006 to view the Storybook in development mode
The Storybook preview display an error in the DevTools console like the image above

To test the component itself in standalone mode

Start terminal 1 with the command: npm run start:webpack
Start terminal 2 with the command: npm run start:server:app
Open http://127.0.0.1:3000/demo to view the component itself in standalone mode
The component preview is working correctly with the dynamic import

System

System:
  OS: macOS 14.4.1
  CPU: (8) arm64 Apple M1
  Shell: 5.2.26 - /opt/homebrew/bin/bash
Binaries:
  Node: 20.14.0 - ~/.nvm/versions/node/v20.14.0/bin/node
  npm: 10.7.0 - ~/.nvm/versions/node/v20.14.0/bin/npm <----- active
Browsers:
  Chrome: 126.0.6478.127
  Edge: 126.0.2592.87
  Safari: 17.4.1
npmPackages:
  @storybook/addon-webpack5-compiler-babel: ^3.0.3 => 3.0.3 
  @storybook/preview-api: 8.1.11 => 8.1.11 
  @storybook/server-webpack5: 8.1.11 => 8.1.11 
  storybook: 8.1.11 => 8.1.11

Additional context

Note: if the dynamic import is replaced by a basic import at the top of the file, it is executed correctly and there is no infinite reloading.

tmeasday commented 2 days ago

@yoriiis thank you for the reproduction

So the issue here is the dynamic import of module.js -- this is not bundled by your (external webpack setup), leading to the browser trying to import() from your code:

https://github.com/yoriiis/storybook-server-dynamic-import/blob/159c86a6625a7e33ca93a3873d3c3ddcbef4cd0b/src/demo/demo.js#L8

This cannot work, because the browser is running in Storybook (localhost:6006) but the server is on port 3000.

The reason that code executes at all is due to the simulatePageLoad() function, called by the server, html and web-component frameworks:

https://github.com/storybookjs/storybook/blob/c7b03095eab43badb28e9d8c7793427f8caba58b/code/core/src/preview-api/modules/preview-web/simulate-pageload.ts#L83-L84

What happen is:

You visit a story at http://localhost:6006/...
That story pulls the HTML from http://localhost:3000/...
The <script> tag in that HTML is injected into the SB browser context by simulatePageLoad
That script tries to do an import() -- this doesn't work.

As to why it infinite loops in production, I am not sure, but if you pause execution you'll see it's the same root problem as in development.

yoriiis commented 2 days ago

Hello @tmeasday thanks for the feedback.

I realise that the demo is probably not suitable. We're using a Symfony application with storybook-server. The Storybook displays the component in an iframe and fetches the content from a route on the site that renders a Twig component.

In production mode, there's no longer any question of a conflict on the port as you say.

Shouldn't a component be able to execute a dynamic import?

Let me know if you need me to create a demo with Symfony and storybook-server.

tmeasday commented 1 day ago

Shouldn't a component be able to execute a dynamic import?

Yes, a component can, but not if it is rendered to html and fetched via Storybook server, which isn't super complicated and is designed for more simple multipage app situations.

Let me know if you need me to create a demo with Symfony and storybook-server

I'm not quite sure I understand how it is different, but I'm happy to take a look if you have one!

yoriiis commented 1 day ago

Yes, a component can, but not if it is rendered to html and fetched via Storybook server.

Interesting, so is there a limit? How do you think this case can be managed?

We have components that sometimes inherit from global code that does dynamic imports. Perhaps they could be ignored to avoid the infinite reload error?

which isn't super complicated and is designed for more simple multipage app situations.

Sorry, I didn't understand that part of your comment.

tmeasday commented 1 day ago

@yoriiis -- I guess my first question is -- what is stopping you from testing your components directly in Storybook, rather than this round-about route using the server framework?

which isn't super complicated and is designed for more simple multipage app situations. Sorry, I didn't understand that part of your comment.

Well @storybook/server-webpack5 is designed for traditional, server rendered multi-page (as opposed to single-page) applications, rendered by server-side frameworks like Ruby on Rails.

In such cases, typically the JS served is pretty simple and doesn't do things like dynamically import other JS files. For such cases the simulatePageLoad() approach tends to do the job.

Obviously the world is complicated but my initial reaction is if your JS is complex, it would make sense to use on of the many Storybook JS frameworks (rather than the non-JS one :) ) to render your components, if they are rendered by that JS.

Another approach which is more similar to the setup you have in that reproduction would be to force your own webpack build to bundle everything and get rid of the dynamic imports.

yoriiis commented 1 day ago

Thanks @tmeasday for your feedback and your time!

what is stopping you from testing your components directly in Storybook, rather than this round-about route using the server framework?

We only use Storybook to test our components. As they are rendered in Twig by a Symfony application, Storybook displays the HTML rendering of the component (in its iframe)

GromNaN, with whom I've worked, has written an article on this subject, if that provides any context: Dev.to: Rendering Twig templates in Storybook

The fact that components inherit global code (which sometimes contains dynamic imports) is a website prerequisite and we have to work with that. The problem is that the Storybook is unusable if the components try to execute a dynamic import.

I'm going to create another demo on the Symfony/Storybook stack so you can see what I'm exposing.

The idea of bundling everything and including dynamic imports for the Storybook is interesting, but I don't know if it's possible with webpack 🤔

I'd also thought of magic comment, which has a case for ignore (/* webpackIgnore: true */).

tmeasday commented 18 hours ago

@yoriiis I think if you wanted to support more complex JS in your server rendered stuff you'd need to take the SB server project further. I can see two angles to go with:

Trying to be smarter in our execution of JS from the HTML. For instance we could search for import("....") and replace it with... something... that works from the SB context. I'd have to play with it -- I'm thinking a simple import("http://localhost:3000/module.js") would probably not work, but, I could be wrong. Either way seems like kind of a fraught process.
A different approach where the "story" renders an iframe containing the html directly. The challenge here would be then you'd have to inject the SB runtime into the HTML rendered from your Symphony server.

We actually prototyped an approach like this for next js here: https://github.com/storybookjs/pages-nextjs-server -- it's not as easy to do as you might like though unfortunately (the "SB runtime" isn't that easy to just inject somewhere).

yoriiis commented 7 hours ago

Hello @tmeasday,

I found an alternative by modifying the webpack configuration so that dynamic imports are handled differently in the Storybook context.

With the module.parser.javascript.dynamicImportMode = 'eager' option, they will be included in the initial bundle and this no longer generates the infinite reload error.

https://webpack.js.org/configuration/module/#moduleparserjavascriptdynamicimportmode

Thank you for your investigation.

Since there is a limitation with storybook-server and dynamic imports, would it be interesting to mention it somewhere in the documentation?

Or do you think it's a bug that could be fixed by the Storybook or webpack team? (It seems to me that the bug wasn't present on v6).

storybookjs / storybook