-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
Throw when the path is undefined in res.redirect. #6391
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
Comments
I think throwing an error will be great whenever path is undefined in res.redirect. |
Yep, it's pretty intuitive to throw an error rather than redirect to /undefined |
Agreed - that sounds like the kind of sneaky, unexpected behavior that is hard to debug. The path is also documented as required so an error would make sense (as a side note, I find it odd that the optional argument comes first for this method - that's not a great practice and most of the other methods don't do that). |
Interesting that this was deprecated in v4 and removed in v5 (See #2941), it probably should never have been that way. I also find it just as strange that the optional argument is the first one. |
Agreed on all of this. Let's keep that 6.x label on here, and when we start the branch we can open a PR for it. |
Oh! Ha, already done just no branch to make it against. We can close this then and just re-target that when we are ready. |
I think it would make sense to throw an error when the path is undefined in res.redirect. The current behavior redirects to /undefined, which I wouldn't consider very intuitive.
This functionality would be a breaking change, so it could be a candidate for Express 6. What do you all think?
The text was updated successfully, but these errors were encountered: