fix: show correct line numbers in stack trace when using vi.resetModules() (#3020)

Author: sheremet-va

packages/vite-node/src/client.ts

@@ -92,6 +92,14 @@ export class ModuleCacheMap extends Map<string, ModuleCache> {
     return this.deleteByModuleId(this.normalizePath(fsPath))
   }
 
+  invalidateModule(mod: ModuleCache) {
+    delete mod.evaluated
+    delete mod.resolving
+    delete mod.promise
+    delete mod.exports
+    return true
+  }
+
   /**
    * Invalidate modules that dependent on the given modules, up to the main entry
    */

packages/vitest/src/utils/index.ts

@@ -31,10 +31,10 @@ export function resetModules(modules: ModuleCacheMap, resetMocks = false) {
     // don't clear mocks
     ...(!resetMocks ? [/^mock:/] : []),
   ]
-  modules.forEach((_, path) => {
+  modules.forEach((mod, path) => {
     if (skipPaths.some(re => re.test(path)))
       return
-    modules.delete(path)
+    modules.invalidateModule(mod)
   })
 }
 

test/core/test/utils.spec.ts

@@ -1,8 +1,9 @@
-import { describe, expect, test } from 'vitest'
+import { beforeAll, describe, expect, test } from 'vitest'
 import { assertTypes, deepClone, objectAttr, toArray } from '@vitest/utils'
 import { deepMerge, resetModules } from '../../../packages/vitest/src/utils'
 import { deepMergeSnapshot } from '../../../packages/vitest/src/integrations/snapshot/port/utils'
-import type { ModuleCacheMap } from '../../../packages/vite-node/src/types'
+import type { EncodedSourceMap } from '../../../packages/vite-node/src/types'
+import { ModuleCacheMap } from '../../../packages/vite-node/dist/client'
 
 describe('assertTypes', () => {
   test('the type of value should be number', () => {
@@ -151,36 +152,44 @@ describe('deepClone', () => {
 })
 
 describe('resetModules doesn\'t resets only user modules', () => {
-  test('resets user modules', () => {
-    const moduleCache = new Map() as ModuleCacheMap
-    moduleCache.set('/some-module.ts', {})
-    moduleCache.set('/@fs/some-path.ts', {})
-
-    resetModules(moduleCache)
-
-    expect(moduleCache.size).toBe(0)
-  })
-
-  test('doesn\'t reset vitest modules', () => {
-    const moduleCache = new Map() as ModuleCacheMap
-    moduleCache.set('/node_modules/vitest/dist/index.js', {})
-    moduleCache.set('/node_modules/vitest-virtual-da9876a/dist/index.js', {})
-    moduleCache.set('/node_modules/some-module@vitest/dist/index.js', {})
-    moduleCache.set('/packages/vitest/dist/index.js', {})
-
+  const mod = () => ({ evaluated: true, promise: Promise.resolve({}), resolving: false, exports: {}, map: {} as EncodedSourceMap })
+
+  const moduleCache = new ModuleCacheMap()
+  const modules = [
+    ['/some-module.ts', true],
+    ['/@fs/some-path.ts', true],
+    ['/node_modules/vitest/dist/index.js', false],
+    ['/node_modules/vitest-virtual-da9876a/dist/index.js', false],
+    ['/node_modules/some-module@vitest/dist/index.js', false],
+    ['/packages/vitest/dist/index.js', false],
+    ['mock:/some-module.ts', false],
+    ['mock:/@fs/some-path.ts', false],
+  ] as const
+
+  beforeAll(() => {
+    modules.forEach(([path]) => {
+      moduleCache.set(path, mod())
+    })
     resetModules(moduleCache)
-
-    expect(moduleCache.size).toBe(4)
   })
 
-  test('doesn\'t reset mocks', () => {
-    const moduleCache = new Map() as ModuleCacheMap
-    moduleCache.set('mock:/some-module.ts', {})
-    moduleCache.set('mock:/@fs/some-path.ts', {})
+  test.each(modules)('Cashe for %s is reseted (%s)', (path, reset) => {
+    const cached = moduleCache.get(path)
 
-    resetModules(moduleCache)
+    if (reset) {
+      expect(cached).not.toHaveProperty('evaluated')
+      expect(cached).not.toHaveProperty('resolving')
+      expect(cached).not.toHaveProperty('exports')
+      expect(cached).not.toHaveProperty('promise')
+    }
+    else {
+      expect(cached).toHaveProperty('evaluated')
+      expect(cached).toHaveProperty('resolving')
+      expect(cached).toHaveProperty('exports')
+      expect(cached).toHaveProperty('promise')
+    }
 
-    expect(moduleCache.size).toBe(2)
+    expect(cached).toHaveProperty('map')
   })
 })
 

test/stacktraces/fixtures/reset-modules.test.ts

@@ -0,0 +1,18 @@
+import { assert, describe, expect, it, vi } from 'vitest'
+
+describe('suite name', () => {
+  it('foo', () => {
+    vi.resetModules()
+    // a comment here
+    // another comment
+    // another comment
+    // another comment
+    // another comment
+    // another comment
+    // this will mess up the stacktrace lines
+    expect(1 + 1).eq(2)
+    expect(2 + 1).eq(3)
+    assert.equal(Math.sqrt(4), 2)
+    expect(Math.sqrt(4)).toBe(1)
+  })
+})

test/stacktraces/test/__snapshots__/runner.test.ts.snap

@@ -1,4 +1,4 @@
-// Vitest Snapshot v1
+// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
 exports[`stacktraces should pick error frame if present > frame.spec.imba > frame.spec.imba 1`] = `
 " FAIL  frame.spec.imba [ frame.spec.imba ]
@@ -43,3 +43,14 @@ exports[`stacktraces should respect sourcemaps > add-in-js.test.js > add-in-js.t
       8|   return expect(add(1, 2, 3)).toBe(6)
 "
 `;
+
+exports[`stacktraces should respect sourcemaps > reset-modules.test.ts > reset-modules.test.ts 1`] = `
+" ❯ reset-modules.test.ts:16:26
+     14|     expect(2 + 1).eq(3)
+     15|     assert.equal(Math.sqrt(4), 2)
+     16|     expect(Math.sqrt(4)).toBe(1)
+       |                          ^
+     17|   })
+     18| })
+"
+`;