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: correctly resolve hmr dep ids and fallback to url #18840

Merged
merged 10 commits into from
Jan 23, 2025
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 59 additions & 24 deletions packages/vite/src/node/plugins/importAnalysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,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
Expand Down Expand Up @@ -357,7 +357,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
Expand Down Expand Up @@ -398,32 +398,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]
}

Expand Down Expand Up @@ -549,7 +552,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.
Expand Down Expand Up @@ -753,9 +757,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,
})
}
Expand Down
23 changes: 23 additions & 0 deletions playground/hmr/__tests__/hmr.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'"
Expand Down
17 changes: 17 additions & 0 deletions playground/hmr/hmr.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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)
Expand Down Expand Up @@ -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)
})
Expand Down
2 changes: 2 additions & 0 deletions playground/hmr/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand Down
4 changes: 4 additions & 0 deletions playground/hmr/modules.d.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
declare module 'virtual:file' {
export const virtual: string
}

declare module 'virtual:file-dep' {
export const virtual: string
}
15 changes: 7 additions & 8 deletions playground/hmr/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading