Skip to content

fix(hmr): run HMR handler sequentially#19793

Merged
patak-cat merged 3 commits into
vitejs:mainfrom
sapphi-red:fix/hmr-wait-for-event-handlers
Apr 11, 2025
Merged

fix(hmr): run HMR handler sequentially#19793
patak-cat merged 3 commits into
vitejs:mainfrom
sapphi-red:fix/hmr-wait-for-event-handlers

Conversation

@sapphi-red

Copy link
Copy Markdown
Member

Description

This PR aims to fix the following flaky fail:

FAIL playground/hmr/tests/hmr.spec.ts > invalidate works with multiple tabs
AssertionError: expected '>>> vite:beforeUpdate -- update' to match '>>> vite:invalidate -- /invalidation/…'
Expected: ">>> vite:invalidate -- /invalidation/child.js"
Received: ">>> vite:beforeUpdate -- update"
https://github.com/vitejs/vite/actions/runs/13994683432/job/39186785003#step:12:214

This fail was happening because multiple HMR event was processed concurrently.
This PR makes the HMR event handler to run sequentially. This makes the HMR behavior in browsers more aligned with the module runner.

I tested this locally with the following test:

  test.only('invalidate works with multiple tabs', async () => {
    const pages: Page[] = []
    try {
      for (let i = 0; i < 10; i++) {
        const p = await browser.newPage()
        await p.goto(viteTestUrl)
        pages.push(p)
      }

      for (let i = 0; i < 30; i++) {
        browserLogs.length = 0
        console.log('update!')

        const el = await page.$('.invalidation-parent')
        await untilBrowserLogAfter(
          () =>
            editFile('invalidation/child.js', (code) =>
              code.replace('child', 'child updated'),
            ),
          [
            '>>> vite:beforeUpdate -- update',
            '>>> vite:invalidate -- /invalidation/child.js',
            '[vite] invalidate /invalidation/child.js',
            '[vite] hot updated: /invalidation/child.js',
            '>>> vite:afterUpdate -- update',
            // if invalidate dedupe doesn't work correctly, this beforeUpdate will be called twice
            '>>> vite:beforeUpdate -- update',
            '(invalidation) parent is executing',
            '[vite] hot updated: /invalidation/parent.js',
            '>>> vite:afterUpdate -- update',
          ],
          true,
        )
        await untilUpdated(() => el.textContent(), 'child updated')
        console.log('update done!')
        await new Promise((r) => setTimeout(r, 10))
        editFile('invalidation/child.js', (code) =>
          code.replace('child updated', 'child'),
        )
        await untilUpdated(() => el.textContent(), /^child$/)
        await new Promise((r) => setTimeout(r, 30))
      }
    } finally {
      await Promise.all(pages.map((p) => p.close()))
    }
  })

refs #16307

@sapphi-red sapphi-red added feat: hmr p2-edge-case Bug, but has workaround or limited in scope (priority) labels Apr 4, 2025
@sapphi-red

Copy link
Copy Markdown
Member Author

/ecosystem-ci run

@pkg-pr-new

pkg-pr-new Bot commented Apr 11, 2025

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/vite@19793

commit: eecb6e4

@vite-ecosystem-ci

Copy link
Copy Markdown

📝 Ran ecosystem CI on eecb6e4: Open

suite result latest scheduled
laravel failure success
storybook failure failure
vite-environment-examples success failure
vike success failure
vite-plugin-react success failure
waku success failure

nuxt, quasar, histoire, ladle, previewjs, marko, analogjs, sveltekit, rakkas, vite-plugin-vue, astro, qwik, vitest, vite-plugin-pwa, vitepress, unocss, vite-plugin-react-swc, react-router, vite-setup-catalogue, vite-plugin-svelte, vuepress, vite-plugin-cloudflare

@patak-cat patak-cat left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we can merge this one in the minor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat: hmr p2-edge-case Bug, but has workaround or limited in scope (priority) trigger: preview

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants