Skip to content

Commit

Permalink
fix(hmr): dedupe virtual modules in module graph (#10144)
Browse files Browse the repository at this point in the history
  • Loading branch information
bluwy authored Sep 19, 2022
1 parent c948e7d commit 71f08e7
Show file tree
Hide file tree
Showing 14 changed files with 106 additions and 31 deletions.
25 changes: 10 additions & 15 deletions packages/vite/src/node/plugins/importAnalysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -42,7 +40,8 @@ import {
stripBomTag,
timeFrom,
transformStableResult,
unwrapId
unwrapId,
wrapId
} from '../utils'
import type { ResolvedConfig } from '../config'
import type { Plugin } from '../plugin'
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
)
Expand Down Expand Up @@ -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(
Expand All @@ -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')) {
Expand Down Expand Up @@ -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
Expand Down
12 changes: 4 additions & 8 deletions packages/vite/src/node/server/middlewares/indexHtml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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)
Expand Down
5 changes: 2 additions & 3 deletions packages/vite/src/node/ssr/ssrModuleLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
21 changes: 18 additions & 3 deletions packages/vite/src/node/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
DEFAULT_EXTENSIONS,
ENV_PUBLIC_PATH,
FS_PREFIX,
NULL_BYTE_PLACEHOLDER,
OPTIMIZABLE_ENTRY_RE,
VALID_ID_PREFIX,
loopbackHosts,
Expand Down Expand Up @@ -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 =>
Expand Down
11 changes: 11 additions & 0 deletions playground/hmr/__tests__/hmr.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]')
})
}
3 changes: 3 additions & 0 deletions playground/hmr/hmr.ts
Original file line number Diff line number Diff line change
@@ -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 }) => {
Expand Down
1 change: 1 addition & 0 deletions playground/hmr/importedVirtual.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const virtual = '[success]'
1 change: 1 addition & 0 deletions playground/hmr/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
13 changes: 13 additions & 0 deletions playground/hmr/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };'
}
}
}
]
})
18 changes: 17 additions & 1 deletion playground/ssr-html/__tests__/ssr-html.spec.ts
Original file line number Diff line number Diff line change
@@ -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}`

Expand Down Expand Up @@ -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]')
})
})
1 change: 1 addition & 0 deletions playground/ssr-html/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@
</head>
<body>
<h1>SSR Dynamic HTML</h1>
<div class="virtual"></div>
</body>
</html>
17 changes: 16 additions & 1 deletion playground/ssr-html/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions playground/ssr-html/src/app.js
Original file line number Diff line number Diff line change
@@ -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
}
1 change: 1 addition & 0 deletions playground/ssr-html/src/importedVirtual.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const virtual = '[success]'

0 comments on commit 71f08e7

Please sign in to comment.