Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Overwriting "build" object in vite config in build mode prevents use of top-level await #22223

Closed
MTRNord opened this issue Apr 23, 2023 · 5 comments · Fixed by #23123
Closed

Comments

@MTRNord
Copy link

MTRNord commented Apr 23, 2023

Describe the bug

I have the need to use a toplevel await in 2 places. The current way to support this with vite and prevent the following error is to add:

build: {
    target: 'esnext'
}

to the config. However the code of storybook deletes this on builds.

The error you get in vite with top-level awaits is:

✓ 2358 modules transformed.
rendering chunks (49)...[vite:esbuild-transpile] Transform failed with 1 error:
assets/preview-!~{00m}~.js:143144:18: ERROR: Top-level await is not available in the configured target environment ("chrome87", "edge88", "es2020", "firefox78", "safari14" + 2 overrides)

Top-level await is not available in the configured target environment ("chrome87", "edge88", "es2020", "firefox78", "safari14" + 2 overrides)
143142|      ...middlewares
143143|    ],
143144|    preloadedState: await getInitialState()
   |                    ^
143145|  });
143146|  sagaMiddleware.run(rootSaga);

For reference: vitejs/vite#6985

To Reproduce

Any storybook is fine as long you got a top-level await anywhere in the code and use vitejs to build it.

System

Environment Info:

  System:
    OS: Linux 6.2 Fedora Linux 37 (Container Image)
    CPU: (16) x64 AMD Ryzen 7 3700X 8-Core Processor
  Binaries:
    Node: 18.7.0 - /usr/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 9.3.0 - /usr/local/bin/npm
  npmPackages:
    @storybook/addon-a11y: ^7.0.6 => 7.0.6 
    @storybook/addon-essentials: ^7.0.6 => 7.0.6 
    @storybook/addon-interactions: ^7.0.6 => 7.0.6 
    @storybook/addon-links: ^7.0.6 => 7.0.6 
    @storybook/addon-styling: ^1.0.1 => 1.0.1 
    @storybook/blocks: ^7.0.6 => 7.0.6 
    @storybook/builder-vite: ^7.0.6 => 7.0.6 
    @storybook/react: ^7.0.6 => 7.0.6 
    @storybook/react-vite: ^7.0.6 => 7.0.6 
    @storybook/testing-library: ^0.0.14-next.2 => 0.0.14-next.2

Additional context

The code causing the issue are these lines

build: {
outDir: options.outputDir,
emptyOutDir: false, // do not clean before running Vite build - Storybook has already added assets in there!
sourcemap: true,
},
which replaces the user-set values for build

@MTRNord
Copy link
Author

MTRNord commented Apr 23, 2023

Using viteFinal works as a workaround. However, I still think this should be considered as a bug since it breaks expectations. I think it is sensible to expect this to work from one place instead of needing to readd it just for storybook.

async viteFinal(config) {
    // Merge custom configuration into the default config
    return mergeConfig(config, {
      // https://github.com/storybookjs/storybook/issues/22223
      build: {
        target: 'esnext'
      },
    });
  },

@Hoishin
Copy link
Contributor

Hoishin commented Jun 18, 2023

It looks like this is intentional according to this comment

// I destructure away the `build` property from the user's config object
// I do this because I can contain config that breaks storybook, such as we had in a lit project.
// If the user needs to configure the `build` they need to do so in the viteFinal function in main.js.

BUT I really think at least target option should be allowed to pass. It does not break storybook build, or if it did break there is something else wrong.

@lsmurray
Copy link

For anyone else stuck on this there are actually two places you need to set the target to esnext. optimizeDeps and build,

In your vite config you'll want the following

export default defineConfig({
  optimizeDeps: {
    esbuildOptions: {
      target: "esnext",
    },
  },
  build: {
    target: "esnext",
  },
})

In your storybook main.ts you'll want something like

const config: StorybookConfig = {
  viteFinal: (viteConfig) =>
    mergeConfig(viteConfig, {
      build: {
        target: "esnext",
      },
    }),
};

@valentinpalkovic
Copy link
Contributor

The PR, which introduced this, states it was a conscious decision not to take over the user's build option automatically, as described above in the comment. Closing this as not planned. Please refer to the workaround if you still want to override the setting.

@valentinpalkovic valentinpalkovic closed this as not planned Won't fix, can't repro, duplicate, stale Nov 16, 2023
@Asvarox
Copy link

Asvarox commented Apr 30, 2024

We just stumbled upon this bug when integrating originjs/vite-plugin-federation which requires top-level await support. The current behaviour of silently overriding the setting and failing is far from ideal DX. At the very least it should detect if that setting is defined in user's vite config and warn with possible workaround

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
6 participants