-
Notifications
You must be signed in to change notification settings - Fork 88
🔧 fix: exclude.paths
is not working when using RegExp
#276
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughReplaces exact-match exclusion for OpenAPI route filtering with a matcher that treats string entries as exact matches and non-string entries as regular expressions; routes are skipped when any matcher matches. Adds a test verifying mixed regex and string exclusions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Elysia App
participant OA as OpenAPI Plugin
participant Ex as exclude.paths
participant Gen as OpenAPI Generator
App->>OA: initialize plugin with options
OA->>Gen: request spec generation
loop for each route
Gen->>Ex: evaluate matchers (some over entries)
alt matcher is string and equals path OR matcher is RegExp and test(path) true
Gen-->>Gen: skip route (excluded)
Note right of Gen #ffe599: Excluded by string or RegExp
else
Gen-->>Gen: include route in spec
Note right of Gen #d9ead3: Included in output
end
end
Gen-->>App: return OpenAPI JSON
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/openapi.ts (1)
317-322
: Prefer.test()
for RegExp matching.The logic correctly enables regex-based path exclusion, but
.exec()
returns an array ornull
, relying on truthiness. Use.test()
for clarity, as it directly returns a boolean.Apply this diff:
excludePaths.some(match => { if (typeof match === 'string') { return match === route.path } - return match.exec(route.path); + return match.test(route.path); })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/openapi.ts
(1 hunks)test/index.test.ts
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
test/index.test.ts (1)
src/index.ts (1)
openapi
(40-181)
2e297d8
to
41602e1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/index.test.ts (1)
289-289
: Consider removing the trailing semicolon for consistency.The trailing semicolon on line 289 is slightly inconsistent with the predominant style in the file, where most method chains end without semicolons. While this is a minor stylistic issue, removing it would improve consistency.
Apply this diff if you prefer consistency:
- .get("/v2", () => "v2"); + .get("/v2", () => "v2")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/openapi.ts
(1 hunks)test/index.test.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/openapi.ts
🧰 Additional context used
🧬 Code graph analysis (1)
test/index.test.ts (1)
src/index.ts (1)
openapi
(40-181)
🔇 Additional comments (1)
test/index.test.ts (1)
276-297
: LGTM! Test correctly validates regex and string exclusion.The test properly verifies that:
- Regex patterns (e.g.,
/^\/v1/
) exclude matching routes like "/v1" and "/v1/foo"- Exact string matches (e.g.,
"/v2"
) exclude only that specific path- Non-excluded routes (e.g.,
"/"
) remain in the generated OpenAPI specThe previous issues flagged in earlier reviews (
.only()
modifier and.listen(3000)
call) have been correctly addressed.
41602e1
to
4544611
Compare
@coderabbitai review |
✅ Actions performedReview triggered.
|
exclude.paths
is not working when using RegExp #275Summary by CodeRabbit
New Features
Tests