feat: remove void type from the ResponseResolverReturnType

[tnyo43]

• close Not make the return type of a resolver void #1671

This change will enforce the return type of resolvers not to be void.

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

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

// the following handler will be invalid by this PR.
const invalidHandler = rest.get<RequestBody, Params, Response>(
  "https://api.github.com/user/:login",
  (req, res, ctx) => {} // <- it does not return anything
);

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit d444bcf:

Sandbox	Source
MSW React	Configuration

[kettanaito]

Hey, @tnyo43. This is a great change! My only feedback is that we should open it against fetch/standard-api (#1436) and not the current main.

The current version of MSW should allow void as the response resolver's return type. If we remove it, it'd be a breaking change, which I'd rather avoid at the moment.

The next version, which is currently in progress under #1436, reworks the library in a major way, including more explicit intentions returned from the response resolver. The change you proposed falls nicely under that next version, so I'd be glad to include them there.

[tnyo43]

@kettanaito Thanks for your feedback! I made a fix commit and changed the base branch.

[kettanaito]

I'm thinking if undefined shouldn't be removed as well. The current return type allows for this:

rest.get('/resource', () => {
  // side-effect
})

Edit: That use case is still allowed. See this for reference. You can return nothing (but you do have to have a return!) to fall through handlers.

[kettanaito]

Thanks for addressing the feedback, @tnyo43! Welcome to contributors 🎉

[kettanaito]

I've looked into the failing tests and learned that return statement without any returned value is considered void by TypeScript. Since we removed that, we are forcing all the users to do return undefined, which is unnecessary:

This screenshot illustrates that a simple return statement does not comply with the Body | undefined return type. It requires an explicit void in the return type to validate.

Looks like we have to keep the void in the return type after all. I wonder if there's a way in TypeScript to force a function to have a return statement so we could guard against the use case that you mentioned in the original issue.

[tnyo43]

I wonder if there's a way in TypeScript to force a function to have a return statement

I understand that omitting void from the return type is an way to do, although we need to add return undefined.

In my opinion, to force add return undefined instead of just return is not that bad since it makes obvious that the handler is "undefined". However it is a kind of breaking change.

Also, the fallthrough you mentioned in #1207 looks nice. If there's anything I can do to help, I'd like to help.

[tnyo43]

I will close this PR as well as #1671.