✅ Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	3fd07f7
🔍 Latest deploy log	https://app.netlify.com/projects/vitest-dev/deploys/6963490abdf06a00087245fd
😎 Deploy Preview	https://deploy-preview-9430--vitest-dev.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

In JavaScript, if a constructor explicitly returns an object, that object is used as the result of the new expression, overriding the default instance. In your example, res would actually be { hello: ... }, not the Klass instance.

Furthermore, this PR aligns with the patterns recommended in the Vitest documentation. For instance, Vitest suggests mocking implementation like this:

const spy = vi.spyOn(cart, 'Apples')
    .mockImplementation(function () {
        this.getApples = () => 0  
    })

Using mockReturnValue({ getApples: () => 0 }) is essentially a shorthand for the above.

Using mockReturnValue({ getApples: () => 0 }) is essentially a shorthand for the above.

In what world is it a shorthand? mockImplementation(function) is not the same as mockReturnValue(object). The object is not an instance of the class:

const CorrectDogClass = vi.fn(function (name) {
  this.name = name
})

const IncorrectDogClass = vi.fn(name => ({
  name
}))

const Marti = new CorrectDogClass('Marti')
const Newt = new IncorrectDogClass('Newt')

Marti instanceof CorrectDogClass // ✅ true
Newt instanceof IncorrectDogClass // ❌ false!

This is why documentation uses mockImplementation. The documentation had this example as an anti pattern, maybe it needs to be returned even after classes support was added: https://v3.vitest.dev/guide/mocking.html#classes

In JavaScript, if a constructor explicitly returns an object, that object is used as the result of the new expression, overriding the default instance. In your example, res would actually be { hello: ... }, not the Klass instance.

You are right, it only ignores non-object values. In any case, the above example makes your pattern invalid and introduces behavioral differences with production, which is something we do not want to encourage.

The core philosophy of mocking is often to provide a simplified version of a dependency that satisfies the specific requirements of the test. In many unit tests, we only care about the interface/shape of the returned object (Duck Typing) rather than its internal prototype chain. Forcing developers to maintain the prototype via mockImplementation adds unnecessary boilerplate when the test doesn't rely on instanceof checks.

Furthermore, relying on instanceof during mocking is often unreliable anyway. Even if we use a function constructor, the prototype chain can still break due to how Vitest handles mocked modules. For example:

// moduleA.js
export class Apple {}

export function isApple(apple) {
    return apple instanceof Apple;
}

// test.js
import { vi, test, expect } from "vitest";
import { Apple, isApple } from "./moduleA.js";

vi.mock("./moduleA.js", async importOriginal => {
    const module = await importOriginal();
    return {
       ...module,
       Apple: vi.fn(function () {}),
    };
});

test("isApple", () => {
	const mockedApple = new Apple();
	isApple(mockedApple); // false
});

While using mockReturnValue might introduce a behavioral difference in terms of inheritance, it's a conscious trade-off for test readability and simplicity. Since this is valid JavaScript behavior and the alternative doesn't even guarantee instanceof consistency, could we consider supporting it with a note in the documentation instead of outright forbidding it?

While using mockReturnValue might introduce a behavioral difference in terms of inheritance, it's a conscious trade-off for test readability and simplicity.

I would disagree that it is a conscious trade-off, many people do not realise the difference which this PR and existence of a note in the documentation proves. I believe that returning objects from a constructor is an anti-pattern and I would like to enforce it properly.

Since this is valid JavaScript behavior and the alternative doesn't even guarantee instanceof consistency, could we consider supporting it with a note in the documentation instead of outright forbidding it?

It is an unrelated and documented quirk of module mocking which is not related to @vitest/spy implementation. Replacing vi.fn with a function will have the same result.

When a user explicitly passes a Plain Object to mockReturnValue, they are making a deliberate choice. It is highly unlikely that a developer would provide a { key: value } literal and expect it to magically inherit the class prototype or pass an instanceof check. In this context, the behavior is transparent and unlikely to cause confusion.

It is an unrelated and documented quirk of module mocking which is not related to @vitest/spy implementation. Replacing vi.fn with a function will have the same result.

As demonstrated by the module mocking example, since instanceof consistency is already 'broken' by the nature of mocking environments, enforcing a strict prohibition here feels like an unnecessary constraint that doesn't actually solve the underlying 'identity' issue.

If the goal is to prevent users from shooting themselves in the foot, a warning or documentation note would be a more balanced approach. Forcing a hard error for valid JavaScript behavior—especially when the 'correct' alternative is more verbose and still doesn't guarantee instance identity—seems to prioritize dogmatism over the practical needs of the user.

We'll discuss with the team what makes the most sense during one of the next meetings

When a user explicitly passes a Plain Object to mockReturnValue, they are making a deliberate choice. It is highly unlikely that a developer would provide a { key: value } literal and expect it to magically inherit the class prototype or pass an instanceof check. In this context, the behavior is transparent and unlikely to cause confusion.

If is likely - this is why the note in the docs exist, there was an issue from a user that expected it. Tell the AI that wrote the response that it's tripping.

Forcing a hard error for valid JavaScript behavior—especially when the 'correct' alternative is more verbose and still doesn't guarantee instance identity.

It does guarantee instance identity because it is separate from the module mocking use case, which I repeat again, is a separate issue not related to how the spy returns the value. If you want to go into that discussion, then I would rather remove module mocking alltogether.

seems to prioritize dogmatism over the practical needs of the user

I do not see anything wrong with this, sometimes it is better to have strict checks rather that a loose environment. We do not prioritize convenience over everything else. Features have to make sense when they work together. After many years of "let's just do what people want", the project has become hard to maintain and some features make very little sense. The rewrite of spying in Vitest 4 is a step in the direction where Vitest is more dogmatic and strict.

alternative is more verbose

Let's not make it out like a few extra characters is the end of the world:

vi.spyOn(module, 'foo').mockImplementation(class { hello = () => 'world' })
vi.spyOn(module, 'foo').mockReturnValue({ hello: () => 'world' })

Your issue is clearly unintended behaviour, but how it will be resolved is not as clear as you are trying to portray.

It seems we have different ideas about 'freedom' and 'strictness'. I respect your vision for Vitest 4. I will wait for your meeting result. Thank you.

The team has decided that it is more harmful to allow class.mockReturnValue because of the unexpected nature of return value of the constructor. When a user does spyOn().mockReturnValue, they don't think that it is a return value of the constructor, and not an instance of the class. What we decided should be implemented:

Vitest docs should have a page explaining what happens in mockReturnValue (and other aliases like mockResolveValue), all helper functions that return a value should throw an error when a function is called with a new keyword, for example:

mock.mockReturnValue = (value) => {
  return mock.mockImplementation(function () {
    if (new.target) {
      throw new Error(`Vitest's "mockReturnValue" is not supported when used with a "new" keyword because of unexpected nature of return values in constructors. Use "mockImplementation(class { ... })" instead to keep the correct prototype chain.\nSee: <link to docs>`)
    }
    return value
  })
}

There should also be more tests of these methods when called on a class. And the type error from this PR should also be fixed.

If user actually want to test what happens when constructor returns a value, they can still do this in mockImplementation:

mock.mockImplementation(class {
  constructor() { return { hello: () => 'world' } }
})

We don't think that an argument to save 9 characters is justifiable when the consequences of calling mockReturnValue on a class are so unexpected. Doing an instanceof check is one the most used features of a class, and no one on the team actually agrees with the statement that "when a user explicitly passes a Plain Object to mockReturnValue, they are making a deliberate choice" - users actually assume a different behavior.