react-router icon indicating copy to clipboard operation
react-router copied to clipboard

[Bug]: Path with emojis no longer working in v6

Open SevenOutman opened this issue 3 years ago • 23 comments

What version of React Router are you using?

6.2.1

Steps to Reproduce

https://stackblitz.com/edit/github-ksmc1e?file=src/App.tsx

  • Declare a route with path set to "🔨"
  • Visit path "/🔨" (by clicking on the "About (🔨)" link)
  • "About" page is not shown and the "🔨" route is not matched

Expected Behavior

Routes that has a path with emoji should be matched as they were in v5.

Actual Behavior

Routes that has a path with emoji is not matched.

SevenOutman avatar Jan 13 '22 06:01 SevenOutman

Paths with umlauts (äöüÄÖÜ) or the Scandinavian letter ø aren't working either. I suspect these issues are related.

marcusleg avatar Jan 13 '22 12:01 marcusleg

You are not able to use _ or @ as the first character as well. I really need to use it and have no idea how

seeden avatar Jan 20 '22 19:01 seeden

It seems to be working properly in version 5. but in version 6 it is broken.

Wei12345 avatar Jan 27 '22 05:01 Wei12345

This is because the package path-to-regex was used in v5, which has been replaced in v6 with a rather underpowered alternate function that allegedly does the same thing. It doesn't.

It's a bit of a Not invented here antipattern, the way I see it.

thany avatar Feb 09 '22 14:02 thany

@marcusleg this may be a solution meanwhile : https://github.com/remix-run/react-router/issues/8824#issuecomment-1112444059

laem avatar Apr 28 '22 16:04 laem

This issue bit us -- it would probably be a good idea to note it in the upgrading docs or release notes. Definitely a breaking change!

sandinmyjoints avatar May 20 '22 20:05 sandinmyjoints

Doesn't look like the team is interested in fixing this bug. Is there any news about it?

mleister97 avatar Jul 10 '22 15:07 mleister97

Fix in the meantime:

<Route path={encodeURIComponent('page-with-umlauts-äöü')} element={<Page />} />

Be aware that if you use route params, that you only use encodeURIComponent for the part with the special characters:

<Route path={`${encodeURIComponent('äöü')}/:param`} element={<Page />} />

mleister97 avatar Jul 10 '22 18:07 mleister97

Encountered the same issue, but with the special character included in the param, e.g.:

// Works if param === "mathematics"
// Error if param === "matemáticas"
<Route path="/study/:param" element={<Study />} />

In our case, the param can be in either English or Spanish, and therefore can have á, é, ñ, etc. Has anybody solved how to fix this if the breaking character(s) come from a route param instead of a hard-coded part thereof?

ogdendavis avatar Jul 28 '22 20:07 ogdendavis

Revert to v5 seems to be the way to go, so long as the authors refuse to fix this (that seems to be the case, so please prove me wrong).

thany avatar Aug 01 '22 14:08 thany

Have any of you that are complaining attempted to solve the issue on your own?

It's not a matter of "authors refuse to fix this"... but that they have limited resources and have to prioritize which bugs/features to focus on. And remember, they are doing this for you for free.

kiliman avatar Aug 01 '22 14:08 kiliman

I have attempted, but there is no way to make react-router "break out of" its constrains with that underpowered function that replaced path-to-regex, other than by patching the source code and essentially reverting the decision to cripple path parsing.

I seriously doubt that a PR containing such reversion will ever be accepted.

Btw, mind you, Remix is a for-profit company. They make money indirectly (and directly, in the past) from Remix the framework.

thany avatar Aug 01 '22 14:08 thany

How do you know? If you solve the problem others are having, I'm sure they'll be happy to consider it. And even if they don't at least your app will continue to work. And the fix will most likely be a few dozen lines of code compared to the several hundred lines that you didn't have to write.

As for whether Remix is a for-profit company is immaterial. You are still getting the software for free. And it doesn't entitle you to free support either. Sure, if the software is buggy, then they'll lose reputation and people will switch to other libraries. I'm sure they are aware of that and try to resolve issues in a timely manner, but just like everyone else, they have limited resources and have to prioritize.

Yes, it sucks that the update broke your existing app. But I don't think they ever claimed it will be a 100% smooth migration from one major version to another.

Anyway, all I'm saying is that you're ultimately responsible for your own app. If the library doesn't work as needed, then your choices are either: fix it yourself, use something else, or complain and do nothing.

BTW: I was one of the first to pay for a Remix license, so I put my money where my mouth is.

kiliman avatar Aug 01 '22 15:08 kiliman

@kiliman - The tool used to do the thing. People therefore began to rely on it to do the thing. A change to the tool broke it so that it no longer does the thing that people now rely on it to do. Why are you angry that people are upset that the tool that used to do the thing no longer does the thing? Also, you may need to re-read the thread. Plenty of evidence of people finding workarounds/solutions, here.

ogdendavis avatar Aug 01 '22 18:08 ogdendavis

How do you know? If you solve the problem others are having, I'm sure they'll be happy to consider it. And even if they don't at least your app will continue to work. And the fix will most likely be a few dozen lines of code compared to the several hundred lines that you didn't have to write.

You're misunderstanding. Remix took effort, time, and money (their time is worth money), and knowlingly and willingly made the product worse. They did so by removing a perfectly working subdepedency (path-to-regex) and replacing it with their own function that is absolutely tiny in comparison, and doesn't support even half of what it used to.

I don't have a problem with a product that happens to have a bug. It gets fixed and that's that. But this bug has been introduced on purpose. That's got nothing to do with entitlement from my end, and has everything to do with incompetence, unwillingness, or mismanagement on their end. You wouldn't break your product on purpose without something in your team going really badly wrong.

As for whether Remix is a for-profit company is immaterial. You are still getting the software for free.

It doesn't matter if it's free or not. If you get a lemonade for free, and it doesn't taste like lemonade at all, the guy who gave you that drink can still be blamed - certainly not you. Also try not to confuse responsibility with liability.

And it doesn't entitle you to free support either.

The Github Issues section is meant for exactly that, depending on what exactly you mean by "entitle" - if you meant "it's there, people can use it" then yes, it absolutely does. If you meant "Remix is legally required to help you" then no, of course not. That's why Remix is not liable, but they are still responsible.

But I don't think they ever claimed it will be a 100% smooth migration from one major version to another.

Well, actually... The migration guide says nothing about path parsing having become severely (or even slightly) more limited. Not a word. At least not at the time I myself upgraded - I haven't checked recently.

thany avatar Aug 04 '22 13:08 thany

For a reason I don't understand, updating to the last version breaks the hack fix I found here https://github.com/remix-run/react-router/issues/8824#issuecomment-1112444059

:/

What about mentioning in the docs that this package is only compatible with english language websites ?

laem avatar Oct 05 '22 16:10 laem

What about mentioning in the docs that this package is only compatible with english language websites ?

Nah, that's not even true. There's no reason English text only ever contains letters from the "regular" 26-letter alphabet. It's perfectly reasonable to find other characters in English text, such as brackets. And of course it's perfectly reasonable to have a non-English proper name. There are loads of reasons why a totally English website would need non-English and/or non-alphanumeric characters in routable page titles.

thany avatar Oct 12 '22 14:10 thany

Can folks try this on 6.4.2? I'm pretty sure this is resolved with https://github.com/remix-run/react-router/pull/9300.

I'd also suggest taking a deep breath and bringing down the temperature in threads like this. React Router has been downloaded almost a billion times at this point. There are constantly new issues being opened and unfortunately we're a small team with lots of varying priorities, so our top priorities may not always align with specific concerns for your application.

Additionally, no matter what decisions are made when it comes to major version changes (and the potential for breaking changes), there will always be an unhappy contingent of users. I can promise you that aggressive and condescending language in issue comments is not the way to get traction on your specific concerns. As @kiliman mentioned, a PR with an attempt to fix this issue or even a PR with a failing test would have a been a great first step 😄

brophdawg11 avatar Oct 12 '22 22:10 brophdawg11

That was hardly a proper fix. It just "allows" more characters and doesn't solve the root issue. The issue is that there is effectively a whitelist of characters that are now allowed.

Expanding this whitelist is not the way to go. This only passes the problem down to future developers.

I'd also suggest taking a deep breath and bringing down the temperature in threads like this.

Sure. But then may I suggest to never again replace perfectly working modules by severly limited custom-made ones, without mentioning it as a breaking change in the changelogs? I'm pretty much convinced that's where this heat is being caused.

there will always be an unhappy contingent of users

This is a scenario where a module that was working perfectly, was willingly and knowingly replaced by one that is objectively worse, without so much as a mention in the changelogs of what this change actually means. So yes, of course existing users will be unhappy, you got that absolutely right 😉

React Router has been downloaded almost a billion times at this point.

Download count is not neccesarily proportionate to quality. It's only an indication.

As @kiliman mentioned, a PR with an attempt to fix this issue or even a PR with a failing test would have a been a great first step 😄

As I mentioned, the only true fix is to revert replacing this module and putting path-to-regex back where it was.

thany avatar Oct 12 '22 23:10 thany

I'm not sure I follow @thany - the fix in #9300 very specifically removes the use of a list of allowed characters. This was previously only required due to the word boundary in the regex. Removing the word boundary meant we no longer needed the positive lookahead for the allowed characters.

The removal of path-to-regexp was documented in the migration guide. It's a great library, but we don't agree that it's the only way to do path matching.


As to the original issue reported here (paths with emoji's, umlaut's, params starting with @/_ etc.) we made some changes to solidify encoding/path matching in 6.4.3-pre.1 that we'd love some community eyes on to make sure they're handling your use cases. At the moment we think all the cases reported in here are handled but please test out against your specific routes and let us know if any of them aren't working! Here's a stackblitz that has a handful of these types of routes setup against 6.4.3-pre.1, so you can easily fork this and test in there: https://stackblitz.com/edit/github-ksmc1e-wekxcf

Thanks all!

brophdawg11 avatar Oct 25 '22 22:10 brophdawg11

The removal of path-to-regexp was documented in the migration guide. It's a great library, but we don't agree that it's the only way to do path matching.

Maybe so, but not in a way that indicates in any meaningful way that path matching may be severly limited and you might have to change your entire app structure and urls. It feels more like a footnote, and not a well explained one at that.

As to the original issue reported here (paths with emoji's, umlaut's, params starting with @/_ etc.) we made some changes to solidify encoding/path matching in 6.4.3-pre.1 that we'd love some community eyes on to make sure they're handling your use cases.

But seriously, please explain why you don't want to go back to the (again..) beautifully and demonstrably perfectly good path-to-regex package? Surely you're not suffering from the Not Invented Here syndrome, I would hope?

Between the lines, I read it as if the change was to simplify path matching. But if that's the case, then I have to ask, who exactly asked for this? Was there a really popular issue where it was asked to replace path-to-regex with a more simplisitc variety?

thany avatar Oct 26 '22 10:10 thany

The reasons are listed out in the migration guide:

  • In v6 we are using a simpler syntax that allows us to predictably parse the path for ranking purposes
  • It also means we can stop depending on path-to-regexp, which is nice for bundle size.

At this point, we've addressed the original issue, and the thread has shifted to a discussion around implementation details. We're happy with the simpler implementation, and we are not interested in bringing back path-to-regexp.

Please keep further conversation focused on the stated issue itself, and we're happy to look into any special-character path-matching issues you may run into in 6.4.3-pre.1.

brophdawg11 avatar Oct 26 '22 14:10 brophdawg11

The reasons are listed out in the migration guide:

  • In v6 we are using a simpler syntax that allows us to predictably parse the path for ranking purposes
  • It also means we can stop depending on path-to-regexp, which is nice for bundle size.

Simpler isn't always better (this issue is a preem example, innit).

You still haven't explained why you replaced path-to-regexp. Reason 1 is a non-reason - people were already using it and migrating is more work than NOT maintaining it (the package is external after all). Reason 2 is just a consequence of excuse number 1 - it's easier or simpler. Of course it is, it's severely limited. No wonder the syntax is easier. In the same way that a webcam is easier to operate than a DSLR.

We're happy with the simpler implementation, and we are not interested in bringing back path-to-regexp.

You're happy with it? Good for you. What about the users? Do you think they are too? Have you checked this issue?

Please keep further conversation focused on the stated issue itself, and we're happy to look into any special-character path-matching issues you may run into in 6.4.3-pre.1.

Please do. So bring back path-to-regexp and stop these nonsensical excuses around just wanting to build that package yourselves for no reason.

It's a bit of a Not invented here antipattern, the way I see it.

I stand by this.

thany avatar Oct 27 '22 11:10 thany

Hey folks! We just released 6.4.3 with these changes so emoji and other special character paths should work now. Please let us know if you find scenarios that don't match properly!

brophdawg11 avatar Nov 01 '22 15:11 brophdawg11