Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(vitest): don't hang when mocking files with cyclic dependencies #4811

Merged
merged 4 commits into from
Dec 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions examples/mocks/src/cyclic-deps/module-1.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './module-2'
1 change: 1 addition & 0 deletions examples/mocks/src/cyclic-deps/module-2.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './module-3'
1 change: 1 addition & 0 deletions examples/mocks/src/cyclic-deps/module-3.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './module-4'
1 change: 1 addition & 0 deletions examples/mocks/src/cyclic-deps/module-4.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './module-1'
13 changes: 13 additions & 0 deletions examples/mocks/test/cyclic-import-actual.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { expect, test, vi } from 'vitest'

import '../src/cyclic-deps/module-1'

vi.mock('../src/cyclic-deps/module-2', async () => {
await vi.importActual('../src/cyclic-deps/module-2')

return { default: () => {} }
})

test('some test', () => {
expect(1 + 1).toBe(2)
})
13 changes: 13 additions & 0 deletions examples/mocks/test/cyclic-import-original.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { expect, test, vi } from 'vitest'

import '../src/cyclic-deps/module-1'

vi.mock('../src/cyclic-deps/module-2', async (importOriginal) => {
await importOriginal()

return { default: () => {} }
})

test('some test', () => {
expect(1 + 1).toBe(2)
})
10 changes: 7 additions & 3 deletions packages/vitest/src/integrations/vi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ function createVitest(): VitestUtils {
_mocker.queueMock(
path,
importer,
factory ? () => factory(() => _mocker.importActual(path, importer)) : undefined,
factory ? () => factory(() => _mocker.importActual(path, importer, _mocker.getMockContext().callstack)) : undefined,
)
},

Expand All @@ -495,7 +495,7 @@ function createVitest(): VitestUtils {
_mocker.queueMock(
path,
importer,
factory ? () => factory(() => _mocker.importActual(path, importer)) : undefined,
factory ? () => factory(() => _mocker.importActual(path, importer, _mocker.getMockContext().callstack)) : undefined,
)
},

Expand All @@ -504,7 +504,11 @@ function createVitest(): VitestUtils {
},

async importActual<T = unknown>(path: string): Promise<T> {
return _mocker.importActual<T>(path, getImporter())
return _mocker.importActual<T>(
path,
getImporter(),
_mocker.getMockContext().callstack,
)
},

async importMock<T>(path: string): Promise<MaybeMockedDeep<T>> {
Expand Down
32 changes: 26 additions & 6 deletions packages/vitest/src/runtime/mocker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ class RefTracker {

type Key = string | symbol

interface MockContext {
/**
* When mocking with a factory, this refers to the module that imported the mock.
*/
callstack: null | string[]
}

function isSpecialProp(prop: Key, parentType: string) {
return parentType.includes('Function')
&& typeof prop === 'string'
Expand All @@ -54,6 +61,10 @@ export class VitestMocker {

private filterPublicKeys: (symbol | string)[]

private mockContext: MockContext = {
callstack: null,
}

constructor(
public executor: VitestExecutor,
) {
Expand Down Expand Up @@ -201,9 +212,9 @@ export class VitestMocker {
throw this.createError(
`[vitest] No "${String(prop)}" export is defined on the "${mockpath}" mock. `
+ 'Did you forget to return it from "vi.mock"?'
+ '\nIf you need to partially mock a module, you can use "vi.importActual" inside:\n\n'
+ `${c.green(`vi.mock("${mockpath}", async () => {
const actual = await vi.importActual("${mockpath}")
+ '\nIf you need to partially mock a module, you can use "importOriginal" helper inside:\n\n'
+ `${c.green(`vi.mock("${mockpath}", async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
// your mocked methods
Expand All @@ -221,6 +232,10 @@ export class VitestMocker {
return moduleExports
}

public getMockContext() {
return this.mockContext
}

public getMockPath(dep: string) {
return `mock:${dep}`
}
Expand Down Expand Up @@ -407,9 +422,9 @@ export class VitestMocker {
this.deleteCachedItem(id)
}

public async importActual<T>(rawId: string, importee: string): Promise<T> {
const { id, fsPath } = await this.resolvePath(rawId, importee)
const result = await this.executor.cachedRequest(id, fsPath, [importee])
public async importActual<T>(rawId: string, importer: string, callstack?: string[] | null): Promise<T> {
const { id, fsPath } = await this.resolvePath(rawId, importer)
const result = await this.executor.cachedRequest(id, fsPath, callstack || [importer])
return result as T
}

Expand Down Expand Up @@ -453,9 +468,14 @@ export class VitestMocker {
if (typeof mock === 'function' && !callstack.includes(mockPath) && !callstack.includes(url)) {
try {
callstack.push(mockPath)
// this will not work if user does Promise.all(import(), import())
// we can also use AsyncLocalStorage to store callstack, but this won't work in the browser
// maybe we should improve mock API in the future?
this.mockContext.callstack = callstack
return await this.callFunctionMock(mockPath, mock)
}
finally {
this.mockContext.callstack = null
const indexMock = callstack.indexOf(mockPath)
callstack.splice(indexMock, 1)
}
Expand Down
Loading