-
-
Notifications
You must be signed in to change notification settings - Fork 562
feat: custom request predicate function #2541
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: custom request predicate function #2541
Conversation
src/core/handlers/HttpHandler.ts
Outdated
| export type HttpCustomPredicate = (args: { | ||
| request: Request | ||
| cookies: Record<string, string> | ||
| }) => boolean | Promise<boolean> |
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.
params is always an empty object when using a custom predicate because path is a function, so users cannot actually use it for any matching logic.
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.
A good point. I wonder if we can do something to help with this. I think exporting a function that parses any request URL against any path would be nice. That way, if the developer is using a custom predicate function, they can still parse out the request url into params.
We have matchRequestUrl() exported from msw to do precisely that!
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 am making the custom predicate return type a union of boolean or { match: boolean, params: PathParams } so the user can provide an extended predicate if they still wish to preserve path parameter parsing.
They should be able to do that using the matchRequestUrl function from MSW:
import { matchRequestUrl } from 'msw'
http.get(({ request }) => {
return {
matches: myCustomLogic,
params: matchRequestUrl(request.url, '/:foo').params,
}
})
commit: |
src/core/http.ts
Outdated
| method: Method, | ||
| ): HttpRequestHandler { | ||
| return (path, resolver, options = {}) => { | ||
| if (typeof path === '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.
What do you think if we handle this branching in the HttpHandler (if not RequestHandler) instead?
- Rename
pathargument topredicate. - Handle it being a function in the
.predicate()method.
This way, these namespace functions don't have to become more complex.
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 completely agree. That is a much cleaner approach.
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.
Hi, @ytoshiki. This looks amazing! I left a few comments, would love to hear your thoughts on them.
|
One great way to reduce complexity in APIs that accept variable input (like our predicate) is to bring all that input to a single shared denominator. In our case, that would be the predicate function! Basically, translate Does this make sense? |
Are you suggesting that the HTTP Handler's constructor should always create a predicate function like below? this.predicateFunction = typeof predicate === 'function'
? predicate
: (args) => {
const hasMatchingMethod = this.matchMethod(args.request.method)
const hasMatchingUrl = matchRequestUrl(new URL(args.request.url), predicate).matches
return hasMatchingMethod && hasMatchingUrl
}So, the type check just moves from the predicate method to the constructor. the logic itself doesn't go away. |
This reverts commit 4452a33.
@kettanaito
As for caching, I commented below. |
|
Thank you so much for your work, @ytoshiki. I will get to this as soon as I have a moment. Would love to get this released. |
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've pushed a few minor changes here and there, otherwise this looks great!
Released: v2.11.0 🎉This has been released in v2.11.0! Make sure to always update to the latest version ( Predictable release automation by @ossjs/release. |
pathis a string/regexp. Do not check them when using a custom predicate (Allow query parameters in request handlers (resource paths in Sveltekit) #2573)Overview
Support for custom predicate functions in both HTTP and GraphQL request handlers
Custom Predicate Support
Type Definitions
requestandcookies(notparams, which was always {} and thus removed for clarity).request,query,operationName,variables, andcookies.