Skip to content

Commit 5d0fd3b

Browse files
authored
fix(spy): throw correct errors when shorthand methods are used on a class (#9513)
1 parent c011860 commit 5d0fd3b

File tree

6 files changed

+141
-9
lines changed

6 files changed

+141
-9
lines changed

docs/api/mock.md

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,47 @@ fn.length // == 2
5050
The custom function implementation in the types below is marked with a generic `<T>`.
5151
:::
5252

53+
::: warning Class Support {#class-support}
54+
Shorthand methods like `mockReturnValue`, `mockReturnValueOnce`, `mockResolvedValue` and others cannot be used on a mocked class. Class constructors have [unintuitive behaviour](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/constructor) regarding the return value:
55+
56+
```ts {2,7}
57+
const CorrectDogClass = vi.fn(class {
58+
constructor(public name: string) {}
59+
})
60+
61+
const IncorrectDogClass = vi.fn(class {
62+
constructor(public name: string) {
63+
return { name }
64+
}
65+
})
66+
67+
const Marti = new CorrectDogClass('Marti')
68+
const Newt = new IncorrectDogClass('Newt')
69+
70+
Marti instanceof CorrectDogClass // ✅ true
71+
Newt instanceof IncorrectDogClass // ❌ false!
72+
```
73+
74+
Even though the shapes are the same, the _return value_ from the constructor is assigned to `Newt`, which is a plain object, not an instance of a mock. Vitest guards you against this behaviour in shorthand methods (but not in `mockImplementation`!) and throws an error instead.
75+
76+
If you need to mock constructed instance of a class, consider using the `class` syntax with `mockImplementation` instead:
77+
78+
```ts
79+
mock.mockReturnValue({ hello: () => 'world' }) // [!code --]
80+
mock.mockImplementation(class { hello = () => 'world' }) // [!code ++]
81+
```
82+
83+
If you need to test the behaviour where this is a valid use case, you can use `mockImplementation` with a `constructor`:
84+
85+
```ts
86+
mock.mockImplementation(class {
87+
constructor(name: string) {
88+
return { name }
89+
}
90+
})
91+
```
92+
:::
93+
5394
## getMockImplementation
5495

5596
```ts

eslint.config.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ export default antfu(
107107
`**/*.md/${GLOB_SRC}`,
108108
],
109109
rules: {
110+
'prefer-arrow-callback': 'off',
110111
'perfectionist/sort-imports': 'off',
111112
'style/max-statements-per-line': 'off',
112113
'import/newline-after-import': 'off',

packages/spy/src/index.ts

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -121,27 +121,63 @@ export function createMockInstance(options: MockInstanceOption = {}): Mock<Proce
121121
}
122122

123123
mock.mockReturnValue = function mockReturnValue(value) {
124-
return mock.mockImplementation(() => value)
124+
return mock.mockImplementation(function () {
125+
if (new.target) {
126+
throwConstructorError('mockReturnValue')
127+
}
128+
129+
return value
130+
})
125131
}
126132

127133
mock.mockReturnValueOnce = function mockReturnValueOnce(value) {
128-
return mock.mockImplementationOnce(() => value)
134+
return mock.mockImplementationOnce(function () {
135+
if (new.target) {
136+
throwConstructorError('mockReturnValueOnce')
137+
}
138+
139+
return value
140+
})
129141
}
130142

131143
mock.mockResolvedValue = function mockResolvedValue(value) {
132-
return mock.mockImplementation(() => Promise.resolve(value))
144+
return mock.mockImplementation(function () {
145+
if (new.target) {
146+
throwConstructorError('mockResolvedValue')
147+
}
148+
149+
return Promise.resolve(value)
150+
})
133151
}
134152

135153
mock.mockResolvedValueOnce = function mockResolvedValueOnce(value) {
136-
return mock.mockImplementationOnce(() => Promise.resolve(value))
154+
return mock.mockImplementationOnce(function () {
155+
if (new.target) {
156+
throwConstructorError('mockResolvedValueOnce')
157+
}
158+
159+
return Promise.resolve(value)
160+
})
137161
}
138162

139163
mock.mockRejectedValue = function mockRejectedValue(value) {
140-
return mock.mockImplementation(() => Promise.reject(value))
164+
return mock.mockImplementation(function () {
165+
if (new.target) {
166+
throwConstructorError('mockRejectedValue')
167+
}
168+
169+
return Promise.reject(value)
170+
})
141171
}
142172

143173
mock.mockRejectedValueOnce = function mockRejectedValueOnce(value) {
144-
return mock.mockImplementationOnce(() => Promise.reject(value))
174+
return mock.mockImplementationOnce(function () {
175+
if (new.target) {
176+
throwConstructorError('mockRejectedValueOnce')
177+
}
178+
179+
return Promise.reject(value)
180+
})
145181
}
146182

147183
mock.mockClear = function mockClear() {
@@ -644,6 +680,12 @@ export function resetAllMocks(): void {
644680
REGISTERED_MOCKS.forEach(mock => mock.mockReset())
645681
}
646682

683+
function throwConstructorError(shorthand: string): never {
684+
throw new TypeError(
685+
`Cannot use \`${shorthand}\` when called with \`new\`. Use \`mockImplementation\` with a \`class\` keyword instead. See https://vitest.dev/api/mock#class-support for more information.`,
686+
)
687+
}
688+
647689
export type {
648690
Constructable,
649691
MaybeMocked,

packages/spy/src/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export type MockParameters<T extends Procedure | Constructable> = T extends Cons
4747
? Parameters<T> : never
4848

4949
export type MockReturnType<T extends Procedure | Constructable> = T extends Constructable
50-
? void
50+
? InstanceType<T>
5151
: T extends Procedure
5252
? ReturnType<T> : never
5353

test/core/test/mocking/vi-fn.test-d.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@ test('spy.mock when implementation is a class', () => {
1717
const Mock = vi.fn(Klass)
1818

1919
expectTypeOf(Mock.mock.calls).toEqualTypeOf<[a: string, b?: number][]>()
20-
expectTypeOf(Mock.mock.results).toEqualTypeOf<MockResult<void>[]>()
20+
expectTypeOf(Mock.mock.results).toEqualTypeOf<MockResult<Klass>[]>()
2121
expectTypeOf(Mock.mock.contexts).toEqualTypeOf<Klass[]>()
2222
expectTypeOf(Mock.mock.instances).toEqualTypeOf<Klass[]>()
2323
expectTypeOf(Mock.mock.invocationCallOrder).toEqualTypeOf<number[]>()
24-
expectTypeOf(Mock.mock.settledResults).toEqualTypeOf<MockSettledResult<void>[]>()
24+
expectTypeOf(Mock.mock.settledResults).toEqualTypeOf<MockSettledResult<Klass>[]>()
2525
expectTypeOf(Mock.mock.lastCall).toEqualTypeOf<[a: string, b?: number] | undefined>()
2626

2727
// static properties are defined

test/core/test/mocking/vi-fn.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -716,6 +716,54 @@ describe('vi.fn() implementations', () => {
716716
expect(callArgs).toEqual(['test', 42])
717717
expect(Mock.mock.calls).toEqual([['test', 42]])
718718
})
719+
720+
test('vi.fn() with mockReturnValue throws when called with new', () => {
721+
const Mock = vi.fn()
722+
Mock.mockReturnValue(42)
723+
expect(() => new Mock()).toThrowError(
724+
'Cannot use `mockReturnValue` when called with `new`. Use `mockImplementation` with a `class` keyword instead.',
725+
)
726+
})
727+
728+
test('vi.fn() with mockReturnValueOnce throws when called with new', () => {
729+
const Mock = vi.fn()
730+
Mock.mockReturnValueOnce(42)
731+
expect(() => new Mock()).toThrowError(
732+
'Cannot use `mockReturnValueOnce` when called with `new`. Use `mockImplementation` with a `class` keyword instead.',
733+
)
734+
})
735+
736+
test('vi.fn() with mockResolvedValue throws when called with new', () => {
737+
const Mock = vi.fn()
738+
Mock.mockResolvedValue(42)
739+
expect(() => new Mock()).toThrowError(
740+
'Cannot use `mockResolvedValue` when called with `new`. Use `mockImplementation` with a `class` keyword instead.',
741+
)
742+
})
743+
744+
test('vi.fn() with mockResolvedValueOnce throws when called with new', () => {
745+
const Mock = vi.fn()
746+
Mock.mockResolvedValueOnce(42)
747+
expect(() => new Mock()).toThrowError(
748+
'Cannot use `mockResolvedValueOnce` when called with `new`. Use `mockImplementation` with a `class` keyword instead.',
749+
)
750+
})
751+
752+
test('vi.fn() with mockRejectedValue throws when called with new', () => {
753+
const Mock = vi.fn()
754+
Mock.mockRejectedValue(new Error('test'))
755+
expect(() => new Mock()).toThrowError(
756+
'Cannot use `mockRejectedValue` when called with `new`. Use `mockImplementation` with a `class` keyword instead.',
757+
)
758+
})
759+
760+
test('vi.fn() with mockRejectedValueOnce throws when called with new', () => {
761+
const Mock = vi.fn()
762+
Mock.mockRejectedValueOnce(new Error('test'))
763+
expect(() => new Mock()).toThrowError(
764+
'Cannot use `mockRejectedValueOnce` when called with `new`. Use `mockImplementation` with a `class` keyword instead.',
765+
)
766+
})
719767
})
720768

721769
function assertStateEmpty(state: MockContext<any>) {

0 commit comments

Comments
 (0)