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

Multiple parameters suffixed with an asterisk does not match - works with v5/v3 #214

Closed
blikblum opened this issue Dec 16, 2019 · 8 comments
Labels

Comments

@blikblum
Copy link

The following pattern used to match:

let re = pathToRegexp("/files/:path*\\.:ext*", paramNames);

let match = "/files/my/photo.jpg/gif".match(re);

Version 3.0 works: https://codesandbox.io/s/old-thunder-x209j3nkko
Version 5.0 works: https://codesandbox.io/s/angry-bhaskara-gkzf3
Version 6.1 does not work: https://codesandbox.io/s/naughty-cherry-lgsiz

@blakeembrey
Copy link
Member

This is expected behaviour, see https://github.com/pillarjs/path-to-regexp/releases/tag/v6.0.0. Version 3 was the first version to introduce this piece of magic, but for 6.0 stabilizing I wanted to remove things that cause confusion and this was a big one.

@blikblum
Copy link
Author

According to https://github.com/pillarjs/path-to-regexp/releases/tag/v6.0.0 the v6 should be compatible with v2 but unfortunately is not the case.

In v2, without the period escape the following would match path and ext
See https://codesandbox.io/s/frosty-leftpad-x75l707pxw

let paramNames = [];
let re = pathToRegexp("/files/:path*.:ext*", paramNames);

let match = "/files/my/photo.jpg/gif".match(re);

But in v6 only path is matched:
See https://codesandbox.io/s/path-to-regexp-61-multiple-splat-no-escape-5k2b4

Notice that the period escape was originally suggested in this issue comment.

@blakeembrey
Copy link
Member

Ah, thanks. I see the issue. When you escape the previous character now, it ends up without a prefix or suffix effectively making it impossible to actually match a repeated group. We could default to / (configurable) as the prefix to support the existing use-case.

@blikblum
Copy link
Author

Is there a way to support multiple splat with current features, e.g., adding some more notation?

I don't mind if i need to change the pattern.

@mastermatt
Copy link

@blakeembrey should this be labeled as a valid bug that needs a fix?

@blakeembrey
Copy link
Member

Sure. It should be pretty easy to fix, but it'd be good to come up with possible notation too.

@mastermatt mastermatt added the bug label Jun 7, 2020
@HansBrende
Copy link

This would be fixed by #270!

@blakeembrey
Copy link
Member

Fixed in V7 by getting rid of implicit prefixes entirely. So in V7, I believe it would be /files{/:path}*.{:ext;/}*.

It should also be a bug to have a repeatable parameter without any separator, as :ext* was implying, and that is the case in V7. If you were to write {:ext}* it would throw Missing separator for "ext": https://git.new/pathToRegexpError.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants