fix: allow async overrideResponse to be passed to msw handler (issue #1389)

[severinh]

Status

READY

Description

Fix #1389.

In MSW, a mock handler can access the response resolver info. For example, it can access the request body to change the response based on the request. Access to that request body is only possible through an async function call to json().

http.post("/some/path", async ({ request }) => {
  const requestJson = await request.json();
  // Do something with the request, such as producing a response based on it.
  return ...
});

In Orval, PR #1375 made it possible for the override in a MSW mock handler to be a function, and Orval passes the response resolver info to that function. However, Orval requires a synchronous method that returns a response object. It does not allow async functions.

Before this PR, it was not possible to migrate the above mock handler to Orval. The following code would not type check:

getSomePathMockHandler(async ({ request }) => {  // This does not type-check. Orval does not allow a Promise<SomeResponse> return type.
  const requestJson = await request.json();
  // Do something with the request, such as producing a response based on it.
  return ...
});

This PR changes the code generation such that both Promise<ResponseType> | ResponseType is allowed as a return type for the override. This mirrors what MSW does.

Related PRs

#1375

[soartec-lab]

Hi, @severinh I would like to understand the background a little more, so please tell me specifically when the problem occurs when it is not a Promise. Is it just that type checking doesn't work?

[severinh]

Hi, @severinh I would like to understand the background a little more, so please tell me specifically when the problem occurs when it is not a Promise. Is it just that type checking doesn't work?

It's not just the type-checking that does not pass. The whole mock handler does not work correctly when passing an async method that returns a Promise. Let's assume we ignore the type checking error. The problem is: When passing an async method, the overrideResponse function returns a Promise of a response, not a response object. Orval then directly passes that Promise into JSON.stringify.

Orval only supports passing a non-async method right now. The following currently works in Orval:

// Suppose the user passes the following overrideResponse function:
const overrideResponse = (info) => {
  // Cannot get the request body from info, because that would require the method to be async
  return {"field": "value"}
}
// Orval then does the following call
JSON.stringify(overrideResponse())
which outputs "{'field': 'value'}"

However, suppose the user passes an async function:

const overrideResponse = async (info) => {
  // Do something async with info, such as reading the request body.
  return {"field": "value"}
}
// Orval (before this PR) then does the following call
JSON.stringify(overrideResponse())
// This outputs "{}", which is wrong, because we're stringifying the Promise itself, not the awaited object.
// This needs `await overrideResponse()` to work

Hope that helps.

[soartec-lab]

@severinh

Thank you for your kindness. I understand that much, but what is the specific background of why I want to use Promise? In the current state, we are only dealing with simple objects or functions that return objects, but adding more `Promises' here will also increase complexity. I would also like to understand the benefits I can receive from this, so please let me know.

[severinh]

@severinh

Thank you for your kindness. I understand that much, but what is the specific background of why I want to use Promise? In the current state, we are only dealing with simple objects or functions that return objects, but adding more `Promises' here will also increase complexity. I would also like to understand the benefits I can receive from this, so please let me know.

Happy to clarify.

There are two concrete use cases:

1. Returning a response based on the request:

Here's what you can do in MSW:

http.post("/greeting", async ({ request }) => {
  const { firstName, lastName } = await request.json();
  return { greeting: `Hello ${firstName} ${lastName}` }
});

The request body is only available via request.json, which returns a Promise. And hence can only be called in an async method.

2. Making assertions in a mock handler on the request in a test:

Here's what you can do in MSW (though this use case is generally discouraged by MSW):

http.post("/greeting", async ({ request }) => {
  const { firstName, lastName } = await request.json();
  expect(firstName).toEqual("Test");
  expect(lastName).toEqual("Name");
  return getGreetingResponseMock();
});

Neither of these is possible if you want to use an Orval mock handler. That means you have to fall back to hard-coding the paths - and not having to copy-paste paths across the code base is a major reason for using Orval in the first place. :)

This is related to issue #1206 that I filed some time ago. Personally, I would have have advised against merging #1375, because it only allows partial flexibility. With the current method signature of Orval's mock handlers (introduced in #1375), you can only override the response body, but you cannot override the entire HttpResponse (for example to simulate a HTTP 500 error, or add some more response headers).

My preferred solution would have been for Orval mock handlers to look like this:

export const getGreetingMockHandler = (override?: GreetingResponse | HttpResponseResolver<{ ... }, GreetingRequest, GreetingResponse>) => {
  return http.post('/greeting', async (info) => {
    return typeof override === 'function'
      ? await override(info)
      : new HttpResponse(JSON.stringify(override !== undefined ? override : getGreetingResponseMock()), {
          status: 200,
          headers: {
            'Content-Type': 'application/json',
          },
        });
  });
};

// One could then use this Orval mock handler like this:
server.use(getGreetingMockHandler(async (info) => {
  // Can produce a response based on the request, make assertions, set custom headers, etc.
  return new HttpResponse(...);
});

This would be both simpler and more flexible than what #1375 introduced. But the problem is that since #1375 was already released in 0.29, this would be a backwards-incompatible change. Would that be a possibility? If yes, I'm happy to prepare a PR to make the mock handlers look like the above, solving #1206 too.

[soartec-lab]

@severinh

First of all, regarding 1, you want to perform calculations in request and calculate the value of the return type object, right? And, on the side that uses mocks, you can do the same thing by creating and using an object that is not a Promise from the beginning, right?

[severinh]

@severinh

First of all, regarding 1, you want to perform calculations in request and calculate the value of the return type object, right? And, on the side that uses mocks, you can do the same thing by creating and using an object that is not a Promise from the beginning, right?

Thanks for the patience. I'm afraid I don't fully understand the question. What do you mean by "on the side that uses mocks"? Could you explain how you would do 1) with an Orval-provided MSW handler like getGreetingMockHandler?

Maybe what could be helpful context: I could of course say: "Fine, for cases like 1) and 2), I just won't use Orval, and instead write the MSW handlers from scratch with http.post('/greeting', ...), hard-coding the HTTP verb post and path. That works.

But as I mentioned in #1206, the main reason why we use Orval is exactly because it lets us avoid having to hard-code HTTP verbs and paths like post and '/greeting' in application and test code, but use type-safe, generated code instead (getGreetingMockHandler). From the perspective of developers in our team, it's confusing if for some types of mocking cases, they can use the Orval-generated handlers, and for others, they have to use plain, unsafe MSW instead. Avoiding that is the main reason why I opened the ticket and PR. :)

[soartec-lab]

OK. In short, it's not clear whether you want to use Promise to optimize it individually for your complex project, or if it will benefit you as a whole. This change itself is minor, so I would like to merge it if there is any benefit. However, when using the original handler, if a response object or calculation is required, wouldn't it be sufficient to input the function? For example, in the case of getGreetingMockHandler, wouldn't it be sufficient to just pass the function without doing any asynchronous processing?

const fullname = (firstName: string, lastName: string) => {
  return { fullname: `${firstName} ${lastName}`
}

getGreetingMockHandler(fullname)