React-router: Update/Fork path-to-regexp (v3 released Jan 13, 2019)

[jeffvandyke]

I'm aware an attempt at v2 was made, but those discussions were closed and locked (by lock bot) before mid-2018. Perhaps it's time to re-investigate version 3? [email protected] still uses [email protected] (latest is version 1), which happens to not have a LICENSE file published.

If the desire is to stick with a particularity of path-to-regex, then rather than remaining locked to an older version, I recommend path-to-regex be forked into this project at that version and at least updating its dependencies, because depending on stale packages tends to causes more issues with the ecosystem and other packages as time moves on, as I'm starting to see with isarray.

[timdorr]

The biggest reason we can't upgrade is they removed /foo* matching, which is a pretty common use case. Also, they handle /foo/ and /foo as two separate things, which would cause a ton of confusion for users here.

I'd say we fork it to maintain separately.

[jeffvandyke]

I'd say that forking would be a better alternative than holding an package at an older version that continues to bit-rot over time.

Since the path-to-regexp repo is essentially 1 file + types, license, tests, and 1 troublesome isarray dependency (to be upgraded >= 1), it may be simplest and best to copy it to react-router's codebase.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[jeffvandyke]

Wait stale-bot, I have more! Ok, so researching a little further, path-to-regexp is only a named dependency of packages/react-router. I'm not sure why website/yarn.lock is referencing versions at 0.1.7 and 1.0.1, in addition to 1.7.0, the one react-router wants.

Uncertainty about website/yarn.lock aside, this still needs to be done, and perhaps packages/react-router/modules/pathToRegexp.js is the right place for this fork to live.

[TPXP]

Hello there! I'm jumping into this path-to-regexp dependency chat to raise a point in favor of upgrading or at least bringing changes to this dependency. 👋

Recently, path-to-regexp brought in support for non-matching parenthesis, which enables some shortcuts in regexp paths. For example, the following:

<Route path={['/:courseName/:quizType((final|intro)Quiz)/:quizId(\\d+)']}>
  <div>Hello, there</div>
</Route>

will match /aa/a(finalQuiz)/1 instead of the intended /aa/finalQuiz/1 when using version 1.7.0 of the dependency. Instead you need to expand the | in the parenthesis explicitely, which can quickly grow in length (even if possible as shown in https://github.com/pillarjs/path-to-regexp/issues/95#issuecomment-400725757).

https://github.com/pillarjs/path-to-regexp/commit/1327699b5c946c262134bdff9af924b09199a9f8 makes the following work, with a good workaround to me (note the ?: at the beginning of the inner parenthesis, which indicates a non-matching group, thus not breaking pathToRegexp key matching).

<Route path={['/:courseName/:quizType((?:final|intro)Quiz)/:quizId(\\d+)']}>
  <div>Hello, there</div>
</Route>

This can be a substancial improvement when dealing with multiple possibilities or parenthesis, it would be great if react-router could bring support for this. Additionally, this point has already been raised in https://github.com/pillarjs/path-to-regexp/issues/95, so I believe I'm not alone to wish to use this kind of matching. 😄

[TPXP]

Also, if I may, I believe one point raised by @timdorr is not anymore valid:

Also, they handle /foo/ and /foo as two separate things, which would cause a ton of confusion for users here.

I did the test with RunKit, and it seems the generated regexp will match paths with or without the trailing slash.

It's true they removed /foo* matching though, it will cause an exception. You can work around this with /:foo(foo)* for example.

[lameimpala]

Is there any update on the status of this issue? The latest version of path-to-regexp breaks react-router.

[casperOne]

At the very least, can a section be put in the readme indicating that if you want to reference path-to-regexp in your project (while using React Router) you must stick to a specific version?

It seems like a pretty significant pain point which can be mitigated easily if moving to the most recent version of path-to-regexp (or forking it) isn't feasible at this time.

[wintercounter]

This needs to be resolved somehow, it'll cause more and more issues by the time goes on. I hate I cannot have an up-to-date codebase because of this. I think the best would be to live with the fact of introducing breaking changes and release a new major version. All the mentioned issues can be worked around even if you need to use some shitty regexp.

[lameimpala]

Do the maintainers not care that this module is currently in a broken state? Redirects are no longer functional.

[timdorr]

Redirects work just fine. I just wrote one yesterday.

[lameimpala]

It must be a conflicting dependency then. I see path-to-regexp is specified as the correct version in react-router, but I have seen others on the internet with the same issue after path-to-regexp was upgraded to version 6.1.0. Not sure its related, but I get an error when using redirects that the import for path-to-regexp is not a valid module.

[lameimpala]

The only other package I'm using that uses path-to-regexp is express, but I wouldn't think that would cause a conflict.

[wintercounter]

The next version (v6) is completely ditching path-to-regexp from it's
dependencies.

On Fri, Apr 24, 2020, 04:58 David Daniel [e-mail] wrote:

This is causing an issue for us at our company. We have modules that use
path-to-regexp at ^6.1.0 but the hoisted package continues to be 1.7.0 for
due to react-router. If we feel like this upgrade is not feasible in a
patch update, maybe with the next major version change, please change it to
a peer dependency so we can avoid this.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/ReactTraining/react-router/issues/6899#issuecomment-618774242,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAHLJQDN5FKM3SKNSVQYEJLROD555ANCNFSM4IUI2YEA
.