-
-
Notifications
You must be signed in to change notification settings - Fork 535
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
Conversation
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:
|
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 was thinking of introducing a base class, then extending that class with specific |
Let me know when you would like a full review of this. Really exciting changes going on here 👀 |
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. |
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. |
I've got some time to work on this today. Hope to wrap it up soon and prepare the next release. |
98a329e
to
8f72804
Compare
Just getting back from some time off and super happy to see this get done! Thanks @Toxiapo and @kettanaito : ) |
Thanks for making this happen, @Toxiapo! 👏 |
Woohoo, I am excited to try this! Thanks, @kettanaito! |
Released: v0.48.0 🎉This has been released in v0.48.0! Make sure to always update to the latest version ( Predictable release automation by @ossjs/release. |
Hey guys! wondering what this would enable us to do? Is this for better edge support? |
Hey, @smblee. This would enable you to create custom // 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. |
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 onsetupWorker
andsetupServer
or a base class and mimic the current logic into a class as I did here. However, this might be tricky to implementinterceptor.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.