-
-
Notifications
You must be signed in to change notification settings - Fork 517
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
fix: preserve search params in "onUnhandledRequest" messages #2128
fix: preserve search params in "onUnhandledRequest" messages #2128
Conversation
nagadevkrishna
commented
Apr 9, 2024
•
edited by kettanaito
Loading
edited by kettanaito
- Fixes Preserve the original request URL in "onUnhandledRequest" warnings #2125
…ve url in warning message
@@ -9,7 +9,9 @@ export function toPublicUrl(url: string | URL): string { | |||
|
|||
const urlInstance = url instanceof URL ? url : new URL(url) | |||
|
|||
const [, relativeUrl] = urlInstance.href.split(urlInstance.origin) |
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 does solve the problem but it changes the other use cases we have for toPublicUrl
.
I think a far better approach is to keep these changes scoped to the immediate problem area: the toPublic()
usage in the onUnhandledRequest.ts
.
msw/src/core/utils/request/onUnhandledRequest.ts
Lines 24 to 27 in e83a669
const url = new URL(request.url) | |
const publicUrl = toPublicUrl(url) | |
const unhandledRequestMessage = `intercepted a request without a matching request handler:\n\n \u2022 ${request.method} ${publicUrl}\n\nIf you still wish to intercept this unhandled request, please create a request handler for it.\nRead more: https://mswjs.io/docs/getting-started/mocks` |
Let's try something like this:
const url = new URL(request.url)
const publicUrl = toPublicUrl(url) + url.search
This way both relative and absolute URLs will be printed correctly but will also have any search parameters preserved.
Then, we can modify the tests in onUnhandledRequest.test.ts
to include two new scenarios:
- Unhandled request with a relative URL and search params;
- Unhandled request with an absolute URL and search params.
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 the suggestion. Made changes now and pushed to PR again.
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 tackling this, @nagadevkrishna! I left a suggestion, let me know what you think about it.
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 looks fantastic!
awesome! |
Don't close it just yet. I need to merge it so it gets to MSW. Be patient. |
Welcome to the contributors, @nagadevkrishna! 👏 |
Released: v2.2.14 🎉This has been released in v2.2.14! Make sure to always update to the latest version ( Predictable release automation by @ossjs/release. |