Better test.extend API

[MOZGIII]

Clear and concise description of the problem

I'd like to propose a better way of reusing the extend API.

The example illustrating both the new pattern and the pain points with the current implementation:

// Painpoint 1: vitest does not expose types I could use here; what is `task`, what is `use` - TypeScript can't tell.
export const myAppFn = (params: MyAppParams) =>
  async ({ task }, use) => {
    const state = runApp(params);
    await use(state);
    await state.cleanup();
  };

// Here is how the new API composition is proposed to work:
// Painpoint 2: no types for use here either.
export const withMyApp = (params: RunNodeParams): WithNode => ({
  myApp: myAppFn(params),
});

Usage:

import { expect, test, describe } from "vitest";
import { withMyApp } from "./testlib";

describe("a context with my app", () => {
  const extest = test.extend(withMyApp({ args: ["--dev", "--tmp"] }));

  extest("hello world with my app", async ({ myApp }) => {
    await myApp.boot;

    await expect(myApp.request("hello")).resolves.toBe("world");
  });
});

The idea here is that we could share the withMyApp functions that enable us to parametrize the test context without being tied to a particular describe/test fn. Normally you would shared test.extend(...) instead of that ... - but then when you need to have a describe.extend(...) or it.extends(...) you have to declare and expose them again.
Another thing about this is it allows parametrizing the context - see how we pass params to withMyApp call? These parameters can be tweaked per test.

Suggested solution

Actually, what's needed to use this today is to expose more internal types, like Fixtures, etc. Maybe some rearchitecting of the TypeSystem will be necessary to make the types more pleasant to use.

If thus pattern is enabled, it should also be documented.

Alternative

1. Let every use who implements this declare their own types - bad for many reasons.
2. Just don't do it - use before/after and etc.

Additional context

In my case, I want to e2e test a CLI app that spawns its own server and exposes a couple of sockets with HTTP servers on them. Not sure if it's relevant.

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that request the same feature to avoid creating a duplicate.

[sheremet-va]

If you just need to expose types, pr welcome. I don't see any new public APIs needed to implement what you want.

[MOZGIII]

Yeah, well, I guess exposing types would be enough here. That is if we don't want to recognize what I outlined as a pattern and do the work associated with that - docs, etc. So, I'll make the PR. What about the pattern though?

[sheremet-va]

Yeah, well, I guess exposing types would be enough here. That is if we don't want to recognize what I outlined as a pattern and do the work associated with that - docs, etc. So, I'll make the PR. What about the pattern though?

Your pattern is just a wrapper around test.extend. I think it's fine to leave implementation to users if the want to experiment with the API.

[MOZGIII]

Ok, I guess I've messed up formulating my point here, I was authoring this very late at night, and I think I didn't communicate my point straight (i.e. the title is misleading).

My point here is that vitest has issues with not exposing types here and there. I have personally stumbled upon a but too many instances where I want to extend vitest somehow, and the types are there but just not exported. This is the root cause, I'd say.

How to solve it? I was thinking if we have a particular pattern that is documented and actually used by people it would create sort of a QA pressure on keeping those types in check. As there is very little way of ensuring people will actually keep the types exported. You'd think it's common sense - but here we are, and this means there is some sort of fundamental issue with the current way design of the modules (i.e. it makes it easy to do this mistake and forget exporting the types) or it is a development culture issue (meaning people omit the type exports intentionally, and it passes the code review and QA). Either of these issues will be solved by pushing the pattern into the documentation, making it into a problem of documentation/implementation coherence, which is ultimately a problem that a lot more people tend to be involved in.

Am I making sense?

Also, sorry for the lengthy messages. I have a trail of thought here and it seems like the suggested solution doesn't make sense without me providing the context for how I got to it.

Also, the proposed solution is actually a partial one, since it will create this QA pressure just for this code area, while leaving the others uncovered. It seemed like a good idea still when I was originally writing it, however it doesn't anymore - I guess we should really try to look and see how to address the core problem.

I am, however, unsure this discussion can reach the whole core team behind vitest - as it is their input is what would be required to solve the lacking types issue now just once and for just these particular types, but once and for all.

[sheremet-va]

I am, however, unsure this discussion can reach the whole core team behind vitest - as it is their input is what would be required to solve the lacking types issue now just once and for just these particular types, but once and for all.

Fundamentally, I don't think we can solve this. Vitest team works in their free time and doesn't have to follow any strict rules. We can limit the code only with linters. QA is done usually by me and it's not unusual for me to miss something. I usually check PR description, so we can add bullet points to export types if there are any.

I think that with the current pace of development, it's fine to miss some things and add them in a fix later on.

If you have any problems with types, you can simply open an issue or even a PR to expose them. Just like here 😄

[MOZGIII]

Ah, I see! :D

So, since you are in charge of the QA, is there a reason not to expose all the types? What is the thinking process behind the decision to make only the surface types exposed, but no the inner ones? UPD: as I'm writing this, I've decided to dig in a bit into the code in this repo - and it doesn't look like the types that are not exposed are there... This makes me think it might a bundler configuration issue! I'd kindly ask you to take a look at the bundled version of the code (as one would see it when installing from npm) and confirm (or deny) whether the example pattern I outline would require types from even this project, or are they just oddly vendored into the bundle and thus unexported automatically?

---

Fundamentally, I'd love to open the PR and all, but unfortunately, with this approach to QA, I feel like, the burden to submit the PRs in this manner would be too much effort - they'd stack up, for what's usually is fixed by altering the contribution guidelines, or common sense. Or, in this case it might be an actual technical issue, which I couldn't actually anticipate was possible. The reason why I am trying to solve this "once and for all" is to actually reduce the maintenance effort. Even for working on free time, that's worth it (at least for me, usually). And I am certain we can improve things in this specific area without causing headaches for the contributors (and you in particular)!

[sheremet-va]

What is the thinking process behind the decision to make only the surface types exposed, but no the inner ones?

Because we risk creating breaking changes by exposing everything.

as I'm writing this, I've decided to dig in a bit into the code in this repo - and it doesn't look like the types that are not exposed are there...

Types are exposed manually here:

vitest/packages/vitest/src/types/index.ts

Line 4 in edb322f

export { expectTypeOf, type ExpectTypeOf } from '../typecheck/expectTypeOf'

I personally think exposing types manually is better for the end user because we control what is bundled in this case. We had a lot of releases where something that should not be bundled as part of the bundle.

Runner-related types are here:

vitest/packages/vitest/src/types/tasks.ts

Line 25 in edb322f

} from '@vitest/runner'

I guess technically it should be fine to expose everything with an asterisk here because if the package exposes it, then we can probably export it. But limit what's exported inside the package itself.

[sheremet-va]

I think this can be closed.