-
Notifications
You must be signed in to change notification settings - Fork 623
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_req(http/routes): add a accept filter #5883
Comments
AFAIK, quite a lot of other routers in the JS ecosystem just match the URL and method. |
This addition seemed obvious to me at first, but as pointed out, routers usually don't support this. Then I started to wonder why they don't, and I came up with the arguments below. If we support Those things can become hard to express in terms of a router - and arguably they go beyond a router's purpose. A router is intended to handle routes - "paths", to put it in a simplistic way. |
It wouldn't necessarly mean supporting all headers, I didn't explicitely mentioned it but I was actually targetting for content negotiation filtering, (which is kind of common, scoped but also standardized), but indeed it'd at least means that Cookies, custom and others headers, etc. are not part officially part of content negotiation so would remain user's responsability if they want custom processing But anyways as @iuioiua mentioned, you can achieve something somewhat similar with the following: {
pattern: new URLPattern({ pathname: "/api/foo" }),
handler: request => {
if (accepts(request.headers, "application/xml")
return new Response("<foo>bar</foo>")
if (accepts(request.headers, "application/json")
return new Response('{"foo":{"bar"}}')
return new Response(null, {status: 406 })
}
}, But it does make the code a bit less readable if you have to duplicate the checks for each route. Maybe the solution would be some kind of simple middleware system, but I don't know if it's possible to do something that stays generic and simple enough. After a certain point it may just be better to just use a fully fledged router |
I didn't know about Content Negotiation. That makes sense. I guess the behavior for wildcards like If my route is Also, the request can have many encodings in no particular order: This could work, but I think we should consider those "edge cases" in advance and explain the behavior in the docs so that it's clear for users 👍. |
I'd say it'd match any, but it'll imply that users orders their routes from most specific to most generic, but this is already the case with routes anyway as they early break on first matching route: Lines 97 to 98 in 4209011
But tbh
Hum it depends, some browser will automatically appends the wildcard, and it can be implicit when not specified
The parsing is already implemented in std/http/negotiation so it'd mostly get back to the original post idea where you pass an array of possible values, and it matches depending on what the |
Thanks for the clarification. I just thought of another concern: the fact that we have to replicate the route to be able to handle a different header value. Could it end up growing exponentially? If our API can return XML and JSON in English and French, then I guess we would need to have each route replicated 4 times, one for each combination of const routes: Route[] = [
{
pattern: new URLPattern({ pathname: "/api/foo" }),
accept: ["application/xml"],
acceptLanguage: ["fr"],
handler: () => new Response("<foo>bonjour</foo>"),
},
{
pattern: new URLPattern({ pathname: "/api/foo" }),
accept: ["application/xml"],
acceptLanguage: ["en"],
handler: () => new Response("<foo>hello</foo>"),
},
{
pattern: new URLPattern({ pathname: "/api/foo" }),
accept: ["application/json"],
acceptLanguage: ["fr"],
handler: () => new Response('{"foo":{"bonjour"}}'),
},
{
pattern: new URLPattern({ pathname: "/api/foo" }),
accept: ["application/json"],
acceptLanguage: ["en"],
handler: () => new Response('{"foo":{"hello"}}'),
}
]; And if you want to respond in French by default in case the request doesn't specify a language, you'd have to duplicate the routes once more without Now I'm inclined towards a middleware approach too, but as you said, it's hard to make it generic. There was an initiative before but it was canceled I think. |
|
Is your feature request related to a problem? Please describe.
Current implementation is supporting http method discriminant probably covers most use-case, but there's also one more use-case which I feel like is pretty common is Accept header discriminant.
Sometimes, same route path may serve different content based on Accept headers (for example a nice page for users when
text/html
is present, and a json object for developers whenapplication/json
is passed.Describe the solution you'd like
Example: sending different format based on requested accept types:
Behaviour could be:
accept
is not specified, keep current behaviour, ie: execute handler regardless of client headeraccept
is specified, only execute handler if one of the requested types is matching the specified arrayWhich also means that it's possible to specify the same route multiple time with different accept headers, and possible specify one without if you want a default handler for this route
Describe alternatives you've considered
Not using
route
or handle in default handler but less elegantThe text was updated successfully, but these errors were encountered: