feat!: rewrite how vite-node resolves id (#2463)

* feat!: rewrite how vite-node resolves id

* chore: try to fix global setup test

* fix: remove /@fs/ from filepath

* chore: cleanup

* fix: normalize id for callstack

* fix: don't append "/" before disk drive on windows

* chore: cleanup

* chore: add test that windows drive is not repeated

* fix: dirname uses \\, update windows paths tests

* refactor: rename variables

* fix: don't provide importer only for /@id/

* chore: remove null byte placeholder as part of unwrapRef

* chore: cleanup

* chore: variables renaming

* fix: don't hang on circular mock

* test: update c8 snapshot

* chore: add compatibility layer for users who don't provide resolveId

* test: fix url handling in web worker

* test: fix file tests on windows

* chore: remove unnecessary normalizations in mocker

* chore: use /@fs/ when fetching module, if possible

examples/mocks/src/A.ts

@@ -0,0 +1,5 @@
+import { funcB } from './B'
+
+export function funcA() {
+  return funcB
+}

examples/mocks/src/B.ts

@@ -0,0 +1,5 @@
+import { funcA } from './A'
+
+export function funcB() {
+  return funcA
+}

examples/mocks/src/main.js

@@ -0,0 +1,5 @@
+import { funcA } from './A'
+
+export function main() {
+  return funcA()
+}

examples/mocks/test/axios-not-mocked.test.ts

@@ -1,7 +1,7 @@
 import axios from 'axios'
 
 test('mocked axios', async () => {
-  const { default: ax } = await vi.importMock('axios')
+  const { default: ax } = await vi.importMock<any>('axios')
 
   await ax.get('string')
 

examples/mocks/test/circular.spec.ts

@@ -0,0 +1,11 @@
+import { expect, test, vi } from 'vitest'
+import { main } from '../src/main.js'
+
+vi.mock('../src/A', async () => ({
+  ...(await vi.importActual<any>('../src/A')),
+  funcA: () => 'mockedA',
+}))
+
+test('main', () => {
+  expect(main()).toBe('mockedA')
+})

examples/mocks/test/error-mock.spec.ts

@@ -4,5 +4,5 @@ vi.mock('../src/default', () => {
 
 test('when using top level variable, gives helpful message', async () => {
   await expect(() => import('../src/default').then(m => m.default)).rejects
-    .toThrowErrorMatchingInlineSnapshot('"[vitest] There was an error, when mocking a module. If you are using vi.mock, make sure you are not using top level variables inside, since this call is hoisted. Read more: https://vitest.dev/api/#vi-mock"')
+    .toThrowErrorMatchingInlineSnapshot('"[vitest] There was an error, when mocking a module. If you are using \\"vi.mock\\" factory, make sure there are no top level variables inside, since this call is hoisted to top of the file. Read more: https://vitest.dev/api/#vi-mock"')
 })

examples/mocks/test/factory.test.ts

@@ -57,7 +57,7 @@ describe('mocking with factory', () => {
 
   it('non-object return on factory gives error', async () => {
     await expect(() => import('../src/default').then(m => m.default)).rejects
-      .toThrowError('[vitest] vi.mock(path: string, factory?: () => unknown) is not returning an object. Did you mean to return an object with a "default" key?')
+      .toThrowError('[vitest] vi.mock("../src/default.ts", factory?: () => unknown) is not returning an object. Did you mean to return an object with a "default" key?')
   })
 
   test('defined exports on mock', async () => {

packages/coverage-c8/src/provider.ts

@@ -85,7 +85,7 @@ export class C8CoverageProvider implements CoverageProvider {
     // This is a magic number. It corresponds to the amount of code
     // that we add in packages/vite-node/src/client.ts:114 (vm.runInThisContext)
     // TODO: Include our transformations in sourcemaps
-    const offset = 224
+    const offset = 203
 
     report._getSourceMap = (coverage: Profiler.ScriptCoverage) => {
       const path = _url.pathToFileURL(coverage.url.split('?')[0]).href

packages/vite-node/package.json

@@ -70,7 +70,7 @@
   },
   "scripts": {
     "build": "rimraf dist && rollup -c",
-    "dev": "rollup -c --watch --watch.include=src -m inline",
+    "dev": "rollup -c --watch --watch.include 'src/**' -m inline",
     "prepublishOnly": "pnpm build",
     "typecheck": "tsc --noEmit"
   },

packages/vite-node/src/client.ts

@@ -1,10 +1,12 @@
 import { createRequire } from 'module'
+// we need native dirname, because windows __dirname has \\
+// eslint-disable-next-line no-restricted-imports
+import { dirname } from 'path'
 import { fileURLToPath, pathToFileURL } from 'url'
 import vm from 'vm'
-import { dirname, extname, isAbsolute, resolve } from 'pathe'
-import { isNodeBuiltin } from 'mlly'
+import { resolve } from 'pathe'
 import createDebug from 'debug'
-import { cleanUrl, isPrimitive, normalizeModuleId, normalizeRequestId, slash, toFilePath } from './utils'
+import { VALID_ID_PREFIX, cleanUrl, isInternalRequest, isPrimitive, normalizeModuleId, normalizeRequestId, slash, toFilePath } from './utils'
 import type { HotContext, ModuleCache, ViteNodeRunnerOptions } from './types'
 import { extractSourceMap } from './source-map'
 
@@ -145,24 +147,24 @@ export class ViteNodeRunner {
   }
 
   async executeFile(file: string) {
-    return await this.cachedRequest(`/@fs/${slash(resolve(file))}`, [])
+    const url = `/@fs/${slash(resolve(file))}`
+    return await this.cachedRequest(url, url, [])
   }
 
-  async executeId(id: string) {
-    return await this.cachedRequest(id, [])
+  async executeId(rawId: string) {
+    const [id, url] = await this.resolveUrl(rawId)
+    return await this.cachedRequest(id, url, [])
   }
 
   getSourceMap(id: string) {
     return this.moduleCache.getSourceMap(id)
   }
 
   /** @internal */
-  async cachedRequest(rawId: string, callstack: string[]) {
-    const id = normalizeRequestId(rawId, this.options.base)
-    const fsPath = toFilePath(id, this.root)
+  async cachedRequest(id: string, fsPath: string, callstack: string[]) {
+    const importee = callstack[callstack.length - 1]
 
     const mod = this.moduleCache.get(fsPath)
-    const importee = callstack[callstack.length - 1]
 
     if (!mod.importers)
       mod.importers = new Set()
@@ -188,79 +190,68 @@ export class ViteNodeRunner {
     }
   }
 
+  async resolveUrl(id: string, importee?: string): Promise<[url: string, fsPath: string]> {
+    if (isInternalRequest(id))
+      return [id, id]
+    // we don't pass down importee here, because otherwise Vite doesn't resolve it correctly
+    if (importee && id.startsWith(VALID_ID_PREFIX))
+      importee = undefined
+    id = normalizeRequestId(id, this.options.base)
+    if (!this.options.resolveId)
+      return [id, toFilePath(id, this.root)]
+    const resolved = await this.options.resolveId(id, importee)
+    const resolvedId = resolved
+      ? normalizeRequestId(resolved.id, this.options.base)
+      : id
+    // to be compatible with dependencies that do not resolve id
+    const fsPath = resolved ? resolvedId : toFilePath(id, this.root)
+    return [resolvedId, fsPath]
+  }
+
   /** @internal */
-  async directRequest(id: string, fsPath: string, _callstack: string[]) {
-    const callstack = [..._callstack, fsPath]
+  async dependencyRequest(id: string, fsPath: string, callstack: string[]) {
+    const getStack = () => {
+      return `stack:\n${[...callstack, fsPath].reverse().map(p => `- ${p}`).join('\n')}`
+    }
 
-    let mod = this.moduleCache.get(fsPath)
+    let debugTimer: any
+    if (this.debug)
+      debugTimer = setTimeout(() => console.warn(() => `module ${fsPath} takes over 2s to load.\n${getStack()}`), 2000)
 
-    const request = async (dep: string) => {
-      const depFsPath = toFilePath(normalizeRequestId(dep, this.options.base), this.root)
-      const getStack = () => {
-        return `stack:\n${[...callstack, depFsPath].reverse().map(p => `- ${p}`).join('\n')}`
+    try {
+      if (callstack.includes(fsPath)) {
+        const depExports = this.moduleCache.get(fsPath)?.exports
+        if (depExports)
+          return depExports
+        throw new Error(`[vite-node] Failed to resolve circular dependency, ${getStack()}`)
       }
 
-      let debugTimer: any
-      if (this.debug)
-        debugTimer = setTimeout(() => console.warn(() => `module ${depFsPath} takes over 2s to load.\n${getStack()}`), 2000)
-
-      try {
-        if (callstack.includes(depFsPath)) {
-          const depExports = this.moduleCache.get(depFsPath)?.exports
-          if (depExports)
-            return depExports
-          throw new Error(`[vite-node] Failed to resolve circular dependency, ${getStack()}`)
-        }
-
-        return await this.cachedRequest(dep, callstack)
-      }
-      finally {
-        if (debugTimer)
-          clearTimeout(debugTimer)
-      }
+      return await this.cachedRequest(id, fsPath, callstack)
+    }
+    finally {
+      if (debugTimer)
+        clearTimeout(debugTimer)
     }
+  }
 
-    Object.defineProperty(request, 'callstack', { get: () => callstack })
-
-    const resolveId = async (dep: string, callstackPosition = 1): Promise<[dep: string, id: string | undefined]> => {
-      if (this.options.resolveId && this.shouldResolveId(dep)) {
-        let importer: string | undefined = callstack[callstack.length - callstackPosition]
-        if (importer && !dep.startsWith('.'))
-          importer = undefined
-        if (importer && importer.startsWith('mock:'))
-          importer = importer.slice(5)
-        const resolved = await this.options.resolveId(normalizeRequestId(dep), importer)
-        return [dep, resolved?.id]
-      }
+  /** @internal */
+  async directRequest(id: string, fsPath: string, _callstack: string[]) {
+    const moduleId = normalizeModuleId(fsPath)
+    const callstack = [..._callstack, moduleId]
 
-      return [dep, undefined]
-    }
+    const mod = this.moduleCache.get(fsPath)
 
-    const [dep, resolvedId] = await resolveId(id, 2)
+    const request = async (dep: string) => {
+      const [id, depFsPath] = await this.resolveUrl(dep, fsPath)
+      return this.dependencyRequest(id, depFsPath, callstack)
+    }
 
     const requestStubs = this.options.requestStubs || DEFAULT_REQUEST_STUBS
     if (id in requestStubs)
       return requestStubs[id]
 
     // eslint-disable-next-line prefer-const
-    let { code: transformed, externalize } = await this.options.fetchModule(resolvedId || dep)
-
-    // in case we resolved fsPath incorrectly, Vite will return the correct file path
-    // in that case we need to update cache, so we don't have the same module as different exports
-    // but we ignore fsPath that has custom query, because it might need to be different
-    if (resolvedId && !fsPath.includes('?') && fsPath !== resolvedId) {
-      if (this.moduleCache.has(resolvedId)) {
-        mod = this.moduleCache.get(resolvedId)
-        this.moduleCache.set(fsPath, mod)
-        if (mod.promise)
-          return mod.promise
-        if (mod.exports)
-          return mod.exports
-      }
-      else {
-        this.moduleCache.set(resolvedId, mod)
-      }
-    }
+    let { code: transformed, externalize } = await this.options.fetchModule(id)
 
     if (externalize) {
       debugNative(externalize)
@@ -270,13 +261,12 @@ export class ViteNodeRunner {
     }
 
     if (transformed == null)
-      throw new Error(`[vite-node] Failed to load ${id}`)
-
-    const file = cleanUrl(resolvedId || fsPath)
+      throw new Error(`[vite-node] Failed to load "${id}" imported from ${callstack[callstack.length - 2]}`)
 
+    const modulePath = cleanUrl(moduleId)
     // disambiguate the `<UNIT>:/` on windows: see nodejs/node#31710
-    const url = pathToFileURL(file).href
-    const meta = { url }
+    const href = pathToFileURL(modulePath).href
+    const meta = { url: href }
     const exports = Object.create(null)
     Object.defineProperty(exports, Symbol.toStringTag, {
       value: 'Module',
@@ -305,7 +295,7 @@ export class ViteNodeRunner {
     })
 
     Object.assign(mod, { code: transformed, exports })
-    const __filename = fileURLToPath(url)
+    const __filename = fileURLToPath(href)
     const moduleProxy = {
       set exports(value) {
         exportAll(cjsExports, value)
@@ -339,10 +329,9 @@ export class ViteNodeRunner {
       __vite_ssr_exports__: exports,
       __vite_ssr_exportAll__: (obj: any) => exportAll(exports, obj),
       __vite_ssr_import_meta__: meta,
-      __vitest_resolve_id__: resolveId,
 
       // cjs compact
-      require: createRequire(url),
+      require: createRequire(href),
       exports: cjsExports,
       module: moduleProxy,
       __filename,
@@ -373,13 +362,6 @@ export class ViteNodeRunner {
     return context
   }
 
-  shouldResolveId(dep: string) {
-    if (isNodeBuiltin(dep) || dep in (this.options.requestStubs || DEFAULT_REQUEST_STUBS) || dep.startsWith('/@vite'))
-      return false
-
-    return !isAbsolute(dep) || !extname(dep)
-  }
-
   /**
    * Define if a module should be interop-ed
    * This function mostly for the ability to override by subclass

packages/vite-node/src/utils.ts

@@ -1,7 +1,6 @@
 import { fileURLToPath, pathToFileURL } from 'url'
 import { existsSync } from 'fs'
-import { relative, resolve } from 'pathe'
-import { isNodeBuiltin } from 'mlly'
+import { resolve } from 'pathe'
 import type { Arrayable, Nullable } from './types'
 
 export const isWindows = process.platform === 'win32'
@@ -14,6 +13,8 @@ export function mergeSlashes(str: string) {
   return str.replace(/\/\//g, '/')
 }
 
+export const VALID_ID_PREFIX = '/@id/'
+
 export function normalizeRequestId(id: string, base?: string): string {
   if (base && id.startsWith(base))
     id = `/${id.slice(base.length)}`
@@ -40,10 +41,14 @@ export const hashRE = /#.*$/s
 export const cleanUrl = (url: string): string =>
   url.replace(hashRE, '').replace(queryRE, '')
 
+export const isInternalRequest = (id: string): boolean => {
+  return id.startsWith('/@vite/')
+}
+
 export function normalizeModuleId(id: string) {
   return id
     .replace(/\\/g, '/')
-    .replace(/^\/@fs\//, '/')
+    .replace(/^\/@fs\//, isWindows ? '' : '/')
     .replace(/^file:\//, '/')
     .replace(/^\/+/, '/')
 }
@@ -52,24 +57,6 @@ export function isPrimitive(v: any) {
   return v !== Object(v)
 }
 
-export function pathFromRoot(root: string, filename: string) {
-  if (isNodeBuiltin(filename))
-    return filename
-
-  // don't replace with "/" on windows, "/C:/foo" is not a valid path
-  filename = filename.replace(/^\/@fs\//, isWindows ? '' : '/')
-
-  if (!filename.startsWith(root))
-    return filename
-
-  const relativePath = relative(root, filename)
-
-  const segments = relativePath.split('/')
-  const startIndex = segments.findIndex(segment => segment !== '..' && segment !== '.')
-
-  return `/${segments.slice(startIndex).join('/')}`
-}
-
 export function toFilePath(id: string, root: string): string {
   let absolute = (() => {
     if (id.startsWith('/@fs/'))

packages/vitest/src/integrations/vi.ts

@@ -2,7 +2,7 @@ import type { FakeTimerInstallOpts } from '@sinonjs/fake-timers'
 import { parseStacktrace } from '../utils/source-map'
 import type { VitestMocker } from '../runtime/mocker'
 import type { ResolvedConfig, RuntimeConfig } from '../types'
-import { getWorkerState, resetModules, setTimeout } from '../utils'
+import { getWorkerState, resetModules, waitForImportsToResolve } from '../utils'
 import { FakeTimers } from './mock/timers'
 import type { EnhancedSpy, MaybeMocked, MaybeMockedDeep, MaybePartiallyMocked, MaybePartiallyMockedDeep } from './spy'
 import { fn, isMockFunction, spies, spyOn } from './spy'
@@ -245,26 +245,12 @@ class VitestUtils {
   }
 
   /**
-   * Wait for all imports to load.
-   * Useful, if you have a synchronous call that starts
-   * importing a module that you cannot wait otherwise.
+   * Wait for all imports to load. Useful, if you have a synchronous call that starts
+   * importing a module that you cannot await otherwise.
+   * Will also wait for new imports, started during the wait.
    */
   public async dynamicImportSettled() {
-    const state = getWorkerState()
-    const promises: Promise<unknown>[] = []
-    for (const mod of state.moduleCache.values()) {
-      if (mod.promise && !mod.evaluated)
-        promises.push(mod.promise)
-    }
-    if (!promises.length)
-      return
-    await Promise.allSettled(promises)
-    // wait until the end of the loop, so `.then` on modules is called,
-    // like in import('./example').then(...)
-    // also call dynamicImportSettled again in case new imports were added
-    await new Promise(resolve => setTimeout(resolve, 1))
-      .then(() => Promise.resolve())
-      .then(() => this.dynamicImportSettled())
+    return waitForImportsToResolve()
   }
 
   private _config: null | ResolvedConfig = null