feat: support parameter patterns #362
Conversation
|
@Kakulukian I wanted to let you know ASAP that I really appreciate the PR! It is the right direction, but unfortunately it's even more involved than this because I want to add it back in a way that ensures it still generates safe regexes. That sadly means more of a regex parser. The good news, if you're interested, is that we could start with a very simple "parser" by rejecting all meta characters except Assuming you're up for it, I'll also separately leave some review comments. |
src/index.ts
Outdated
| @@ -153,7 +156,29 @@ function* lexer(str: string): Generator<LexToken, LexToken> { | |||
| throw new TypeError(`Missing parameter name at ${i}: ${DEBUG_URL}`); | |||
| } | |||
|
|
|||
| return value; | |||
| if (chars[i] === "(" && options?.pattern) { | |||
There was a problem hiding this comment.
Take a look at the previous code here: 60f2121#diff-a2a171449d862fe29692ce031981047d7ab755ae7f84c707aef80701b3ea0c80L155
I'd use that as a starting point, e.g. move the ( into the lexer itself (not part of name) and yield a PATTERN directly. Handle the fact that PATTERN is expected in a certain position in the if (param) below by doing tryConsume("PATTERN").
That's ok for me, updated with your suggestions :) |
src/index.ts
Outdated
| while (i < chars.length && depth > 0) { | ||
| const char = chars[i]; | ||
|
|
||
| if (INVALID_PATTERN_CHARS.includes(char)) { |
There was a problem hiding this comment.
Unfortunately it’s more complex than this since you can be escaping a meta character to make it a valid character to use, so we actually need to iterate over the string and do some light parsing.
I’d also recommend leaving the parsing out of this section, as it should be part of the logic translating it into a regex as the library does accept raw tokens.
src/index.ts
Outdated
| @@ -579,7 +626,14 @@ function toRegExp(tokens: Flattened[], delimiter: string, keys: Keys) { | |||
| } | |||
|
|
|||
| if (token.type === "param") { | |||
| result += `(${negate(delimiter, isSafeSegmentParam ? "" : backtrack)}+)`; | |||
| if (token.pattern) { | |||
| result += `(${token.pattern})`; | |||
There was a problem hiding this comment.
We should do the validation of the pattern here instead. Create a new function for validation so it can be expanded over time without refactoring inside this existing code.
… escaping support, support for wildcard
I'm planning to move to the newer version of path-to-regexp. In our codebase heavily relies on regex for parameter validation, especially in some legacy components we still have running in production.
I stumbled across this comment that mentions parameter pattern support might be coming in future versions. Based on that, I went ahead and implemented the functionality in this PR.
Feel free to ignore this PR if it doesn't align with the project's direction. I understand you may have different plans for handling parameter patterns in future versions!