feat(vitest): add return type and promisable mockFactory

[syi0808]

Description

Adjust MockFactoryWithHelper type and add PromiseMockFactoryWithHelper.

screenshot

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
• Ideally, include a test that fails without this PR but passes with it.
• Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Tests

• Run the tests with pnpm test:ci.

Documentation

• If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

• Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

Resolve: #6089

[syi0808]

I removed generic in importOriginal because of a type error, which could be a breaking change. I guess it's a typescript problem, but let's look at it more.

TypeError playground

[acidoxee]

My apologies if my initial issue wasn't clear, but what I meant with a "compatible" mock type was not only regarding the keys in the export map, but their values as well.

Ensuring compatible top-level keys would already be a welcome improvement of course, but this doesn't address the correctness of the exports themselves. I'd still be able to provide a completely false (type-wise) mock from the factory, and TypeScript wouldn't tell me anything about it. I would still need to manually add the Awaited<ReturnType<typeof importOriginal>> return type to each of my factories to ensure my mocks are correct.

I reckon being this strict would probably break some people's current types in tests, because they actually have no idea that their mocks are incomplete or plain wrong, but that's the point. I'd understand that some people would want wildly different mocks than their runtime version, even if it breaks their types and the initial module's signature, but in that case they can still escape type validation with a manual any, while the default could be strict adherence to the original module's signature. WDYT?

[syi0808]

It was a change to keep it as compatible as possible. For typescript users, I thought maybe it could be a breaking change.

I think there are advantages to both from different perspectives. If necessary, I will revise it according to the opinions of other users or maintainers.

[kettanaito]

I think this is a good change. any certainly doesn't belong in the return type of the mock factory function. It has to be T, where it stands for the module you've imported (I'm okay if that works only with dynamic imports).

Another related issue is that Vitest may not provide correct mock implementations for functions out of the box. So while you technically don't have to provide any factory function, the default Vitest behavior will result in a broken module.

Example:

// api.js
export const foo = { 
  bar: async () => 123
}

Here, foo.bar is a function that returns a Promise that resolves to 123. However, when mocked vi vi.mock()/vi.doMock(), it doesn't do that:

vi.doMock(import('./api.js'))

const { api } = await import('./api.js')
api.bar() // returns nothing, and is NOT an async function.

Vitest converts all functions to vi.fn() regardless if they are sync or async. In this scenario, I'd expect foo.bar to translate to vi.fn().mockResolvedValue(undefined).

Since the mock function is synchronous, assertions like this will fail:

await expect(foo.bar()).resolves.toBeUndefined()

[sheremet-va]

I think this is a good change. any certainly doesn't belong in the return type of the mock factory function. It has to be T, where it stands for the module you've imported (I'm okay if that works only with dynamic imports).

So, do you think users should be required to return every export even if it's not used?

Another related issue is that Vitest may not provide correct mock implementations for functions out of the box. So while you technically don't have to provide any factory function, the default Vitest behavior will result in a broken module.

I'd say it is an intentionally broken module, this is how automocking works (and it's even documented!)

Vitest converts all functions to vi.fn() regardless if they are sync or async. In this scenario, I'd expect foo.bar to translate to vi.fn().mockResolvedValue(undefined).

it is not possible to know if the function returns a promise without calling it which defeats the purpose of automocking. (You can look at the function type for AsyncFunction, but regular functions can also return Promises)

[kettanaito]

So, do you think users should be required to return every export even if it's not used?

Yes and no. Yes, because I like being explicit. I can do await importOriginal() and return/modify the original module in my factory. No, because Vitest provides automocking for exports, so, technically, all exports are already returned even if I haven't returned anything from the factory function.

I'm leaning toward No here, on the assumption that Vitest also provides the correct mock implementations for my exports (see the async function issue above). If it doesn't, I'd actually expect my factory function to type-error, letting me know that certain exports need my attention.

I'd say it is an intentionally broken module, this is how automocking works (and it's even documented!)

It's a nice gotcha!

it is not possible to know if the function returns a promise without calling it which defeats the purpose of automocking.

I presumed that was the reason. Thanks for clarifying!

[sheremet-va]

No, because Vitest provides automocking for exports, so, technically, all exports are already returned even if I haven't returned anything from the factory function.

This is not true, Vitest doesn't automock anything returned from the factory. That would require importing the original module. Vitest returns a proxy that throws an error if you try to access a property that you didn't return from the factory.

[kettanaito]

Sorry if I phrased it wrong. Here's what I meant: calling vi.doMock(import('./foo.js')) will automatically replace all exports in foo.js with Vitest mocks. This is the behavior I experience on 2.x.

[sheremet-va]

Looks like you have type errors now. You need to add ts-expect-error to

vitest/test/core/test/mocking/factory.test.ts

Line 58 in a7be50d

vi.mock('../../src/mocks/default.ts', () => null)

[sheremet-va]

I think the documentation should reflect the change. Both in type and description

[syi0808]

I think the documentation should reflect the change. Both in type and description

I've drafted the document.

[netlify]

✅ Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	679af8a
🔍 Latest deploy log	https://app.netlify.com/sites/vitest-dev/deploys/66b218b388f8360008152ea0
😎 Deploy Preview	https://deploy-preview-6139--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 site configuration.

[netlify]

✅ Deploy Preview for vitest-dev ready!

Name	Link
🔨 Latest commit	78143bd
🔍 Latest deploy log	https://app.netlify.com/sites/vitest-dev/deploys/66b21fc6f3ef170008689363
😎 Deploy Preview	https://deploy-preview-6139--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 site configuration.