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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
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
74 changes: 53 additions & 21 deletions packages/vite/src/node/plugins/importAnalysis.ts
Original file line number Diff line number Diff line change
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 @@ -753,9 +756,38 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
// normalize and rewrite accepted urls
const normalizedAcceptedUrls = new Set<string>()
patricklx marked this conversation as resolved.
Show resolved Hide resolved
for (const { url, start, end } of acceptedUrls) {
const [normalized] = await moduleGraph.resolveUrl(toAbsoluteUrl(url))
let [normalized, resolvedId] = await normalizeUrl(url, start)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sapphi-red I changed this to use normalizeUrl which already does the resolve and also calls _ensureEntryFromUrl.
I had to move _ensureEntryFromUrl out of if (environment.config.consumer === 'client') { check so its also called in ssr

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not taken a deeper look, but my guess is that this normalizeUrl throws an error when it fails to resolve, which is why the fallback isn't working.

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,
})
}
sapphi-red marked this conversation as resolved.
Show resolved Hide resolved
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