Skip to content

Commit e8ca643

Browse files
authoredDec 28, 2023
fix(vitest): don't hang when mocking files with cyclic dependencies (#4811)
1 parent 039814b commit e8ca643

File tree

8 files changed

+63
-9
lines changed

8 files changed

+63
-9
lines changed
 
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import './module-2'
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import './module-3'
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import './module-4'
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import './module-1'
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { expect, test, vi } from 'vitest'
2+
3+
import '../src/cyclic-deps/module-1'
4+
5+
vi.mock('../src/cyclic-deps/module-2', async () => {
6+
await vi.importActual('../src/cyclic-deps/module-2')
7+
8+
return { default: () => {} }
9+
})
10+
11+
test('some test', () => {
12+
expect(1 + 1).toBe(2)
13+
})
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { expect, test, vi } from 'vitest'
2+
3+
import '../src/cyclic-deps/module-1'
4+
5+
vi.mock('../src/cyclic-deps/module-2', async (importOriginal) => {
6+
await importOriginal()
7+
8+
return { default: () => {} }
9+
})
10+
11+
test('some test', () => {
12+
expect(1 + 1).toBe(2)
13+
})

‎packages/vitest/src/integrations/vi.ts

+7-3
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ function createVitest(): VitestUtils {
482482
_mocker.queueMock(
483483
path,
484484
importer,
485-
factory ? () => factory(() => _mocker.importActual(path, importer)) : undefined,
485+
factory ? () => factory(() => _mocker.importActual(path, importer, _mocker.getMockContext().callstack)) : undefined,
486486
)
487487
},
488488

@@ -495,7 +495,7 @@ function createVitest(): VitestUtils {
495495
_mocker.queueMock(
496496
path,
497497
importer,
498-
factory ? () => factory(() => _mocker.importActual(path, importer)) : undefined,
498+
factory ? () => factory(() => _mocker.importActual(path, importer, _mocker.getMockContext().callstack)) : undefined,
499499
)
500500
},
501501

@@ -504,7 +504,11 @@ function createVitest(): VitestUtils {
504504
},
505505

506506
async importActual<T = unknown>(path: string): Promise<T> {
507-
return _mocker.importActual<T>(path, getImporter())
507+
return _mocker.importActual<T>(
508+
path,
509+
getImporter(),
510+
_mocker.getMockContext().callstack,
511+
)
508512
},
509513

510514
async importMock<T>(path: string): Promise<MaybeMockedDeep<T>> {

‎packages/vitest/src/runtime/mocker.ts

+26-6
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,13 @@ class RefTracker {
3232

3333
type Key = string | symbol
3434

35+
interface MockContext {
36+
/**
37+
* When mocking with a factory, this refers to the module that imported the mock.
38+
*/
39+
callstack: null | string[]
40+
}
41+
3542
function isSpecialProp(prop: Key, parentType: string) {
3643
return parentType.includes('Function')
3744
&& typeof prop === 'string'
@@ -54,6 +61,10 @@ export class VitestMocker {
5461

5562
private filterPublicKeys: (symbol | string)[]
5663

64+
private mockContext: MockContext = {
65+
callstack: null,
66+
}
67+
5768
constructor(
5869
public executor: VitestExecutor,
5970
) {
@@ -201,9 +212,9 @@ export class VitestMocker {
201212
throw this.createError(
202213
`[vitest] No "${String(prop)}" export is defined on the "${mockpath}" mock. `
203214
+ 'Did you forget to return it from "vi.mock"?'
204-
+ '\nIf you need to partially mock a module, you can use "vi.importActual" inside:\n\n'
205-
+ `${c.green(`vi.mock("${mockpath}", async () => {
206-
const actual = await vi.importActual("${mockpath}")
215+
+ '\nIf you need to partially mock a module, you can use "importOriginal" helper inside:\n\n'
216+
+ `${c.green(`vi.mock("${mockpath}", async (importOriginal) => {
217+
const actual = await importOriginal()
207218
return {
208219
...actual,
209220
// your mocked methods
@@ -221,6 +232,10 @@ export class VitestMocker {
221232
return moduleExports
222233
}
223234

235+
public getMockContext() {
236+
return this.mockContext
237+
}
238+
224239
public getMockPath(dep: string) {
225240
return `mock:${dep}`
226241
}
@@ -407,9 +422,9 @@ export class VitestMocker {
407422
this.deleteCachedItem(id)
408423
}
409424

410-
public async importActual<T>(rawId: string, importee: string): Promise<T> {
411-
const { id, fsPath } = await this.resolvePath(rawId, importee)
412-
const result = await this.executor.cachedRequest(id, fsPath, [importee])
425+
public async importActual<T>(rawId: string, importer: string, callstack?: string[] | null): Promise<T> {
426+
const { id, fsPath } = await this.resolvePath(rawId, importer)
427+
const result = await this.executor.cachedRequest(id, fsPath, callstack || [importer])
413428
return result as T
414429
}
415430

@@ -453,9 +468,14 @@ export class VitestMocker {
453468
if (typeof mock === 'function' && !callstack.includes(mockPath) && !callstack.includes(url)) {
454469
try {
455470
callstack.push(mockPath)
471+
// this will not work if user does Promise.all(import(), import())
472+
// we can also use AsyncLocalStorage to store callstack, but this won't work in the browser
473+
// maybe we should improve mock API in the future?
474+
this.mockContext.callstack = callstack
456475
return await this.callFunctionMock(mockPath, mock)
457476
}
458477
finally {
478+
this.mockContext.callstack = null
459479
const indexMock = callstack.indexOf(mockPath)
460480
callstack.splice(indexMock, 1)
461481
}

0 commit comments

Comments
 (0)
Please sign in to comment.