mswjs / msw

Industry standard API mocking for JavaScript.
https://mswjs.io
MIT License
15.97k stars 519 forks source link

fix: export `SetupServerCommonApi` publicly #2302

Closed ryanmagoon closed 1 week ago

ryanmagoon commented 1 month ago

Implements #2072

This enables custom interceptors to be used to create a setupServer function

kettanaito commented 1 month ago

Thanks for looking into this, @ryanmagoon! Let me think if this is the best course of action here. I dislike that one cannot have the API of setupServer and then swap the interceptors underneath. I think that's a DX limitation that can be avoided. The SetupServerCommonApi is environment-agnostic so it won't have some Node.js-specific features like server.boundary(), which is a shame (and also quite difficult to replicate on your own).

I think instead of exposing SetupServerCommonApi, we need to come up with a way to extend SetupServerApi with the list of custom handlers.

ryanmagoon commented 1 month ago

Would it be clean enough to optionally allow additional interceptors to be passed into SetupServerApi constructor and setupServer? Or is there some extra typing work there around the event map

setupServer having more params will be a breaking change obv

vishakha94 commented 1 month ago

@kettanaito Any updates on this?

kettanaito commented 1 week ago

I think instead of exposing the common API, we should restructure these setup classes so that each has a protected interceptor property. Protected properties can be overridden by derived classes, allowing you to create custom setup classes where you can just swap out the interceptor to any other BatchInterceptor of your choice.

class CustomSetup extends SetupServer {
  protected interceptor = new BatchInterceptor([...])
}

As the next step, we can simplify the conversion of setup classes to setupX(...handlers) APIs.

export function toSetupApi(SetupClass) {
  return (...handlers) => {
    return new SetupClass(handlers)
  }
}

This is a draft; the exact API may vary.

@ryanmagoon, would like to hear your thoughts on this. With my proposal, I'm trying to follow the "open to extension, closed to modification" principle to keep the setup APIs sane to work with.

ryanmagoon commented 1 week ago

Yeah that sounds way better, this is just what I had patched in to unblock myself 😅 will update

kettanaito commented 1 week ago

The interceptor property already exists on all those classes, just need to check if it's protected. We can iterate on the toSetupApi utility together!

ryanmagoon commented 1 week ago

will follow up with setup util for custom SetupServerApi classes