Skip to content

feat: support parameter patterns #362

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,10 @@ Parameter names must be provided after `:` or `*`, and they must be a valid Java

Parameter names can be wrapped in double quote characters, and this error means you forgot to close the quote character.

### Unterminated parameter pattern

Parameter patterns must be wrapped in parentheses, and this error means you forgot to close the parentheses.

### Express <= 4.x

Path-To-RegExp breaks compatibility with Express <= `4.x` in the following ways:
Expand Down
46 changes: 46 additions & 0 deletions src/cases.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,20 @@ export const PARSER_TESTS: ParserTestSet[] = [
{ type: "text", value: "stuff" },
]),
},
{
path: "/:locale(de|en)",
expected: new TokenData([
{ type: "text", value: "/" },
{ type: "param", name: "locale", pattern: "de|en" },
]),
},
{
path: "/:foo(a|b|c)",
expected: new TokenData([
{ type: "text", value: "/" },
{ type: "param", name: "foo", pattern: "a|b|c" },
]),
},
];

export const STRINGIFY_TESTS: StringifyTestSet[] = [
Expand Down Expand Up @@ -270,6 +284,16 @@ export const COMPILE_TESTS: CompileTestSet[] = [
{ input: { test: "123/xyz" }, expected: "/123/xyz" },
],
},
{
path: "/:locale(de|en)",
tests: [
{ input: undefined, expected: null },
{ input: {}, expected: null },
{ input: { locale: "de" }, expected: "/de" },
{ input: { locale: "en" }, expected: "/en" },
{ input: { locale: "fr" }, expected: "/fr" },
],
},
];

/**
Expand Down Expand Up @@ -376,6 +400,28 @@ export const MATCH_TESTS: MatchTestSet[] = [
],
},

/**
* Parameter patterns.
*/
{
path: "/:locale(de|en)",
tests: [
{ input: "/de", expected: { path: "/de", params: { locale: "de" } } },
{ input: "/en", expected: { path: "/en", params: { locale: "en" } } },
{ input: "/fr", expected: false },
{ input: "/", expected: false },
],
},
{
path: "/:foo(\\d)",
tests: [
{ input: "/1", expected: { path: "/1", params: { foo: "1" } } },
{ input: "/123", expected: false },
{ input: "/", expected: false },
{ input: "/foo", expected: false },
],
},

/**
* Case-sensitive paths.
*/
Expand Down
5 changes: 5 additions & 0 deletions src/index.bench.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const PATHS: string[] = [

const STATIC_PATH_MATCH = match("/user");
const SIMPLE_PATH_MATCH = match("/user/:id");
const SIMPLE_PATH_MATCH_WITH_PATTERN = match("/user/:id(\\d+)");
const MULTI_SEGMENT_MATCH = match("/:x/:y");
const MULTI_PATTERN_MATCH = match("/:x-:y");
const TRICKY_PATTERN_MATCH = match("/:foo|:bar|");
Expand All @@ -25,6 +26,10 @@ bench("simple path", () => {
for (const path of PATHS) SIMPLE_PATH_MATCH(path);
});

bench("simple path with parameter pattern", () => {
for (const path of PATHS) SIMPLE_PATH_MATCH_WITH_PATTERN(path);
});

bench("multi segment", () => {
for (const path of PATHS) MULTI_SEGMENT_MATCH(path);
});
Expand Down
8 changes: 8 additions & 0 deletions src/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ describe("path-to-regexp", () => {
),
);
});

it("should throw on unterminated parameter pattern", () => {
expect(() => parse("/:foo((bar")).toThrow(
new TypeError(
"Unterminated parameter pattern at 10: https://git.new/pathToRegexpError",
),
);
});
});

describe("compile errors", () => {
Expand Down
69 changes: 58 additions & 11 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ type TokenType =
interface LexToken {
type: TokenType;
index: number;
value: string;
value: string | { name: string; pattern?: string };
}

const SIMPLE_TOKENS: Record<string, TokenType> = {
Expand Down Expand Up @@ -119,7 +119,10 @@ function* lexer(str: string): Generator<LexToken, LexToken> {
const chars = [...str];
let i = 0;

function name() {
function name(options?: { pattern?: boolean }): {
name: string;
pattern?: string;
} {
let value = "";

if (ID_START.test(chars[++i])) {
Expand Down Expand Up @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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").

let depth = 1;
let pattern = "";
i++;
while (i < chars.length && depth > 0) {
if (chars[i] === "(") {
depth++;
} else if (chars[i] === ")") {
depth--;
}
if (depth > 0) {
pattern += chars[i++];
}
}
if (depth !== 0) {
throw new TypeError(
`Unterminated parameter pattern at ${i}: ${DEBUG_URL}`,
);
}
i++;
return { name: value, pattern };
}
return { name: value };
}

while (i < chars.length) {
Expand All @@ -165,10 +190,14 @@ function* lexer(str: string): Generator<LexToken, LexToken> {
} else if (value === "\\") {
yield { type: "ESCAPED", index: i++, value: chars[i++] };
} else if (value === ":") {
const value = name();
yield { type: "PARAM", index: i, value };
const value = name({ pattern: true });
yield {
type: "PARAM",
index: i,
value,
};
} else if (value === "*") {
const value = name();
const { name: value } = name();
yield { type: "WILDCARD", index: i, value };
} else {
yield { type: "CHAR", index: i, value: chars[i++] };
Expand All @@ -191,15 +220,23 @@ class Iter {
return this._peek;
}

tryConsume(type: TokenType): string | undefined {
tryConsume(type: Extract<TokenType, "PARAM">): {
name: string;
pattern?: string;
};
tryConsume(type: Exclude<TokenType, "PARAM">): string;
tryConsume(
type: TokenType,
): string | { name: string; pattern?: string } | undefined {
const token = this.peek();
if (token.type !== type) return;
this._peek = undefined; // Reset after consumed.
return token.value;
}

consume(type: TokenType): string {
const value = this.tryConsume(type);
consume(type: TokenType): string | { name: string; pattern?: string } {
const value =
type === "PARAM" ? this.tryConsume(type) : this.tryConsume(type);
if (value !== undefined) return value;
const { type: nextType, index } = this.peek();
throw new TypeError(
Expand Down Expand Up @@ -231,6 +268,7 @@ export interface Text {
export interface Parameter {
type: "param";
name: string;
pattern?: string;
}

/**
Expand Down Expand Up @@ -287,9 +325,11 @@ export function parse(str: string, options: ParseOptions = {}): TokenData {

const param = it.tryConsume("PARAM");
if (param) {
const { name, pattern } = param;
tokens.push({
type: "param",
name: param,
name,
pattern,
});
continue;
}
Expand Down Expand Up @@ -579,7 +619,14 @@ function toRegExp(tokens: Flattened[], delimiter: string, keys: Keys) {
}

if (token.type === "param") {
result += `(${negate(delimiter, isSafeSegmentParam ? "" : backtrack)}+)`;
if (token.pattern) {
result += `(${token.pattern})`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

} else {
result += `(${negate(
delimiter,
isSafeSegmentParam ? "" : backtrack,
)}+)`;
}
} else {
result += `([\\s\\S]+)`;
}
Expand Down