-
Notifications
You must be signed in to change notification settings - Fork 89
🔧 fix: avoid error when using exclude.tags
with an API without tags
#274
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
WalkthroughThe change makes hooks.detail optional in toOpenAPISchema and updates tag-exclusion checks to use optional chaining. A new test verifies that routes tagged with excluded tags are omitted from the generated OpenAPI JSON while untagged routes remain. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant S as Server
participant OA as OpenAPI Plugin
participant R as Routes
C->>S: GET /openapi.json
S->>OA: Generate OpenAPI spec
loop For each route
OA->>R: Read route hooks
Note over OA,R: Check exclusion by tags
OA->>OA: if hooks.detail?.tags intersects exclude.tags -> skip
OA->>OA: else include operation in spec
end
OA-->>S: OpenAPI document
S-->>C: 200 OK + JSON
%% Styling (labels also describe changes)
rect rgba(220,240,255,0.5)
Note right of OA: Updated guard uses optional chaining on detail/tags
end
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)
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: 0
🧹 Nitpick comments (1)
test/index.test.ts (1)
276-308
: LGTM! Test validates the fix for issue #273.The test correctly verifies that:
- Routes with excluded tags are omitted from OpenAPI JSON
- Routes without tags remain in the output
- No TypeError occurs (which was the original bug)
Consider adding test coverage for additional edge cases:
it('should partially exclude routes with mixed tags', async () => { const app = new Elysia() .use(openapi({ exclude: { tags: ['internal'] } })) .get('/public', () => 'public', { detail: { tags: ['public', 'api'] } }) .get('/mixed', () => 'mixed', { detail: { tags: ['internal', 'public'] } }) await app.modules const res = await app.handle(req('/openapi/json')) const response = await res.json() // /public should be included (no internal tag) expect(response.paths['/public']).toBeDefined() // /mixed should be excluded (has internal tag) expect(response.paths['/mixed']).toBeUndefined() })This would verify behavior when a route has multiple tags, some of which match the exclusion list.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/openapi.ts
(2 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)
🔇 Additional comments (2)
src/openapi.ts (2)
323-323
: LGTM! Correctly makesdetail
optional.This change accurately reflects that routes may not have
detail
defined, preventing TypeErrors when accessing properties on undefined.
381-385
: LGTM! Proper optional chaining prevents TypeError.The optional chaining on
hooks.detail?.tags
correctly handles cases wheredetail
is undefined, which was the root cause of the bug reported in #273.
Summary by CodeRabbit
Bug Fixes
Tests