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

[Vite 3] HMR doesn't work on first dev #8719

Closed
7 tasks done
brillout opened this issue Jun 22, 2022 · 9 comments · Fixed by #8748
Closed
7 tasks done

[Vite 3] HMR doesn't work on first dev #8719

brillout opened this issue Jun 22, 2022 · 9 comments · Fixed by #8748

Comments

@brillout
Copy link
Contributor

Describe the bug

HMR doesn't work the first time the dev server is started.

Reproduction

https://github.com/brillout/vite-3-bug_hmr-first-dev

System Info

System:
    OS: Linux 5.10 Debian GNU/Linux 10 (buster) 10 (buster)
    CPU: (2) x64 Intel(R) Celeron(R) N4020 CPU @ 1.10GHz
    Memory: 2.71 GB / 2.71 GB
    Container: Yes
    Shell: 5.0.3 - /bin/bash
  Binaries:
    Node: 18.0.0 - ~/.config/nvm/versions/node/v18.0.0/bin/node
    Yarn: 1.22.17 - /usr/local/bin/yarn
    npm: 8.6.0 - ~/.config/nvm/versions/node/v18.0.0/bin/npm
  npmPackages:
    @vitejs/plugin-react: 2.0.0-beta.0 => 2.0.0-beta.0 
    vite: 3.0.0-beta.0 => 3.0.0-beta.0

Used Package Manager

pnpm

Logs

No response

Validations

@brillout
Copy link
Contributor Author

Also isn't the following surprising?

3:06:09 PM [vite] ✨ new dependencies optimized: .pnpm/vite@3.0.0-beta.0/node_modules/vite/dist/client/client.mjs

Maybe I'm missing something here, but I've doubts this is the first impression Vite wants to give to users.

@brillout
Copy link
Contributor Author

Also isn't the following surprising?

3:06:09 PM [vite] ✨ new dependencies optimized: .pnpm/vite@3.0.0-beta.0/node_modules/vite/dist/client/client.mjs

Maybe I'm missing something here, but I've doubts this is the first impression Vite wants to give to users.

Actually, I'm seeing that this message isn't shown in other examples. Not sure why it's shown?

@patak-dev
Copy link
Member

@brillout people switch to vite@3.0.0-beta.1, the client error you show here looks resolved there.

The HMR issue still persists though. Trying to understand what is going on

@sapphi-red
Copy link
Member

sapphi-red commented Jun 23, 2022

HMR one happens with 3.0.0-alpha.10 but does not happen with 3.0.0-alpha.9. (tried with plugin-react@2.0.0-alpha.3)
So something in https://github.com/vitejs/vite/blob/v3.0.0-alpha.10/packages/vite/CHANGELOG.md#300-alpha10-2022-06-10 seems to be causing.

@patak-dev
Copy link
Member

Thanks @sapphi-red, looks like the first PR where it fails is:

There is a refactoring and a fix on it, but at least we are getting closer.

@patak-dev
Copy link
Member

@brillout this should be fixed by:

vite-plugin-ssr is currently adding all pages to optimizeDeps.entries. This may not be the best strategy in v3. The entries listed here will be processed and the initial request will be blocked until all the pages are crawled. Every dep that isn't in a lazy loaded chunk will be found but at the cost of a slower cold start.
Maybe this should be optional. Some users may prefer a faster cold start, and not care about a quick reload when navigating to other pages (only the first time these are navigated)

@brillout
Copy link
Contributor Author

👍 Neat.

This may not be the best strategy in v3.

Makes sense. Thanks a lot for the hint; I've changed vite-plugin-ssr to only set optimizeDeps.entries for CI.

(Btw. I'm wondering whether these messages ✨ new dependencies optimized: react-dom/client, react are stilled needed. I'd assume that most users don't care about these. Although I think it makes sense to show a message when the browser reloads.)

@patak-dev
Copy link
Member

We are only showing the message now when there is a reload (IIRC). Why would you want to set optimizeDeps.entries for CI? Seems better to let it work like the user will experience? Or maybe it makes your tests more difficult as you need to wait for a potential reload when you navigate?

@brillout
Copy link
Contributor Author

Or maybe it makes your tests more difficult as you need to wait for a potential reload when you navigate?

This.

It's one reason I've opened #8721 (but feel free to ignore it :-)). But the primary motivation is the subpar end-user DX: the user is wondering why the browser reloads and may think "something's wrong with my app, why is my app reloading at the beginning?".

@github-actions github-actions bot locked and limited conversation to collaborators Jul 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants