Not make the return type of a resolver void

[tnyo43]

Scope

Improves an existing behavior

Compatibility

• This is a breaking change

Feature description

(I'm not sure if this can be a breaking change.)

With typescript, we currently can make a handler as like follows:

type RequestBody = {};
type Params = PathParams<"login">;
type Response = { login: string | readonly string[] };

const goodHandler = rest.get<RequestBody, Params, Response>(
  "https://api.github.com/user/:login",
  (req, res, ctx) => {
    return res(ctx.json({ login: req.params.login }));
  }
);

But also the following handler is valid in typescript even thought its return type is void.

const badHandler = rest.get<RequestBody, Params, Response>(
  "https://api.github.com/user/:login",
  (req, res, ctx) => {} // <- it does not return anything
);

Is there any case that the return type of a handler should be void? Otherwise, I really appreciate if that void is omitted from the return type.

---

I have made a handler as like follows and of course it didn't work.
This kind of problem is likely to happen but it actually took a lot of time to find the cause.

const handler = rest.get<RequestBody, Params, Response>(
  "https://api.github.com/user/:login",
  (req, res, ctx) => {
    res(ctx.json({ login: req.params.login })); // it does not return anything
  }
);

[tnyo43]

FYI, the void comes from the ResponseResolverReturnType type.

msw/src/handlers/RequestHandler.ts

Lines 56 to 60 in e87e00b

	export type ResponseResolverReturnType<ReturnType> =
	| ReturnType
	| undefined
	| void

[kettanaito]

Hi, @tnyo43. Thanks for proposing this.

Historically, a resolver could not return anything if you wished to observe the request but don't affect it in any way. That changed since #1204, as now you have to explicitly tell MSW that you wish to perform the request as-is.

However, to keep backward compatibility, the void return type was left without changes.

I thought we tackled it in #1404, but based on the source code, we didn't:

msw/src/core/handlers/RequestHandler.ts

Line 35 in 61f6f3b

| void

Would you like to open a pull request that removes void from the return type of response resolvers? That'd be awesome!

[kettanaito]

As I'm looking into the open pull request, I'm discovering that simply removing the void from the return type won't be enough. Looks like there's no way in TypeScript to force a function to have a return statement.

In TypeScript, all of the following scenarios satisfy the void return type:

• A function doesn't have any return statement.
• A function has a return statement.
• A function has a return undefined statement.

TS Playground

Those also have no distinction in JavaScript, since all three effectively represent undefined being returned from a function. What surprises me is that those tree are matching void, which I assumed meant "nothing is returned", as in "no return statement present".

[kettanaito]

Looks like my initial assumption about the return intentions from the resolver needs adjustments. I mention it here #1207 (comment)

[kettanaito]

Sharing my thoughts on this.

I do like the default fallthrough behavior. I like default behaviors in general, and this one in particular. If you define a list of handlers, a matching request will flow through them from top to bottom:

rest.get('/resource', () => console.log('a')),
rest.get('/resource', () => console.log('b')),
rest.get('/resource', () => console.log('c')),

I think this is a great feature of the library, and a part of it is that it's implicit. I'm usually not a fan of implicit (read "magic") but this may be a rare exception.

Now, if we start demanding an explicit return all the time, this behavior loses part of its appeal, that being its succinctness.

rest.get('/resource', () => {
  console.log('a')
  return fallthrough()
}),
rest.get('/resource', () => {
  console.log('b')
  return fallthrough()
}),
rest.get('/resource', () => {
  console.log('c')
  return fallthrough()
}),

It becomes much more verbose to achieve the same thing.

At this point, it's not an API question but a behavior question. Let's compare these two behaviors.

Implicit fallthrough

Pros:

• Reasonable default behavior: I don't return any intention from the resolver, so the request continues to be resolved by MSW until I do.

Cons:

• Can be confused with a forgotten return, leading to unexpected behavior.

Explicit return/fallthrough

Pros:

• Easier to guard against forgotten return statements.

Cons:

• Effectively means that MSW now does nothing with the request unless you explicitly tell it to. This is quite a big philosophy and expectation change since MSW has always been "transparent", non-intrusive by design.
• Takes more code to achieve the same thing.

With this comparison in mind, I'm slowly leaning towards changing nothing. There's a positive jing, a negative jing, and there's also a neutral jing. This may be the case when no change is the best change.

But how do we guard against a forgotten return?

Alas, we don't. I know it may bite sometimes but a missing return is an easy thing to notice and should be the first thing a developer looks at. I'm considering this thinking flow:

1. I wrote the handler but it doesn't do anything.
2. This means I'm either: not telling MSW what to do with the request, or MSW is not set up correctly.
3. I check the first by looking at what I return from the resolver. If I forgot a return, I add it.
4. I check the set up following the docs and the Debugging runbook.

I know this can be improved but TypeScript doesn't give us any means to improve it without a significant sacrifice of the default behavior of the library. The gain is not worth the price here.

I will close this issue but we can discuss it still, maybe someone will find good points that'd prove me wrong. I'd like that.