Allow matching newlines in splat portions of paths

[RobinClowers]

What version of React Router are you using?

6.21.1

Steps to Reproduce

When using relative child routes in separate components (a route renders a component that has its own Routes), whitespace seems to cause routes not to match when they should.

git clone https://github.com/RobinClowers/react-router-whitespace-bug.git
cd react-router-whitespace-bug
npm i && npm run dev

The vite dev server will pick a random port, open that url and click the various links on the page. Notice that "working path" displays name : "bar", but the two broken paths do not.

Expected Behavior

Both of the broken paths should render the Name component, since %20 and %0A are valid path segements.

Actual Behavior

When there is a trailing space, the deepest child is not rendered. In the newline case, the whole tree of routes is unmounted.

[brophdawg11]

👍 The space example in this is a duplicate of #10814 and has a fix prepared (and an experimental you can test against - see this comment)

Could you elaborate on your use-case for needing a newline in the path? That's a different underlying issue - not encoding related. It's that we currently use a dot (.) in the regex used to match splat portions of the URL, which matches any character except a newline.

I'm unsure if we'll want to change those underlying regexes without a compelling reason to include newlines in paths?

For the time being you can use patch-package to change the lines in compilePath to use [\s\S] as a workaround for match anything including newlines per https://www.regular-expressions.info/dot.html

[RobinClowers]

Great, I'll test that out shortly and let you know how it goes. It seems like newlines should be supported, since they are valid characters in paths. My specific case is I use user generated database object names in paths for my app, which can contain newlines (though I have no idea why you would want to do this!). It's definitely an edge case, but I don't see a downside to supporting it.

[brophdawg11]

Is it possible to strip the newlines prior to generating the routes or replace them with - or something to make it a non-issue?

though I have no idea why you would want to do this!

Totally agree - this is my main concern with adding support. Seems like it's a very small edge case so very little benefit to React Router apps as a whole - and introduces some level of risk by making regexes newline-inclusive. I think I'd prefer to move this to a Proposal Discussion if you're interested and if it gains enough traction there we can reconsider.

[RobinClowers]

We could definitely handle this in our code, but I feel like it's an odd limitation that is pretty arbitrary. I'm happy to move it to a proposal, did you want me to create that?

[brophdawg11]

I can actually just convert this over directly so we retain the context 👍 . In the meantime you could also give the patch-package approach a shot if you didn't want to pre-process those routes. I chatted with @mjackson about this yesterday too and we're in agreement that we don't think newlines in paths are common enough that we'd want to support them in the internal path matching (or even necessarily a good idea as you said) .

[RobinClowers]

Thanks for moving this to a discussion!

Just to restate my thoughts on this: React router is the most widely used router, and generally offers a ton of flexibility for users to build any sort of routing structure they can think of. I know from following Remix that you all are big fans of using platform standards as much as possible. Given that a newline, percent encoded as %0A is a valid character in a path, it follows that react router should support it. Sure, I could do some custom encoding to replace newlines, but why should I have to when a standard way to escape these characters already exists?

Another question worth asking here: what is the downside to supporting newlines? As Matt already pointed out, it's easy enough to use [\s\S] in a regex to do this matching.

[brophdawg11]

I can't speak for the team or @mjackson/@ryanflorence, but I can give you my personal response :)

you all are big fans of using platform standards as much as possible

I agree - but I don't interpret that as full 100% unfiltered raw browser behavior. React Router is an abstraction built on top of built-in primitives which are harder to work with directly (i.e., history) to make things easier, less confusing, etc. And, in some cases, it's opinionated about how to make things easier. In my opinion - putting newlines in your paths is confusing, potentially error-prone, and uncommon, so I don't see it as a large flaw in the current behavior.

React router is the most widely used router

This is the second reason I hesitate. Path matching is a very core bit of logic, and as we all know regex's can be fickle. So I always hesitate to make unnecessary changes deep down in the path matching logic. I'm even looking for as much real world testing of the current encoding changes in #10814 because I want to be very careful not to introduce regressions when touching such a core piece of logic. That's my reason for moving this to a discussion - if we get a large number of users requesting this then it could be revisited and potentially deemed "worth the risk." But as of now, with a single known use-case that can be solved with pre-encoding or patch-package, I'm not inclined to take the risk.

I know this probably isn't the answer you're looking for, but I hope you can appreciate where we're coming from as maintainers of such a widely used-library.

[RobinClowers]

I totally appreciate your concern about touching such core logic for an edge case. That said, it's hard to imagine how the change needed here would break anyone, unless they happen to be relying on the fact that newlines are ignored? If we wanted to be really safe, we could make it an option that is turned off by default.

I will also say that while having a newline in a path is odd and not common, I don't think it's "potentially error-prone," since it's part of the core browser standards, and all the major browsers support it.

[brophdawg11]

That said, it's hard to imagine how the change needed here would break anyone, unless they happen to be relying on the fact that newlines are ignored

React Router is downloaded over 10 million times per week, someone is always relying on every aspect of your API (see Hyrum's Law)

[RobinClowers]

I just discovered another bug in this same class of issues: path segments with %23 (the # encoded) also don't match correctly. Any chance we could get these issues fixed in the upcoming v7 release?