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

[Bug]: "TypeError: URL constructor: // is not a valid URL" while using data router #11911

Open
Blargel opened this issue Aug 20, 2024 · 9 comments · May be fixed by #11924
Open

[Bug]: "TypeError: URL constructor: // is not a valid URL" while using data router #11911

Blargel opened this issue Aug 20, 2024 · 9 comments · May be fixed by #11924
Labels

Comments

@Blargel
Copy link

Blargel commented Aug 20, 2024

EDIT: The report below is incorrect, please see this comment for the real issue.


What version of React Router are you using?

6.26.0

Steps to Reproduce

Luckily, this can be reproduced in the data router StackBlitz example.
https://stackblitz.com/github/remix-run/react-router/tree/main/examples/data-router

If you edit url in the preview pane to add double slash ("//") at the end, you will get an invalid URL error and no route will render, not even the default 404 page.

Expected Behavior

I expect that the application would render the 404 page. Alternatively, it would also be okay if react router removed extra trailing slashes automatically.

Actual Behavior

An error is being thrown. In the case of the StackBlitz, a developer error page comes up, and when dismissed, the application does is not render anything, not even the root route's Layout component. In the case of my own applications, one of them has the same behavior, and another somehow manages to render the root layout, but none of the links work. Both applications will display the same error in the console, if it's open.

I recognize that a double slash is indeed an invalid url. However, this is a potential thing that an end user can do, and that I as a developer cannot seem to handle on my own since the error is coming from react router's internals.

@Blargel Blargel added the bug label Aug 20, 2024
@Blargel
Copy link
Author

Blargel commented Aug 20, 2024

Also, interestingly, examples that don't use data router on StackBlitz appear to work fine so this seems to be something specific to data routers.

The auth example doesn't have this issue:
https://stackblitz.com/github/remix-run/react-router/tree/main/examples/auth

@timdorr
Copy link
Member

timdorr commented Aug 21, 2024

@brophdawg11 Maybe a regression from things changed around the time of #10005?

@brophdawg11
Copy link
Contributor

What version of vite are you using? The error stack comes from vite middleware so I'm not sure if this is related to RR code?

Screenshot 2024-08-22 at 10 52 07 AM

It also doens't look like it happens on Vite 5 - I crewated a brand new vite app from https://vitejs.dev/guide/#trying-vite-online and added a data router and trailing double slashes don't cause an issue:

https://stackblitz.com/edit/vitejs-vite-uwqys1

@Blargel
Copy link
Author

Blargel commented Aug 22, 2024

Oh dang, it looks like the error is different on StackBlitz than it is on my own apps. Sorry, I should've double checked that. Let me see if I can get a minimum repro based off my apps. The stack trace I get does point to node_modules/@remix-run/router/dist/router.js as the origin of the error

@Blargel
Copy link
Author

Blargel commented Aug 22, 2024

The error comes from using the navigate function provided by useNavigate or the Navigate component. Please see this StackBlitz: https://stackblitz.com/edit/vitejs-vite-ht64rf?file=src%2FHomePage.jsx

Navigation with either button will work initially, but if you add the double slash to the url in the preview pane, both of them will stop working. Your browser console will display the error.

@brophdawg11 brophdawg11 linked a pull request Aug 22, 2024 that will close this issue
@brophdawg11 brophdawg11 linked a pull request Aug 22, 2024 that will close this issue
@brophdawg11
Copy link
Contributor

We'll look into this in #11924 but I think in general I would recommend doing some app-level detection of these invalid URLs and trigger a hard reload to get the user back to a valid URL:

let { pathname, search, hash} = window.location
if (pathname.includes('//')) {
  window.location = pathname.replace(/\/{2,}/, '/') + search + hash
} else {
  hydrateRoot(el, <App />);
}

I'm curious the use-case is where you end up with a double slash in the first place?

@Blargel
Copy link
Author

Blargel commented Aug 22, 2024

Nothing in my application will cause this, but it's something a user can do if they felt like it. Which I guess someone did, because I have a Sentry error event with this error, haha.

@brophdawg11
Copy link
Contributor

If this is one singular occurrence from one user who seems to have manually manipulated the URL and gotten an error page, I'm not sure it's something that needs to be addressed at the React Router level to try to correct/sanitize invalid URLs. Can you fix this with the suggestion in #11911 (comment)?

@Blargel
Copy link
Author

Blargel commented Aug 29, 2024

That's a fair point. I personally don't mind if you don't address this. I just wanted to report it because it's an unhandled error. I forgot to acknowledge it but the suggested solution is perfectly acceptable to me and I'm already using it so thank you for that.

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

Successfully merging a pull request may close this issue.

3 participants