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

Invalid prop path of type regexp supplied to Route, expected string #6418

Closed
Fi1osof opened this issue Oct 23, 2018 · 5 comments
Closed

Invalid prop path of type regexp supplied to Route, expected string #6418

Fi1osof opened this issue Oct 23, 2018 · 5 comments

Comments

@Fi1osof
Copy link

Fi1osof commented Oct 23, 2018

Version

PropTypes require String or [String] for Route, but Route allow use regex too.

Test Case

https://codesandbox.io/s/yv2z0x7qo1 (See route for topics).

Expected Behavior

No warnings.
Allow types: String, RegExp, [String, RegExp]

Actual Behavior

Warning: Failed prop type: Invalid prop path of type regexp supplied to Route, expected string.
in Route

react-router-dom@4.3.1

@Fi1osof Fi1osof closed this as completed Oct 23, 2018
@Fi1osof Fi1osof reopened this Oct 23, 2018
@timdorr
Copy link
Member

timdorr commented Oct 24, 2018

Regex isn't allowed in the path. We cache our generated regexps, so this would break that.

@timdorr timdorr closed this as completed Oct 24, 2018
@Fi1osof
Copy link
Author

Fi1osof commented Oct 24, 2018

@timdorr, so this is undocumented features. And it should be allowed. Because it works. Have a look: https://codesandbox.io/s/yv2z0x7qo1
I switch URLs /topics/topic-234.html and /topics/topic-CharID.html and got 234 and CharID in separated components. Switch component uses regexp https://github.com/ReactTraining/react-router/blob/master/packages/react-router/modules/matchPath.js#L42

@pshrmn
Copy link
Contributor

pshrmn commented Oct 24, 2018

It may technically work, but it shouldn't be encouraged.

path-to-regexp (what React Router uses for path matching) supports custom match parameters if you want to only match specific content in a segment.

Using a RegExp path can also cause issues. What happens if you want to nest a route as a child of a route whose path is a RegExp? With strings you can grab match.path and concatenate, but there is no simple way to join regular expressions.

@Fi1osof
Copy link
Author

Fi1osof commented Oct 25, 2018

@pshrmn, thank you for the clarification. OK.

@kevich
Copy link

kevich commented Oct 26, 2018

I think technically there is no problem to convert RegExp to strings on react-router level, so that it will behave pretty the same as now. Even concatenating is ok, since even now there is kind of support RegExp in stings with custom match parameters, and it will be the same issue either with text with RegExp or real RegExp. So it should be up to developer not to shoot into his own leg with that concatenations.

Still my point is if react-router uses path-to-regexp, it should support all of it's features.

But for what it's really needed to be supported as RegExp by react-router is because of this issue in pillarjs/path-to-regexp#99. And for now there is no other way to use (?!something) than using it as RegExp

@lock lock bot locked as resolved and limited conversation to collaborators Dec 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants