Skip to content
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

x-target.3xx redirect to current page misbehaves when query params have different order #118

Open
lachlancotter opened this issue Dec 20, 2024 · 3 comments

Comments

@lachlancotter
Copy link

Thanks for this fantastic library. Love the elegance of your approach. However, I ran into some difficulty when needing to swap content or redirect based on the outcome of a form submission (similar case to #79)...

PR #84 introduces a special handling for a _self target on a 3xx redirect:

Any 3xx class status code redirecting to a different page will load the redirected URL in the browser window
Any 3xx class status code redirecting back to the current page will target my_form.

However the URLs are compared as strings:

function sameUrl(urlA, urlB) {
  return (stripTrailingSlash(urlA.pathname) + urlA.search) === (stripTrailingSlash(urlB.pathname) + urlB.search)
}

This makes the behaviour dependant on the order of query params in the current URL and the response location header. If the order differs, this behaviour no longer applies.

If the current page was reached via a form (as in my case), the order of query params depends on the order of form fields. In my case I was able to solve this by sorting my data and using hidden form fields. But that adds complexity and implicit dependencies that could be avoided by comparing the URL queries as Maps.

The meaning of query parameters should generally be order independent.

@imacrayon
Copy link
Owner

Hey! Thanks for the clear explanation. I totally agree, we need a more robust comparison method.
I’ve got some time set aside today to work down a few issues, this is on my priority list.

@lachlancotter
Copy link
Author

Awesome. Thanks! I wonder if would make sense just to leave out the query params entirely for the purposes of matching the current page. The base path is usually the significant part for routing.

@imacrayon
Copy link
Owner

@lachlancotter you’re right, in that case, I think we can do away with the sameURL and stripTrailingSlash functions and simply compare pathname for equality. That seems to be consistent with how my browser’s URL bar handles sameness.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants