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

Open
blikblum opened this issue Dec 16, 2019 · 7 comments · May be fixed by #270
Open

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

blikblum opened this issue Dec 16, 2019 · 7 comments · May be fixed by #270
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!

@HansBrende HansBrende linked a pull request Feb 2, 2022 that will close this issue
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