-
-
Notifications
You must be signed in to change notification settings - Fork 756
fix(router): support /files/:name{.*}
#4329
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
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I fixed the TrieRouter, but RegExpRouter still throws the error:
Hey @usualoma , can you take a look at it? I think the expected behavior in the test is correct. |
Hi @yusukebe, thank you for addressing the issue.
diff --git i/src/router/reg-exp-router/node.ts w/src/router/reg-exp-router/node.ts
index 91abea7f..ce521420 100644
--- i/src/router/reg-exp-router/node.ts
+++ w/src/router/reg-exp-router/node.ts
@@ -81,6 +81,9 @@ export class Node {
const name = pattern[1]
let regexpStr = pattern[2] || LABEL_REG_EXP_STR
if (name && pattern[2]) {
+ if (regexpStr === '.*') {
+ throw PATH_ERROR
+ }
regexpStr = regexpStr.replace(/^\((?!\?:)(?=[^)]+\)$)/, '(?:') // (a|b) => (?:a|b)
if (/\((?!\?:)/.test(regexpStr)) {
// prefix(?:a|b) is allowed, but prefix(a|b) is not
diff --git i/src/router/reg-exp-router/router.test.ts w/src/router/reg-exp-router/router.test.ts
index 305cee27..736c2ca9 100644
--- i/src/router/reg-exp-router/router.test.ts
+++ w/src/router/reg-exp-router/router.test.ts
@@ -14,6 +14,7 @@ describe('RegExpRouter', () => {
'Capture Group > Complex capturing group > GET request',
'Capture complex multiple directories > GET /part1/middle-b/latest',
'Capture complex multiple directories > GET /part1/middle-b/end-c/latest',
+ 'Complex > Parameter with {.*} regexp',
],
},
{ |
Co-authored-by: Taku Amano <[email protected]>
Bundle size check
Compiler Diagnostics (tsc)
Compiler Diagnostics (typescript-go)
Reported by octocov |
HTTP Performance Benchmark
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4329 +/- ##
==========================================
+ Coverage 91.60% 91.63% +0.03%
==========================================
Files 170 170
Lines 10881 10798 -83
Branches 3211 3129 -82
==========================================
- Hits 9967 9895 -72
+ Misses 913 902 -11
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Yeah. It's okay. Thank you for the patch. |
I investigated to fix #4325, and I found bugs in the routers.
If the registered path is
/files/:name{.*}
, the behaviors of some routers are incorrect.Fixes #4325
The author should do the following, if applicable
bun run format:fix && bun run lint:fix
to format the code