Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add "SetupApi" base class #1445

Merged
merged 12 commits into from Nov 7, 2022
Merged

Conversation

Toxiapo
Copy link
Contributor

@Toxiapo Toxiapo commented Oct 26, 2022

Hi @kettanaito, I would like to hear your thoughts about the current setup and if I am on the right track. I am also curious if you mean to write the SetupApi as an interface and update the return types on setupWorker and setupServer or a base class and mimic the current logic into a class as I did here. However, this might be tricky to implement interceptor.on('request') for both the server and the worker.

We will have to create a custom setup specifically for Edge in the consuming app to consume the API.

import { SetupApi } from "msw"
import { FetchInterceptor } from '@mswjs/interceptor'

class SetupEdge extends SetupApi {
  constructor(handlers: RequestHandler[]) {
      super([ClientRequestInterceptor], handlers)
  }
  
   public listen(options: ...): void {
      // ...
      super.apply()
    }
  // ...
}

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 26, 2022

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 1b03936:

Sandbox Source
MSW React Configuration

@kettanaito
Copy link
Member

Hey, @Toxiapo. I'm so excited to see this in the works! Thanks for working on the changes, I will try to get to them today/tomorrow to give some feedback.

I am also curious if you mean to write the SetupApi as an interface and update the return types on setupWorker and setupServer or a base class and mimic the current logic into a class as I did here.

I was thinking of introducing a base class, then extending that class with specific SetupServerApi and SetupWorkerApi classes, and then returning their instances from the setupServer() and setupWorker() existing API to preserve the same public interfaces. I think a class is the best syntax to describe fixed life-cycles and behaviors, which is exactly what we're doing here.

src/createSetupApi.ts Outdated Show resolved Hide resolved
src/node/setupServer.ts Outdated Show resolved Hide resolved
src/node/setupServer.ts Outdated Show resolved Hide resolved
@kettanaito
Copy link
Member

Let me know when you would like a full review of this. Really exciting changes going on here 👀

src/node/setupServer.ts Outdated Show resolved Hide resolved
@Toxiapo
Copy link
Contributor Author

Toxiapo commented Oct 31, 2022

Yes, @kettanaito I think we can start the review! I left some comments that I thought might be useful, but most of the logic was kept the same.

@Toxiapo Toxiapo marked this pull request as ready for review October 31, 2022 16:58
@Toxiapo Toxiapo changed the title [WIP] feat: wip setupApi feat: wip setupApi Oct 31, 2022
@Toxiapo Toxiapo changed the title feat: wip setupApi feat: add setupApi Oct 31, 2022
@kettanaito
Copy link
Member

Thanks for moving forward with this, @Toxiapo! This looks great. I've had a chance to try it out locally, and also pushed a few changes regarding tests and child classes. One thing that remains is to distribute the interceptors properly (not all child classes rely on interceptors at all). I can pick this up once I have time next week so we can finish this.

@kettanaito kettanaito changed the title feat: add setupApi feat: add "SetupApi" base class Nov 7, 2022
@kettanaito
Copy link
Member

I've got some time to work on this today. Hope to wrap it up soon and prepare the next release.

kettanaito
kettanaito previously approved these changes Nov 7, 2022
@kristojorg
Copy link

Just getting back from some time off and super happy to see this get done! Thanks @Toxiapo and @kettanaito : )

@kettanaito kettanaito merged commit 85ba844 into mswjs:main Nov 7, 2022
@kettanaito
Copy link
Member

Thanks for making this happen, @Toxiapo! 👏

@Toxiapo
Copy link
Contributor Author

Toxiapo commented Nov 7, 2022

Woohoo, I am excited to try this! Thanks, @kettanaito!

@kettanaito
Copy link
Member

Released: v0.48.0 🎉

This has been released in v0.48.0!

Make sure to always update to the latest version (npm i msw@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.

@smblee
Copy link

smblee commented Nov 8, 2022

Hey guys! wondering what this would enable us to do? Is this for better edge support?

@kettanaito
Copy link
Member

Hey, @smblee. This would enable you to create custom setup* APIs that manage interception. One example of this is indeed creating a subset of setupServer that'd use different interceptors to run on the edge.

// Import the interceptors you want from @mswjs/interceptors
const edgeInterceptors = [fetchInterceptor]

class SetupEdgeServerApi extends SetupServerApi {
  constructor(handlers) {
    super(edgeInterceptors, handlers)
  }
}

export function setupEdgeServer(...handlers) {
  return new SetupEdgeServerApi(handlers)
}

Also note this is a rather advanced usage. If this doesn't make sense to you, it's very likely you don't need this API at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Edge Runtime (no http module)
4 participants