fix: correctly resolve hmr dep ids and fallback to url (#18840)

Co-authored-by: 翠 / green <[email protected]>

packages/vite/src/node/plugins/importAnalysis.ts

@@ -321,7 +321,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
         url: string,
         pos: number,
         forceSkipImportAnalysis: boolean = false,
-      ): Promise<[string, string]> => {
+      ): Promise<[string, string | null]> => {
         url = stripBase(url, base)
 
         let importerFile = importer
@@ -355,7 +355,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
         if (!resolved || resolved.meta?.['vite:alias']?.noResolved) {
           // in ssr, we should let node handle the missing modules
           if (ssr) {
-            return [url, url]
+            return [url, null]
           }
           // fix#9534, prevent the importerModuleNode being stopped from propagating updates
           importerModule.isSelfAccepting = false
@@ -396,32 +396,35 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
               url = injectQuery(url, versionMatch[1])
             }
           }
+        }
 
+        try {
+          // delay setting `isSelfAccepting` until the file is actually used (#7870)
+          // We use an internal function to avoid resolving the url again
+          const depModule = await moduleGraph._ensureEntryFromUrl(
+            unwrapId(url),
+            canSkipImportAnalysis(url) || forceSkipImportAnalysis,
+            resolved,
+          )
           // check if the dep has been hmr updated. If yes, we need to attach
           // its last updated timestamp to force the browser to fetch the most
           // up-to-date version of this module.
-          try {
-            // delay setting `isSelfAccepting` until the file is actually used (#7870)
-            // We use an internal function to avoid resolving the url again
-            const depModule = await moduleGraph._ensureEntryFromUrl(
-              unwrapId(url),
-              canSkipImportAnalysis(url) || forceSkipImportAnalysis,
-              resolved,
-            )
-            if (depModule.lastHMRTimestamp > 0) {
-              url = injectQuery(url, `t=${depModule.lastHMRTimestamp}`)
-            }
-          } catch (e: any) {
-            // it's possible that the dep fails to resolve (non-existent import)
-            // attach location to the missing import
-            e.pos = pos
-            throw e
+          if (
+            environment.config.consumer === 'client' &&
+            depModule.lastHMRTimestamp > 0
+          ) {
+            url = injectQuery(url, `t=${depModule.lastHMRTimestamp}`)
           }
-
-          // prepend base
-          if (!ssr) url = joinUrlSegments(base, url)
+        } catch (e: any) {
+          // it's possible that the dep fails to resolve (non-existent import)
+          // attach location to the missing import
+          e.pos = pos
+          throw e
         }
 
+        // prepend base
+        if (!ssr) url = joinUrlSegments(base, url)
+
         return [url, resolved.id]
       }
 
@@ -547,7 +550,8 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
             }
 
             // normalize
-            const [url, resolvedId] = await normalizeUrl(specifier, start)
+            let [url, resolvedId] = await normalizeUrl(specifier, start)
+            resolvedId = resolvedId || url
 
             // record as safe modules
             // safeModulesPath should not include the base prefix.
@@ -751,9 +755,40 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
       // normalize and rewrite accepted urls
       const normalizedAcceptedUrls = new Set<string>()
       for (const { url, start, end } of acceptedUrls) {
-        const [normalized] = await moduleGraph.resolveUrl(toAbsoluteUrl(url))
+        let [normalized, resolvedId] = await normalizeUrl(url, start).catch(
+          () => [],
+        )
+        if (resolvedId) {
+          const mod = moduleGraph.getModuleById(resolvedId)
+          if (!mod) {
+            this.error(
+              `module was not found for ${JSON.stringify(resolvedId)}`,
+              start,
+            )
+            return
+          }
+          normalized = mod.url
+        } else {
+          try {
+            // this fallback is for backward compat and will be removed in Vite 7
+            const [resolved] = await moduleGraph.resolveUrl(toAbsoluteUrl(url))
+            normalized = resolved
+            if (resolved) {
+              this.warn({
+                message:
+                  `Failed to resolve ${JSON.stringify(url)} from ${importer}.` +
+                  ' An id should be written. Did you pass a URL?',
+                pos: start,
+              })
+            }
+          } catch {
+            this.error(`Failed to resolve ${JSON.stringify(url)}`, start)
+            return
+          }
+        }
         normalizedAcceptedUrls.add(normalized)
-        str().overwrite(start, end, JSON.stringify(normalized), {
+        const hmrAccept = normalizeHmrUrl(normalized)
+        str().overwrite(start, end, JSON.stringify(hmrAccept), {
           contentOnly: true,
         })
       }

playground/hmr/__tests__/hmr.spec.ts

@@ -779,6 +779,29 @@ if (!isBuild) {
     }, '[wow]1')
   })
 
+  test('handle virtual module accept updates', async () => {
+    await page.goto(viteTestUrl)
+    const el = await page.$('.virtual-dep')
+    expect(await el.textContent()).toBe('0')
+    editFile('importedVirtual.js', (code) => code.replace('[success]', '[wow]'))
+    await untilUpdated(async () => {
+      const el = await page.$('.virtual-dep')
+      return await el.textContent()
+    }, '[wow]')
+  })
+
+  test('invalidate virtual module and accept', async () => {
+    await page.goto(viteTestUrl)
+    const el = await page.$('.virtual-dep')
+    expect(await el.textContent()).toBe('0')
+    const btn = await page.$('.virtual-update-dep')
+    btn.click()
+    await untilUpdated(async () => {
+      const el = await page.$('.virtual-dep')
+      return await el.textContent()
+    }, '[wow]2')
+  })
+
   test('keep hmr reload after missing import on server startup', async () => {
     const file = 'missing-import/a.js'
     const importCode = "import 'missing-modules'"

playground/hmr/hmr.ts

@@ -1,4 +1,5 @@
 import { virtual } from 'virtual:file'
+import { virtual as virtualDep } from 'virtual:file-dep'
 import { foo as depFoo, nestedFoo } from './hmrDep'
 import './importing-updated'
 import './file-delete-restore'
@@ -14,17 +15,29 @@ text('.app', foo)
 text('.dep', depFoo)
 text('.nested', nestedFoo)
 text('.virtual', virtual)
+text('.virtual-dep', virtualDep)
 text('.soft-invalidation', softInvalidationMsg)
 setImgSrc('#logo', logo)
 setImgSrc('#logo-no-inline', logoNoInline)
 
+text('.virtual-dep', 0)
+
 const btn = document.querySelector('.virtual-update') as HTMLButtonElement
 btn.onclick = () => {
   if (import.meta.hot) {
     import.meta.hot.send('virtual:increment')
   }
 }
 
+const btnDep = document.querySelector(
+  '.virtual-update-dep',
+) as HTMLButtonElement
+btnDep.onclick = () => {
+  if (import.meta.hot) {
+    import.meta.hot.send('virtual:increment', '-dep')
+  }
+}
+
 if (import.meta.hot) {
   import.meta.hot.accept(({ foo }) => {
     console.log('(self-accepting 1) foo is now:', foo)
@@ -55,6 +68,10 @@ if (import.meta.hot) {
     handleDep('single dep', foo, nestedFoo)
   })
 
+  import.meta.hot.accept('virtual:file-dep', ({ virtual }) => {
+    text('.virtual-dep', virtual)
+  })
+
   import.meta.hot.accept(['./hmrDep'], ([{ foo, nestedFoo }]) => {
     handleDep('multi deps', foo, nestedFoo)
   })

playground/hmr/index.html

@@ -6,6 +6,7 @@
   />
 </div>
 <button class="virtual-update">update virtual module</button>
+<button class="virtual-update-dep">update virtual module via accept</button>
 
 <script type="module" src="./invalidation/root.js"></script>
 <script type="module" src="./hmr.ts"></script>
@@ -24,6 +25,7 @@
 <div class="custom"></div>
 <div class="toRemove"></div>
 <div class="virtual"></div>
+<div class="virtual-dep"></div>
 <div class="soft-invalidation"></div>
 <div class="invalidation-parent"></div>
 <div class="invalidation-root"></div>

playground/hmr/modules.d.ts

@@ -1,3 +1,7 @@
 declare module 'virtual:file' {
   export const virtual: string
 }
+
+declare module 'virtual:file-dep' {
+  export const virtual: string
+}

playground/hmr/vite.config.ts

@@ -45,23 +45,22 @@ function virtualPlugin(): Plugin {
   return {
     name: 'virtual-file',
     resolveId(id) {
-      if (id === 'virtual:file') {
-        return '\0virtual:file'
+      if (id.startsWith('virtual:file')) {
+        return '\0' + id
       }
     },
     load(id) {
-      if (id === '\0virtual:file') {
+      if (id.startsWith('\0virtual:file')) {
         return `\
 import { virtual as _virtual } from "/importedVirtual.js";
 export const virtual = _virtual + '${num}';`
       }
     },
     configureServer(server) {
-      server.environments.client.hot.on('virtual:increment', async () => {
-        const mod =
-          await server.environments.client.moduleGraph.getModuleByUrl(
-            '\0virtual:file',
-          )
+      server.environments.client.hot.on('virtual:increment', async (suffix) => {
+        const mod = await server.environments.client.moduleGraph.getModuleById(
+          '\0virtual:file' + (suffix || ''),
+        )
         if (mod) {
           num++
           server.environments.client.reloadModule(mod)