orval-labs/orval

[Proposal] possible to pass the intercept function to the `msw`mock handler

soartec-lab opened this issue ยท 8 comments

What are the steps to reproduce this issue?

This issue is new proposal

What happens?

This makes it easy to check whether this API is being called correctly in a test, for example.
Currently, to achieve this, it is not possible to use generated mocks, and necessary to implement them in tests each time.
By doing this, it becomes possible to intercept function execution, making testing easier.

import { Ok } from '/schemas'

getGetPetMockHandler({ message: 'hello'})

export const getPutPetMockHandler = (overrideResponse?: Ok | ((info: Parameters<Parameters<typeof http.get>[1]>[0]) => Promise<Ok> | Ok)) ) => {
  http.get('*/pets/:id', async (info) => {
    return new HttpResponse(
      JSON.stringify(
        overrideResponse !== undefined
          ? typeof overrideResponse === 'function'
            ? await overrideResponse(info)
            : overrideResponse
          : getGetPetResponseMock(),
      ),
      {
        status: 200,
        headers: {
          'Content-Type': 'application/json',
        },
      },
    )
  })
}

What were you expecting to happen?

I want to use it like below:

import { Ok } from '/schemas'
import { vi } from '/vitest'

const mockFn = vi.fn()

getGetPetMockHandler(
  {
    overrideResponse: { message: 'hello'},
    intercept: (info: any) => {
      mockFn(info)
    }
  }
)

export const getPutPetMockHandler = (
  {
    overrideResponse,
    intercept,
  }: {
    overrideResponse?: Ok | ((info: Parameters<Parameters<typeof http.get>[1]>[0]) => Promise<Ok> | Ok),
    intercept?: (info: Parameters<Parameters<typeof http.get>[1]>[0]) => void
  }
) =>
  http.get('*/pets/:id', async (info) => {
    if (intercept) {
      intercept(info)
    }

    return new HttpResponse(
      JSON.stringify(
        overrideResponse !== undefined
          ? typeof overrideResponse === 'function'
            ? await overrideResponse(info)
            : overrideResponse
          : getGetPetResponseMock(),
      ),
      {
        status: 200,
        headers: {
          'Content-Type': 'application/json',
        },
      },
    )
  })

Any logs, error output, etc?

none.

Any other comments?

If i make this change, it will be a breaking change.

What versions are you using?

Please execute npx envinfo --system --npmPackages orval,zod,axios,msw,swr,@tanstack/react-query,@tanstack/vue-query,react,vue and paste the results here.

Hi, @melloware
What do you think this?

Sounds reasonable to me

Thanks for taking the time to write the proposal, @soartec-lab!

I'd like to better understand what you'd like to achieve in the example you give. Couldn't you already achieve what you describe with the current API of orval?

getGetPetMockHandler((info) => {
  // You can do anything you want here, such as asserting that the API is called.
  return { message: 'hello'};
});

What worries me (as you already point out) is that the proposal would be a breaking change that will require developers who use orval to change many/all of their tests. And simple tests (without intercept) would require more boilerplate, such as getGetPetMockHandler({ overrideResponse: {...} }) instead of just getGetPetMockHandler({...}).

@severinh

Let me introduce you to that thinking.
I would like the function to run when the API is called, not if I call the handler function.
Using handler(function) will call function now. However, intercept is executed when the API is called.

Thanks @soartec-lab. I think there's a misunderstanding here.

When you pass a function to getGetPetMockHandler, this actually gets called not immediately, but whenever the API is called. Each time the application makes a request to the example pet endpoint, that function will be called.

getGetPetMockHandler((info) => {
  // This will be called each time a request to the API is made. You can anything you want to do here, such as making assertions.
  // You can for example even access the concrete request that the application made, via `info.request`.
  return { message: 'hello'};
});

@severinh

Exactly. I certainly misunderstood.
I thought this feature existed before, and it seemed strange, but now my misunderstanding has been cleared up.
Thank you let me know ๐Ÿ‘

But there is a further problem.
After making this improvement, I was trying to add status and header to the option type of the argument.
To add these, I think it is necessary to introduce an option type that involves breaking changes.
You previously said in #1206 that you also want to override status, but wouldn't you like to be able to write something like this in the end?

getGetPetMockHandler(
  {
    overrideResponse: { message: 'hello'},
    overrideStatus: 500,
    overrideHeader: { 'Content-Type': 'application/pdf' },
    intercept: (info: any) => {
      mockFn(info)
    },
  }
)

@soartec-lab You're right that being able to override more aspects of the HttpResponse could be useful. Right now our team does not have a strong need for this anymore, but others might.

However, I don't think passing an object with all kinds of override* fields that map to HttpResponse properties is a clean approach. You would end up building an unnecessary API clone/mirror of the MSW HttpResponse API. I think it's important that we have a clean separation of responsibilities between MSW and Orval. Orval should complement MSW, not replace/clone MSW APIs, or prevent developers from interacting directly with MSW.

To give developers more control over the MSW HttpResponse, I think the best approach would be for Orval to directly let developers provide that MSW HttpResponse object themselves, if they need to. Something like this:

getGetPetMockHandler((info) => {
  // if the developer wants, they can do anything here, such as inspect info.request
  return HttpResponse.json({ message: 'hello'}, { headers: 500, contentType: ... });
});

That would not require a complex new API - you'd just give Orval developers the option access the MSW API, if they want to. Btw, this is also the approach taken by other MSW integrations (such as for GraphQL).

Most importantly, Orval should keep the option to just use the existing, simple, very compact getGetPetMockHandler({message: 'hello'}). I'm sure that's what 95%+ of Orval developers mainly use, and this should not change. Requiring developers using Orval to replace these calls with getGetPetMockHandler({overrideResponse: {message: 'hello'}}) would not be well received due to the unnecessary boilerplate.

What do you think? I hope that's understandable. Happy to provide more details, but I'll be off the grid for the next 3 weeks.

@severinh

Thank you for your reply.
I can understand what you are saying. This isn't an urgent issue for me, so I'll wait for other people's feedback before considering it.
I think this approach is my make sense for now ๐Ÿ‘

getGetPetMockHandler((info) => {
  // if the developer wants, they can do anything here, such as inspect info.request
  return HttpResponse.json({ message: 'hello'}, { headers: 500, contentType: ... });
});