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

Broken Routing: Remix + React Router integration branch #4570

Closed
cephalization opened this issue Nov 11, 2022 · 4 comments
Closed

Broken Routing: Remix + React Router integration branch #4570

cephalization opened this issue Nov 11, 2022 · 4 comments
Labels
bug Something isn't working

Comments

@cephalization
Copy link
Contributor

What version of Remix are you using?

0.0.0-experimental-5b4bceda6

Steps to Reproduce

Within a Remix loader, call the redirect function with a fully qualified URL like https://google.com

Example loader:

export const loader = async ({ request }: LoaderArgs) => {
  const session = await requestSession(request);
  const auth = getAuthFromSession(session);

  // don't try to login again if already logged in
  if (auth) {
    return redirect("/");
  }

  // this hits a 3rd party api and returns a 304 redirect response with destination
  const res = await getLogin();

  if (res.redirected) {
    return redirect(res.url);
  }

  return redirect("/");
};

Expected Behavior

Calling redirect with a fully qualified url takes my user to that URL.

Actual Behavior

Calling redirect with a fully qualified url takes my user to my site url with the redirect url appended to the end

Like http://localhost:3000/https://google.com

@cephalization
Copy link
Contributor Author

I found the code causing this issue and wrote an integration test to verify.

I will play around with a few solutions. I assume this issue happens because in react router we mostly assume you are routing within an SPA but in remix that’s not necessarily the case.

@brophdawg11
Copy link
Contributor

This should be resolved in 1.8.0-pre.2 - @cephalization do you want to confirm your use-case?

@cephalization
Copy link
Contributor Author

Works great @brophdawg11 , thanks!

@machour machour added the awaiting release This issue has been fixed and will be released soon label Dec 1, 2022
@brophdawg11
Copy link
Contributor

Released in 1.8.0

@brophdawg11 brophdawg11 removed the awaiting release This issue has been fixed and will be released soon label Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants