-
-
Notifications
You must be signed in to change notification settings - Fork 773
fix(openapi): only filter undefined from array nodes
#3894
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
|
@kricsleo is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
π WalkthroughWalkthroughFixes a bug where falsy enum values (like 0 and empty strings) were excluded from OpenAPI definitions. Modifies the array filtering logic in the route metadata plugin to remove only undefined values instead of all falsy values, while updating test fixtures and snapshots to verify the fix. Changes
Estimated code review effortπ― 2 (Simple) | β±οΈ ~8 minutes
Pre-merge checks and finishing touchesβ Failed checks (1 warning)
β Passed checks (4 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/fixture/server/routes/api/meta/test.ts (1)
7-14: Good test coverage for the falsy value bug, consider expanding.The new parameter with
enum: [0, 1]effectively tests that the falsy value0is now correctly preserved in the OpenAPI output, directly addressing the reported issue.Consider adding test coverage for other falsy values like empty strings (
""),false, or edge cases to ensure comprehensive validation of the fix.π Example: Additional test parameter with empty string enum
{ in: "query", name: "status", schema: { type: "string", enum: ["", "active", "inactive"] }, }
π Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (3)
src/build/plugins/route-meta.tstest/fixture/server/routes/api/meta/test.tstest/presets/nitro-dev.test.ts
β° Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: tests-rollup (windows-latest)
- GitHub Check: tests-rollup (ubuntu-latest)
- GitHub Check: tests-rolldown (windows-latest)
- GitHub Check: tests-rolldown (ubuntu-latest)
π Additional comments (2)
test/presets/nitro-dev.test.ts (1)
48-58: LGTM! Snapshot correctly reflects the fix.The updated snapshot confirms that the falsy value
0now correctly appears in the OpenAPI enum, validating that the fix works end-to-end from route metadata definition to OpenAPI output.src/build/plugins/route-meta.ts (1)
106-108: The fix correctly preserves null values in arraysβthis is intentional.The change from
.filter(Boolean)to.filter((obj) => obj !== undefined)is the correct fix. According to commit e9d1b7c, this was specifically designed to "only filterundefinedfrom array nodes," preserving legitimate falsy values like0,"",false, andnullthat may appear in route metadata. This prevents legitimate data from being lost during OpenAPI metadata extraction.
commit: |
pi0
left a 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.
Thanks!
π Linked issue
resolves #3892
β Type of change
π Description
π Checklist