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

Describe how to redirect from load function but preserving query #2490

Closed
AndreasHald opened this issue Sep 24, 2021 · 3 comments
Closed

Describe how to redirect from load function but preserving query #2490

AndreasHald opened this issue Sep 24, 2021 · 3 comments

Comments

@AndreasHald
Copy link

Describe the problem

I just spent a lot of time debugging an issue where apparently our url query was being url encoded twice.

The issue stemmed from the following approach in a load function

export const load = async ({page}) => {
  if (page.path === '/actions' {
    return {
       status: 301,
       redirect: `/actions/board?${page.query}`,
    }
  }
...
}

However when calling toString on an instance of UrlSearchParams the resulting string is url encoded, and it appears that the string returned as redirect is also encoded causing it to be encoded twice.

Describe the proposed solution

The most awesome approach would be kit detecting this issue and when compiling inserting decodeURI

 return {
   status: 301,
   redirect: `/actions/board?${decodeURI(page.query.toString())}`,
 };

However you could argue that this would be too much "magic".

Alternatives considered

An alternative approach could be a secondary argument for the return type of a load function that preserves existing query params on redirect

 return {
   status: 301,
   redirect: `/actions/board`,
   preserveQuery: true
 };

However this could be argued as inelegant, as it would both be an unnecessary property in 90% of the load cases, and it would be difficult it you wanted to preserve the existing query, and append a parameter

Importance

would make my life easier

Additional Information

As a minimum I would suggest updating the docs here to make it clear that the string is url encoded and you don't have to do it on your end.

@odama626
Copy link

In my opinion, the most user friendly thing would be to not automatically encode the redirect url in the first place, that seems like some unexpected magic to me.

@kamholz
Copy link

kamholz commented Nov 17, 2021

Agreed. I just got bitten by this and it took a long time to track down the issue. This sort of auto encoding isn't standard or expected as far as I know. And you'd think generating the query from URLSearchParams should be supported, but right now it isn't. I'd call this a bug, not just lack of documentation.

@Rich-Harris
Copy link
Member

At a certain point we stopped URL-encoding redirects, so this should work correctly:

export const load = async ({page}) => {
  if (page.url.pathname === '/actions' {
    return {
       status: 301,
       redirect: `/actions/board?${page.url.searchParams}`,
    }
  }
...
}

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

4 participants