storybookjs / storybook

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

Top-level `await` in stories break them. Storybook 6.3, Webpack 5 #15129

Closed LinuCC closed 1 year ago

LinuCC commented 3 years ago

Describe the bug If a top-level await is used inside a .stories.tsx, the stories break - they are not being displayed in storybook anymore.

To Reproduce Add storybook with webpack5 builder to a React Project, start it:

npx sb@next init --builder webpack5
npm i
npm run storybook

=> The default example components are being displayed, including Button.

Change Button.stories.tsx to contain a top-level await:

import React from 'react';
import { Story, Meta } from '@storybook/react';
import { Button, ButtonProps } from './Button';

await Promise.resolve();

export default {
  title: 'Example/Button',
  component: Button,
  argTypes: {
    backgroundColor: { control: 'color' },
  },
} as Meta;

// [...]

=> The component Button is not being displayed anymore in storybook, direct URL access does not work anymore, too. It is gone.

Why is this important?

Using Webpack 5, we can use module federation to create a federated Storybook; A storybook dynamically referencing components from other projects. This would make integrating components from other microfrontends much easier. The standard way to import these federated components is with a top-level await like this:

const Button = (await import('app2/Button')).Button;

System

╰❯ npx sb@next info                                                                                                                                                                                                              130 ↵

Environment Info:

  System:
    OS: macOS 11.4
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
  Binaries:
    Node: 14.15.1 - ~/.asdf/installs/nodejs/14.15.1/bin/node
    Yarn: 1.22.10 - ~/.asdf/installs/nodejs/14.15.1/bin/yarn
    npm: 6.14.8 - ~/.asdf/installs/nodejs/14.15.1/bin/npm
  Browsers:
    Chrome: 91.0.4472.77
    Firefox: 88.0.1
    Safari: 14.1.1

Happy to repro a repo using Webpack module federation if it is more complicated than just the top-level await. Thanks for the help.

zgottlieb commented 3 years ago

Were you able to sort this out? I am experiencing the same issue. I am using Storybook 6.3 and Webpack 5.40, and I am trying to import a federated module / component into my host Storybook module using top-level await. My implementation suffers the same failure to render the story, despite the local Storybook app loading fine otherwise.

When I import a federated module / component into a story, such as const Button = (await import('common/components/Button')).default; I am able to log the imported function to the console, and I am also to click that to link to the corresponding code in Chrome Dev Tools. That links to the correct file and code, which is imported from a remote module, so it appears things are wired up correctly. The problem is that the entire story fails to render or even appear in the sidebar, and there are no errors.

Hoping someone can help clarify what the issue might be.

This is my .storybook/main.js file, for reference:


module.exports = {
  stories: ['../stories/**/*.stories.mdx', '../stories/**/*.stories.@(js|jsx|ts|tsx)'],
  addons: ['@storybook/addon-links', '@storybook/addon-essentials'],
  core: {
    builder: 'webpack5',
  },
  previewHead: (head) => `
    ${head}
    <script
      data-webpack="common"
      src="http://localhost:3001/_next/static/runtime/remoteEntry.js"
    ></script>
  `,
  webpackFinal: async (config) => {
    config.plugins.push(new ModuleFederationPlugin({
      name: 'storybook',
      library: { type: 'var', name: 'storybook'},
      filename: 'static/runtime/remoteEntry.js',
      remotes: {
        common: `common`,
      },
    }));

    config.experiments = {
      ...config.experiments,
      topLevelAwait: true
    }

    return config;
  }
};
zgottlieb commented 3 years ago

@LinuCC, I think I managed to successfully load a component asynchronously! Rather than do the top-level await, I loaded my remote component by doing the following in my story file:

import React from 'react';

export default {
  title: 'Example/RemoteButton',
};

export const MyStory = (args, {loaded: { Component }}) => {
  return <Component {...args}>Remote Button</Component>
};
MyStory.loaders = [async () => ({ Component: (await import('common/components/button')).default })];
FrozenKiwi commented 3 years ago

I've traced the root cause of this (I think... - I'm not familiar with either webpack or storybook, so salty expectations etc)

In the loading code for storybook there is a loop to load all the stories:

    if (reqs) {
      reqs.forEach(function (req) {
        req.keys().forEach(function (filename) {
          try {
            var fileExports = req(filename);
            currentExports.set(fileExports, // t

For a regular module, the result looks like: image

For a top-level await, the result of the require is a promise image

That's probably why the workaround above works. Unfortunately, in my case the top-level await is actually in another module, quite far down the import tree. Will keep digging.

Top-level await is a killer feature... It's very painful to find it breaking things here.

MichaelAllenWarner commented 3 years ago

Support for top-level await would be most welcome. Without it, I believe we are constrained to put all asynchronous code in loaders—not the end of the world but certainly inconvenient.

robdonn commented 3 years ago

While having top-level await would be useful for so many things, there is now a library that enables the use of module federation in Storybook. Check out the storybook-host example in the repo to see an example of creating stories for components imported from remote apps without requiring top-level await.

https://github.com/robdonn/storybook-module-federation

shilman commented 3 years ago

@tmeasday is this something we're in a position to provide with the rearchitecture?

tmeasday commented 3 years ago

I'm guessing it would work out of the box with the v7 story store - or could be made to work easily.

For top-level await, does await import() both await the import and the top-level await? If so, I think it probably works already.

For the v6 story store, it would be a bit more complicated. We could probably make it work, but it would break storyshots, for one thing. So we couldn't do it in a backwards compatible way.

shilman commented 3 years ago

Great. I'm making a note to tag this issue in the V7 story store PR!

MichaelAllenWarner commented 3 years ago

For what it's worth: if there were an option to turn on top-level await in v6 at the cost of breaking storyshots, I would use it.

I do a lot of front-end work for Drupal projects, and my use-case (which I suspect may become a more common one in the next year or two) is working with Twig in Storybook. As far as I know, the best JS implementation of Twig is Twing, which renders templates as functions that return not markup-strings but promises (that resolve to markup).

I handle this in Storybook/HTML with a global async loader, where each story-function does nothing but return what the global loader passes it, and where a story's rendering logic is offloaded to a custom async .render() method (that the global loader calls). Sounds a little convoluted, but it works just fine, and most of the ugly implementation details are abstracted away.

Without top-level await, however, I have to make sure that all my rendering logic—not just the story's—gets piped through the loader. For example, if I want to generate some markup from an imported Twig template and use it in a story's args, the best I can do is use the template's props in the args, and call/await the template-function in the story's asynchronous .render() method. And then, what if one of the template's props should itself be markup generated from another template? Things can quickly get not so nice.

With top-level await, I'd still have to use async loaders for each story, but I'd be able to import a Twig template into a .stories.js file, extract some markup from it at the module-level, and use the markup directly in a story's args. Much cleaner.

tmeasday commented 3 years ago

@MichaelAllenWarner

For what it's worth: if there were an option to turn on top-level await in v6 at the cost of breaking storyshots, I would use it.

Sure, in 6.4 we will have a storyStoreV7 feature flag that opts into the new store. We may or may not figure out a way to make Storyshots work with it (ultimately we are transitioning to a new test runner, but probably not in 6.4).

I handle this in Storybook/HTML with a global async loader, where each story-function does nothing but return what the global loader passes it, and where a story's rendering logic is offloaded to a custom async .render() method

Well the actual renderToDOM function of a framework is allowed to be async, and in fact we have @storybook/server for exactly this kind of use case -- you might want to look at this for a more established pattern of what you are doing.

With top-level await, I'd still have to use async loaders for each story, but I'd be able to import a Twig template into a .stories.js file, extract some markup from it at the module-level, and use the markup directly in a story's args. Much cleaner.

That seems cool, although if you look at @storybook/server you see a different approach to generating things like args from the server.

MichaelAllenWarner commented 3 years ago

@tmeasday Thanks for the recommendation. I'll check it out!

MichaelAllenWarner commented 3 years ago

In case anyone's wondering, I can confirm that top-level await is working now in 6.4.0-beta.1, with the storyStoreV7 feature turned on (using Webpack 5's experiments.topLevelAwait). Have to include your Storybook flavor as an add-on in main.js. The a11y add-on doesn't seem to be working yet (for storyStoreV7 I think).

shilman commented 1 year ago

Closing this as fixed in 7.0 (or 6.4+ with the v7 store enabled)

Migration guide: https://storybook.js.org/migration-guides/7.0