fix(hmr): dedupe virtual modules in module graph (#10144)

vitejs · Sep 19, 2022 · 71f08e7 · 71f08e7
1 parent c948e7d
commit 71f08e7
Show file tree

Hide file tree

Showing 14 changed files with 106 additions and 31 deletions.
diff --git a/packages/vite/src/node/plugins/importAnalysis.ts b/packages/vite/src/node/plugins/importAnalysis.ts
@@ -15,9 +15,7 @@ import {
   CLIENT_DIR,
   CLIENT_PUBLIC_PATH,
   DEP_VERSION_RE,
-  FS_PREFIX,
-  NULL_BYTE_PLACEHOLDER,
-  VALID_ID_PREFIX
+  FS_PREFIX
 } from '../constants'
 import {
   debugHmr,
@@ -42,7 +40,8 @@ import {
   stripBomTag,
   timeFrom,
   transformStableResult,
-  unwrapId
+  unwrapId,
+  wrapId
 } from '../utils'
 import type { ResolvedConfig } from '../config'
 import type { Plugin } from '../plugin'
@@ -330,8 +329,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
         // prefix it to make it valid. We will strip this before feeding it
         // back into the transform pipeline
         if (!url.startsWith('.') && !url.startsWith('/')) {
-          url =
-            VALID_ID_PREFIX + resolved.id.replace('\0', NULL_BYTE_PLACEHOLDER)
+          url = wrapId(resolved.id)
         }
 
         // make the URL browser-valid if not SSR
@@ -361,7 +359,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
           try {
             // delay setting `isSelfAccepting` until the file is actually used (#7870)
             const depModule = await moduleGraph.ensureEntryFromUrl(
-              url,
+              unwrapId(url),
               ssr,
               canSkipImportAnalysis(url)
             )
@@ -536,9 +534,9 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
           }
 
           // record for HMR import chain analysis
-          // make sure to normalize away base
-          const urlWithoutBase = url.replace(base, '/')
-          importedUrls.add(urlWithoutBase)
+          // make sure to unwrap and normalize away base
+          const hmrUrl = unwrapId(url.replace(base, '/'))
+          importedUrls.add(hmrUrl)
 
           if (enablePartialAccept && importedBindings) {
             extractImportedBindings(
@@ -551,7 +549,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
 
           if (!isDynamicImport) {
             // for pre-transforming
-            staticImportedUrls.add({ url: urlWithoutBase, id: resolvedId })
+            staticImportedUrls.add({ url: hmrUrl, id: resolvedId })
           }
         } else if (!importer.startsWith(clientDir)) {
           if (!importer.includes('node_modules')) {
@@ -712,10 +710,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
       // by the deps optimizer
       if (config.server.preTransformRequests && staticImportedUrls.size) {
         staticImportedUrls.forEach(({ url, id }) => {
-          url = unwrapId(removeImportQuery(url)).replace(
-            NULL_BYTE_PLACEHOLDER,
-            '\0'
-          )
+          url = removeImportQuery(url)
           transformRequest(url, server, { ssr }).catch((e) => {
             if (e?.code === ERR_OUTDATED_OPTIMIZED_DEP) {
               // This are expected errors

diff --git a/packages/vite/src/node/server/middlewares/indexHtml.ts b/packages/vite/src/node/server/middlewares/indexHtml.ts
@@ -19,19 +19,15 @@ import {
 } from '../../plugins/html'
 import type { ResolvedConfig, ViteDevServer } from '../..'
 import { send } from '../send'
-import {
-  CLIENT_PUBLIC_PATH,
-  FS_PREFIX,
-  NULL_BYTE_PLACEHOLDER,
-  VALID_ID_PREFIX
-} from '../../constants'
+import { CLIENT_PUBLIC_PATH, FS_PREFIX } from '../../constants'
 import {
   cleanUrl,
   ensureWatchedFile,
   fsPathFromId,
   injectQuery,
   normalizePath,
-  processSrcSetSync
+  processSrcSetSync,
+  wrapId
 } from '../../utils'
 import type { ModuleGraph } from '../moduleGraph'
 
@@ -144,7 +140,7 @@ const devHtmlHook: IndexHtmlTransformHook = async (
     // and ids are properly handled
     const validPath = `${htmlPath}${trailingSlash ? 'index.html' : ''}`
     proxyModulePath = `\0${validPath}`
-    proxyModuleUrl = `${VALID_ID_PREFIX}${NULL_BYTE_PLACEHOLDER}${validPath}`
+    proxyModuleUrl = wrapId(proxyModulePath)
   }
 
   const s = new MagicString(html)

diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts
@@ -12,7 +12,6 @@ import { transformRequest } from '../server/transformRequest'
 import type { InternalResolveOptions } from '../plugins/resolve'
 import { tryNodeResolve } from '../plugins/resolve'
 import { hookNodeResolve } from '../plugins/ssrRequireHook'
-import { NULL_BYTE_PLACEHOLDER } from '../constants'
 import {
   ssrDynamicImportKey,
   ssrExportAllKey,
@@ -38,7 +37,7 @@ export async function ssrLoadModule(
   urlStack: string[] = [],
   fixStacktrace?: boolean
 ): Promise<SSRModule> {
-  url = unwrapId(url).replace(NULL_BYTE_PLACEHOLDER, '\0')
+  url = unwrapId(url)
 
   // when we instantiate multiple dependency modules in parallel, they may
   // point to shared modules. We need to avoid duplicate instantiation attempts
@@ -138,7 +137,7 @@ async function instantiateModule(
       return nodeImport(dep, mod.file!, resolveOptions)
     }
     // convert to rollup URL because `pendingImports`, `moduleGraph.urlToModuleMap` requires that
-    dep = unwrapId(dep).replace(NULL_BYTE_PLACEHOLDER, '\0')
+    dep = unwrapId(dep)
     if (!isCircular(dep) && !pendingImports.get(dep)?.some(isCircular)) {
       pendingDeps.push(dep)
       if (pendingDeps.length === 1) {

diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts
@@ -25,6 +25,7 @@ import {
   DEFAULT_EXTENSIONS,
   ENV_PUBLIC_PATH,
   FS_PREFIX,
+  NULL_BYTE_PLACEHOLDER,
   OPTIMIZABLE_ENTRY_RE,
   VALID_ID_PREFIX,
   loopbackHosts,
@@ -53,10 +54,24 @@ export function slash(p: string): string {
   return p.replace(/\\/g, '/')
 }
 
-// Strip valid id prefix. This is prepended to resolved Ids that are
-// not valid browser import specifiers by the importAnalysis plugin.
+/**
+ * Prepend `/@id/` and replace null byte so the id is URL-safe.
+ * This is prepended to resolved ids that are not valid browser
+ * import specifiers by the importAnalysis plugin.
+ */
+export function wrapId(id: string): string {
+  return id.startsWith(VALID_ID_PREFIX)
+    ? id
+    : VALID_ID_PREFIX + id.replace('\0', NULL_BYTE_PLACEHOLDER)
+}
+
+/**
+ * Undo {@link wrapId}'s `/@id/` and null byte replacements.
+ */
 export function unwrapId(id: string): string {
-  return id.startsWith(VALID_ID_PREFIX) ? id.slice(VALID_ID_PREFIX.length) : id
+  return id.startsWith(VALID_ID_PREFIX)
+    ? id.slice(VALID_ID_PREFIX.length).replace(NULL_BYTE_PLACEHOLDER, '\0')
+    : id
 }
 
 export const flattenId = (id: string): string =>

diff --git a/playground/hmr/__tests__/hmr.spec.ts b/playground/hmr/__tests__/hmr.spec.ts
@@ -627,4 +627,15 @@ if (!isBuild) {
     btn = await page.$('button')
     expect(await btn.textContent()).toBe('Compteur 0')
   })
+
+  test('handle virtual module updates', async () => {
+    await page.goto(viteTestUrl)
+    const el = await page.$('.virtual')
+    expect(await el.textContent()).toBe('[success]')
+    editFile('importedVirtual.js', (code) => code.replace('[success]', '[wow]'))
+    await untilUpdated(async () => {
+      const el = await page.$('.virtual')
+      return await el.textContent()
+    }, '[wow]')
+  })
 }
diff --git a/playground/hmr/hmr.ts b/playground/hmr/hmr.ts
@@ -1,10 +1,13 @@
+// @ts-ignore
+import { virtual } from 'virtual:file'
 import { foo as depFoo, nestedFoo } from './hmrDep'
 import './importing-updated'
 
 export const foo = 1
 text('.app', foo)
 text('.dep', depFoo)
 text('.nested', nestedFoo)
+text('.virtual', virtual)
 
 if (import.meta.hot) {
   import.meta.hot.accept(({ foo }) => {

diff --git a/playground/hmr/importedVirtual.js b/playground/hmr/importedVirtual.js
@@ -0,0 +1 @@
+export const virtual = '[success]'
diff --git a/playground/hmr/index.html b/playground/hmr/index.html
@@ -19,6 +19,7 @@
 <div class="dep"></div>
 <div class="nested"></div>
 <div class="custom"></div>
+<div class="virtual"></div>
 <div class="custom-communication"></div>
 <div class="css-prev"></div>
 <div class="css-post"></div>

diff --git a/playground/hmr/vite.config.ts b/playground/hmr/vite.config.ts
@@ -19,6 +19,19 @@ export default defineConfig({
           client.send('custom:remote-add-result', { result: a + b })
         })
       }
+    },
+    {
+      name: 'virtual-file',
+      resolveId(id) {
+        if (id === 'virtual:file') {
+          return '\0virtual:file'
+        }
+      },
+      load(id) {
+        if (id === '\0virtual:file') {
+          return 'import { virtual } from "/importedVirtual.js"; export { virtual };'
+        }
+      }
     }
   ]
 })
diff --git a/playground/ssr-html/__tests__/ssr-html.spec.ts b/playground/ssr-html/__tests__/ssr-html.spec.ts
@@ -1,7 +1,7 @@
 import fetch from 'node-fetch'
 import { describe, expect, test } from 'vitest'
 import { port } from './serve'
-import { page } from '~utils'
+import { editFile, isServe, page, untilUpdated } from '~utils'
 
 const url = `http://localhost:${port}`
 
@@ -39,3 +39,19 @@ describe('injected inline scripts', () => {
     }
   })
 })
+
+describe.runIf(isServe)('hmr', () => {
+  test('handle virtual module updates', async () => {
+    await page.goto(url)
+    const el = await page.$('.virtual')
+    expect(await el.textContent()).toBe('[success]')
+    editFile('src/importedVirtual.js', (code) =>
+      code.replace('[success]', '[wow]')
+    )
+    await page.waitForNavigation()
+    await untilUpdated(async () => {
+      const el = await page.$('.virtual')
+      return await el.textContent()
+    }, '[wow]')
+  })
+})
diff --git a/playground/ssr-html/index.html b/playground/ssr-html/index.html
@@ -12,5 +12,6 @@
   </head>
   <body>
     <h1>SSR Dynamic HTML</h1>
+    <div class="virtual"></div>
   </body>
 </html>
diff --git a/playground/ssr-html/server.js b/playground/ssr-html/server.js
@@ -48,7 +48,22 @@ export async function createServer(root = process.cwd(), hmrPort) {
         port: hmrPort
       }
     },
-    appType: 'custom'
+    appType: 'custom',
+    plugins: [
+      {
+        name: 'virtual-file',
+        resolveId(id) {
+          if (id === 'virtual:file') {
+            return '\0virtual:file'
+          }
+        },
+        load(id) {
+          if (id === '\0virtual:file') {
+            return 'import { virtual } from "/src/importedVirtual.js"; export { virtual };'
+          }
+        }
+      }
+    ]
   })
   // use vite's connect instance as middleware
   app.use(vite.middlewares)

diff --git a/playground/ssr-html/src/app.js b/playground/ssr-html/src/app.js
@@ -1,3 +1,11 @@
+import { virtual } from 'virtual:file'
+
 const p = document.createElement('p')
 p.innerHTML = '✅ Dynamically injected script from file'
 document.body.appendChild(p)
+
+text('.virtual', virtual)
+
+function text(el, text) {
+  document.querySelector(el).textContent = text
+}
diff --git a/playground/ssr-html/src/importedVirtual.js b/playground/ssr-html/src/importedVirtual.js
@@ -0,0 +1 @@
+export const virtual = '[success]'