-
Notifications
You must be signed in to change notification settings - Fork 32
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: hardcoded redirection http status #608
Conversation
…ers to be specified for .redirect() Trying to make this more like the response class' body methods.
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.
My bad lol
src/types.ts
Outdated
export type HttpStatusRedirection = | ||
| 300 | ||
| 301 | ||
| 302 | ||
| 303 | ||
| 304 | ||
| 305 | ||
| 307 | ||
| 308; |
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.
Question about this. I know that 3xx are the redirect status codes, but do you think we should be so strict about the status codes? Maybe someone wants to mask the url and not give a 3xx status code. I'm not sure...
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 see. yea maybe we shouldn't be strict in this regard. i don't know if a use case outside of 3xx, but maybe there is one
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.
Yes, my question is exactly that... Don't know if it's a valid thing or not. I'm for the restriction of 3xx, but I'm not sure if there's a use case.
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.
Closes #607
Summary
resource.redirect()
methodstatus
andheaders
to be specified inresource.redirect()
(just like the response class' methods)status
can only be of typeHttpStatusRedirection
which ranges from 300 - 308 (excluding 306 since it's not a used status)Other Notes
resource.redirect()
method in favor ofresponse.redirect()
sinceresponse.redirect()
is the native way to do it based on MDN docs. I didn't knowresponse.redirect()
existed, so my bad on opting forresource.redirect()