storybook: [Bug]: 7.0.0-beta.28 HMR not working with @storybook/react-vite

Describe the bug

HMR doesn’t seem to be working with storybook react with the vite builder.

Package version: @storybook/react-vite@7.0.0-beta.28

Wondering if anyone has anymore context on why the following options are set for the vite builder dev server config: Reference: https://github.com/storybookjs/storybook/blob/v7.0.0-beta.28/code/lib/builder-vite/src/vite-server.ts#LL18-L21

hmr: {
  port: options.port,
  server: devServer,
},

It seems to work fine if the vite hmr options are removed which allows vite to initialize its own websocket server for hmr.

With the existing config, the websocket server doesn’t seem to be able to receive the upgrade event below. Reference: https://github.com/vitejs/vite/blob/v4.0.4/packages/vite/src/node/server/ws.ts#L99-L105

wsServer.on('upgrade', (req, socket, head) => {
  if (req.headers['sec-websocket-protocol'] === HMR_HEADER) {
    wss.handleUpgrade(req, socket as Socket, head, (ws) => {
      wss.emit('connection', ws, req)
    })
  }
})

To Reproduce

No response

System

Environment Info:

  System:
    OS: macOS 13.1
    CPU: (8) x64 Intel(R) Core(TM) i5-1038NG7 CPU @ 2.00GHz
  Binaries:
    Node: 19.4.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 9.2.0 - /usr/local/bin/npm
  Browsers:
    Chrome: 108.0.5359.124
    Edge: 109.0.1518.52
    Firefox: 85.0.2
    Safari: 16.2

Additional context

No response

About this issue

Original URL
State: closed
Created a year ago
Reactions: 2
Comments: 17 (10 by maintainers)

Most upvoted comments

It seems like with the new version (v7.0.0-beta.30) I get this when opening the storybook preview:

1:33:27 AM [vite] Internal server error: Failed to resolve import "@storybook/preview-api" from "../../../../../../virtual:/@storybook/builder-vite/vite-app.js". Does the file exist?
  Plugin: vite:import-analysis
  File: /virtual:/@storybook/builder-vite/vite-app.js:1:56
  1  |  import { composeConfigs, PreviewWeb, ClientApi } from '@storybook/preview-api';
     |                                                         ^
  2  |    import '/virtual:/@storybook/builder-vite/setup-addons.js';
  3  |    import { importFn } from '/virtual:/@storybook/builder-vite/storybook-stories.js';
      at formatError (file:///<projectDir>/node_modules/.pnpm/vite@4.0.4_@types+node@18.11.18/node_modules/vite/dist/node/chunks/dep-5e7f419b.js:41235:46)
      at TransformContext.error (file:///<projectDir>/node_modules/.pnpm/vite@4.0.4_@types+node@18.11.18/node_modules/vite/dist/node/chunks/dep-5e7f419b.js:41231:19)
      at normalizeUrl (file:///<projectDir>/node_modules/.pnpm/vite@4.0.4_@types+node@18.11.18/node_modules/vite/dist/node/chunks/dep-5e7f419b.js:39540:33)
      at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
      at async TransformContext.transform (file:///<projectDir>/node_modules/.pnpm/vite@4.0.4_@types+node@18.11.18/node_modules/vite/dist/node/chunks/dep-5e7f419b.js:39673:47)
      at async file:///<projectDir>/node_modules/.pnpm/vite-plugin-inspect@0.7.14_vite@4.0.4/node_modules/vite-plugin-inspect/dist/index.mjs:676:23
      at async Object.transform (file:///<projectDir>/node_modules/.pnpm/vite@4.0.4_@types+node@18.11.18/node_modules/vite/dist/node/chunks/dep-5e7f419b.js:41506:30)
      at async loadAndTransform (file:///<projectDir>/node_modules/.pnpm/vite@4.0.4_@types+node@18.11.18/node_modules/vite/dist/node/chunks/dep-5e7f419b.js:39313:29)

stevezhu on Jan 19, 2023

w00t!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.0.0-beta.30 containing PR #20654 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb@next upgrade --prerelease

Closing this issue. Please re-open if you think there’s still more to do.

shilman on Jan 18, 2023

I finally figured out the issue after an unreasonable amount of debugging on bundled vite server code 😂. The vite-plugin-externals plugin isn’t properly following the vite api for modifying config. Fix here: https://github.com/crcong/vite-plugin-externals/pull/27. After the fix is published, https://github.com/storybookjs/storybook/pull/20655 could probably be merged in to revert the addition of disableInServe: true.

Could make sense to move to any internal plugin too if the plugin is as simple as https://github.com/storybookjs/storybook/pull/20698, but otherwise this should fix the issue.

If anyone is curious, it’s because plugin configs are deeply merged by vite in the config hook.

conf = mergeConfig(conf, res)

https://github.com/vitejs/vite/blob/v4.0.4/packages/vite/src/node/config.ts#L1109

Since vite-plugin-externals was passing in the entire config to be merged, the merge code was recursively merging the hmr server from @storybook/builder-vite with itself.

hmr: {
  port: options.port,
  server: devServer,
},

https://github.com/storybookjs/storybook/blob/v7.0.0-beta.29/code/lib/builder-vite/src/vite-server.ts#L20

stevezhu on Jan 20, 2023

I sent them an email as well this morning. But yes, I think if we don’t get a response in a couple of days we’ll need to take a different approach.

IanVS on Jan 20, 2023

See #20655

thebuilder on Jan 17, 2023

¡Ay Caramba!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.0.0-beta.36 containing PR #20698 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb@next upgrade --prerelease

Closing this issue. Please re-open if you think there’s still more to do.

shilman on Jan 28, 2023

Thank you so much for your time debugging this! 🎉I will ask internally, whether we want to wait for a fix of vite-plugins-externals, or moving that plugin internally.

FYI: @ndelangen, @IanVS

valentinpalkovic on Jan 20, 2023

@valentinpalkovic @ndelangen any idea why this would now be failing in pnpm projects? I thought we had all the dependencies straightened out even before the pre-bundling, so not sure why this is breaking now.

@stevezhu does it still work correctly when building for production?

It looks like it’s related to this issue it was mentioned in https://github.com/storybookjs/builder-vite/issues/55#issuecomment-1396899350. I was using pnpm and the hoisting workaround mentioned does work for me. Seems fine with the production build.

Also trying to look into why vite-plugin-externals is breaking HMR from the websocket upgrade route.

stevezhu on Jan 19, 2023

Yes, but with pnpm (and maybe yarn pnp), it needs to also be imported in the right context. Because this is being loaded in the browser, we need to make sure it’s in the user’s package.json. I remember now, previously we imported these internal packages from the framework, which re-exported from preview-api, etc. Now we rely on the pre-bundling, which is turned off in dev now, so it’s breaking. I am guessing that it still works fine in builds.

IanVS on Jan 19, 2023

It’s literally a dependency of the vite-builder?! https://github.com/storybookjs/storybook/blob/8285f7b0badca6c798f51e8bd16463e6fd397c07/code/lib/builder-vite/package.json#L54

ndelangen on Jan 19, 2023

Also noticing that HMR broke after upgrading. I’ve been testing different versions, and seems like it broke with beta.26 - Maybe it’s caused by #20594 ?

Changing a component causes it to skip printing the invalidate message: In beta.25:

2:43:13 PM [vite] hmr update /src/modules/Footer/Footer.tsx, /src/styles/globals.css
2:43:13 PM [vite] hmr invalidate /src/modules/Footer/Footer.tsx
2:43:13 PM [vite] hmr update /src/styles/globals.css, /virtual:/@storybook/builder-vite/vite-app.js

In beta.26:

2:43:13 PM [vite] hmr update /src/modules/Footer/Footer.tsx, /src/styles/globals.css

thebuilder on Jan 17, 2023