fix: correctly resolve hmr dep ids and fallback to url

[patricklx]

Description

fixes #12912
fixes #16375

this is the most basic to get it working. There are more issues regarding this, like having multiple modules in the module graph. one for 0virtual:file and another for /@id/__00__/virtual:file

[sapphi-red]

Thank you for the PR!
I don't have much context for the linked issue and PRs, but my understandings are:

• import.meta.hot.accept currently expects the deps argument to be a URL.
• for a module with an id starting with \0, the url property of the module node for that module starts with \0
• writing import.meta.hot.accept(urlForTheVirtualModule) doesn't work, because urlForTheVirtualModule starts with \0 instead of / and thus toAbsolutePath appends some string

I think there are two ways to fix the issue.

The first is to make import.meta.hot.accept to expect the deps argument to be an ID instead of a URL. I think this is more intuitive. But given that we have been expecting URLs, it's a breaking change. Maybe simply falling back to treating as a URL when resolving as an id fails would be fine.

The second is to make the url property of the module node to always start with /. Since it's a "URL", I expect it to always be a (non-relative) path part of the URL, and thus start with /. This fix would probably be more simple than the first one.

IIUC this PR makes import.meta.hot.accept to expect the deps arugument to be a URL for relative specifiers and to be an id for non-relative specifiers. I think that is confusing and better to expect only URLs or only ids.

[patricklx]

hi, alright, I think it should accept Ids, since the imports can be transformed based on imported and specifier. i think meta.hot.accept should do the same.

is it necessary a breaking change? I did not find that its documented to use url for deps.

[patricklx]

@sapphi-red this is now passing and still uses urls, but tries first to resolve the module via resolveId and then uses the resolved module.url.

on meta.hot.accept:
to be more precise:

• import.meta.hot.accept currently expects the deps argument to be a URL at runtime and id at build time.

[patricklx]

@sapphi-red does this look okay now?

[sapphi-red]

Thank you for the change!

[patricklx]

I looked at vitest failure. vitest is currently failing on main

[sapphi-red]

@patricklx Would you also describe where you needed to use import.meta.hot.accept(virtualDeps)?

[patricklx]

@patricklx Would you also describe where you needed to use import.meta.hot.accept(virtualDeps)?

I have a vite plugin that generates virtual files which can also change depending on other files on the filesystem.
currently it's impossible to hot accept them.

[sapphi-red]

@patricklx Would you also describe where you needed to use import.meta.hot.accept(virtualDeps)?

I have a vite plugin that generates virtual files which can also change depending on other files on the filesystem. currently it's impossible to hot accept them.

Is self-accepting on the virtual module side (i.e. call import.meta.accept(cb) in the virtual module) not an option?

[sapphi-red]

/ecosystem-ci run

[pkg-pr-new]

Open in Stackblitz

npm i https://pkg.pr.new/vite@18840

commit: d17d16f

[vite-ecosystem-ci]

📝 Ran ecosystem CI on ff09f8b: Open

suite	result	latest scheduled
histoire	❌ failure	❌ failure
nuxt	✅ success	❌ failure
rakkas	❌ failure	✅ success
redwoodjs	❌ failure	❌ failure
remix	❌ failure	❌ failure
storybook	❌ failure	✅ success
vike	❌ failure	❌ failure
vite-plugin-svelte	❌ failure	✅ success
vite-plugin-vue	❌ failure	✅ success
vitepress	❌ failure	✅ success
waku	❌ failure	❌ failure

✅ analogjs, astro, ladle, laravel, marko, previewjs, quasar, qwik, sveltekit, unocss, vite-environment-examples, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-swc, vite-setup-catalogue, vitest, vuepress

[sapphi-red]

Hmm, it seems rakkas and vitepress is passing a URL. cc @patak-dev

[patricklx]

vue is also failing in their hmr tests

[sapphi-red]

plugin-vue's fail is a flaky fail, so that one is fine (vitejs/vite-plugin-vue#485).

[patak-dev]

Hmm, it seems rakkas and vitepress is passing a URL

Maybe we could add back the URL fallback in the next minor if we're clear that this is just for backward compat and we'll move to a warning on Vite v7.

[patricklx]

So, should i add back the fallback then?

[sapphi-red]

Yeah. Would you add back the fallback with a comment that this is just for backward compat and will remove it in Vite 7?

[patricklx]

Yeah. Would you add back the fallback with a comment that this is just for backward compat and will remove it in Vite 7?

I added the change

[sapphi-red]

Thank you

The fallback does not seem to be working

[sapphi-red]

/ecosystem-ci run

[vite-ecosystem-ci]

📝 Ran ecosystem CI on d17d16f: Open

suite	result	latest scheduled
histoire	❌ failure	❌ failure
laravel	❌ failure	✅ success
marko	❌ failure	✅ success
redwoodjs	❌ failure	❌ failure
remix	❌ failure	❌ failure
vike	❌ failure	❌ failure
vite-plugin-svelte	✅ success	❌ failure
waku	❌ failure	❌ failure

✅ analogjs, astro, ladle, nuxt, previewjs, quasar, qwik, rakkas, storybook, sveltekit, unocss, vite-environment-examples, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-swc, vite-plugin-vue, vite-setup-catalogue, vitepress, vitest, vuepress

[sapphi-red]

Thank you 💚