-
-
Notifications
You must be signed in to change notification settings - Fork 544
Fix wrong generated type when processing enums containing "null" #492
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
Codecov Report
@@ Coverage Diff @@
## main #492 +/- ##
==========================================
+ Coverage 78.52% 78.80% +0.28%
==========================================
Files 9 9
Lines 298 302 +4
Branches 92 95 +3
==========================================
+ Hits 234 238 +4
Misses 64 64
Continue to review full report at Codecov.
|
if (typeof item === "string") items.push(`'${item.replace(/'/g, "\\'")}'`); | ||
else if (typeof item === "number" || typeof item === "boolean") items.push(item); | ||
else if (item === null && !node.nullable) items.push("null"); | ||
}); |
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.
Very nice! This is much better handling than we were doing.
type: "object", | ||
}) | ||
).toBe(`{ | ||
"string"?: (('Totoro') | (2) | (false)) | null; |
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.
👍🏻
properties: | ||
myField: | ||
type: string | ||
enum: ["foo", 2, false, null] |
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.
Nice! Thanks for adding a snapshot test, too 💯
This looks great! Thank you 🙏🏻 |
@all-contributors please add @FedeBev for bug, code, test |
I've put up a pull request to add @FedeBev! 🎉 |
Hi there,
this is a WIP PR cointaining a failing test related to issue #482this PR contains a fix for the issue #482.
Please note: I tried to keep the enum generation logic as is, but in my opinion an enum declared as string should not be allowed to contain number or boolean.