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

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

Merged
merged 9 commits into from Sep 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 10 additions & 15 deletions packages/vite/src/node/plugins/importAnalysis.ts
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
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
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
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
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
@@ -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
@@ -0,0 +1 @@
export const virtual = '[success]'
1 change: 1 addition & 0 deletions playground/hmr/index.html
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
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
@@ -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
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
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
@@ -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
@@ -0,0 +1 @@
export const virtual = '[success]'