-
-
Couldn't load subscription status.
- Fork 7
#18 - add onUnhandledRequest #19
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
base: main
Are you sure you want to change the base?
#18 - add onUnhandledRequest #19
Conversation
|
@kettanaito @zachatrocity |
|
I'm with ya @lukebickell ! I think things have slowed for @kettanaito with open source stuff. I'm still willing to actively support this pr I think I'm just kinda blocked waiting for reviews. Unless by chance you know of other maintainers that could review! |
src/index.ts
Outdated
| return | ||
| } else { | ||
| if (onUnhandledRequest) { | ||
| onUnhandledRequest(fetchRequest) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe we need to invoke this manually. If you provide it to worker.start(), MSW will apply it for you. All you need here is a type-safe onUnhandledRequest option in the args.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kettanaito I tried removing this but was having issues getting the test to fire the onUnhandledRequest.
Correct me if I'm wrong but it doesn't seem like msw/playwright leverages handleRequest so we never get to the lookup logic in msw/core that executes the onUnhandledRequest
It seems we use the getResponse but to parse and return handler values but rely more on the native playwright page.route to trigger and return requests. It would be nice to let msw invoke onUnhandledRequest so please let me know if I'm missing something
For now I've explicitly invoked it and added a test so we can see how we'd like to proceed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this, @zachatrocity. It took me some time to get to this as you've rightfully concluded I'm a bit busy with personal stuff and trying to focus on that as much as I can.
I've reviewed the changes, you can continue with reusing the OnUnhandledRequest-related types from msw now!
|
No worries @kettanaito! Personal stuff always comes first. I'll get this updated Monday! |
| }) | ||
| return | ||
| } else { | ||
| if (typeof onUnhandledRequest === 'function') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is quite less than ideal, waiting on closure in this thread from @kettanaito for guidance on how to proceed.
This is a PR to add the basic
onUnhandledRequestfunctionality as part of #18Pending this PR: mswjs/msw#2562