remix-run / react-router

Declarative routing for React
https://reactrouter.com
MIT License
53.06k stars 10.31k forks source link

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

Open Blargel opened 2 months ago

Blargel commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

brophdawg11 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 https://github.com/remix-run/react-router/issues/11911#issuecomment-2305529047?

Blargel commented 2 months ago

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.