fix(module-runner): keep same import with different importer separately

[sapphi-red]

Description

Since fetchModule may return a different value depending on importer even if url is the same, I think this.concurrentModuleNodePromises should have url + importer as a key.

I noticed this while reading the code and haven't encountered any specific issues.

[hi-ogawa]

Makes sense. Externalized dep (i.e. bare specifier) is resolved based on importer in fetchModule, so it would technically get mixed up if there are two different node_modules separately.

[sheremet-va]

Strange that you did not encounter any issue. We had the code work like this before, but it actually doesn't make sense because it will only cache modules inside the same module in that case, so the cache will almost never be hit.

The url is almost always already resolved by the import analyser plugin, and dynamic imports also resolves it manually before calling fetch.

I will try to find the issue why we changed it back to only url

[hi-ogawa]

Oh right. Caching module for already resolved url was the point. I guess this is technically desired only for unresolved url at this point, which is externalized dep, but I'd imagine that's less problematic than changing it.

[hi-ogawa]

/ecosystem-ci run

[pkg-pr-new]

Open in StackBlitz

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

commit: cbfc200

[sheremet-va]

Oh right. Caching module for already resolved url was the point. I guess this is technically desired only for unresolved url at this point, which is externalized dep, but I'd imagine that's less problematic than changing it.

True, importer is only used by the externalized dependency, we can just have a guard like we do in

vite/packages/vite/src/node/ssr/fetchModule.ts

Line 46 in b4a124b

if (!isFileUrl && importer && url[0] !== '.' && url[0] !== '/') {

[vite-ecosystem-ci]

📝 Ran ecosystem CI on a0fabd1: Open

suite	result	latest scheduled
react-router	✅ success	⏹️ cancelled
qwik	✅ success	❌ failure
laravel	❌ failure	❌ failure
waku	❌ failure	✅ success

✅ ladle, analogjs, one, rakkas, histoire, marko, quasar, nuxt, unocss, storybook, vite-environment-examples, vite-plugin-vue, vike, vite-plugin-react, vitepress, vuepress, sveltekit, vite-setup-catalogue, vite-plugin-svelte, vite-plugin-cloudflare, vite-plugin-pwa, vitest, vite-plugin-rsc, astro