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

chore: backport fixes #917

Merged
merged 3 commits into from
May 29, 2024
Merged

chore: backport fixes #917

merged 3 commits into from
May 29, 2024

Conversation

dominikg
Copy link
Member

these 2 patches are good to have in v3 too

dominikg added 2 commits May 25, 2024 14:52
* feat: disable hmr when running in vitest by default

* refactor: use vite server.hmr config instead that is set by vitest

* fix: enforce hmr false, update changeset

(cherry picked from commit f7409c8)
…process (#912)

* fix: ensure vite config is only resolved once

* fix: add back inlined function to please ts

(cherry picked from commit 1211f97)
@dominikg
Copy link
Member Author

had to refactor the hmr one slightly due to the changes in v4 in the options, but spirit is identical

const hot =
!viteConfig.isProduction &&
!preResolveOptions.isBuild &&
viteConfig.server &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something I just noticed but not a big deal to change. It looks like viteConfig.server is always truthy, but this line implies otherwise, which could be confusing because we don't want to disable HMR if the server is not configured (HMR would've been enabled by default in dev)

*/
function enforceOptionsForHmr(options) {
function enforceOptionsForHmr(options, viteConfig) {
if (options.hot && viteConfig.server?.hmr === false) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ?. seem unnecessary too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants