-
-
Notifications
You must be signed in to change notification settings - Fork 533
Fix oneOf
number const
#1056
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
Fix oneOf
number const
#1056
Conversation
@@ -98,7 +98,7 @@ export function defaultSchemaObjectTransform( | |||
// oneOf (no discriminator) | |||
if ("oneOf" in schemaObject && !schemaObject.oneOf.some((t) => "$ref" in t && ctx.discriminators[t.$ref])) { | |||
const maybeTypes = schemaObject.oneOf.map((item) => transformSchemaObject(item, { path, ctx })); | |||
if (maybeTypes.some((t) => t.includes("{"))) return tsOneOf(...maybeTypes); // OneOf<> helper needed if any objects present ("{") | |||
if (maybeTypes.some((t) => typeof t === "string" && t.includes("{"))) return tsOneOf(...maybeTypes); // OneOf<> helper needed if any objects present ("{") |
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.
👍
@@ -62,7 +62,7 @@ export function defaultSchemaObjectTransform( | |||
} | |||
|
|||
// const (valid for any type) | |||
if (schemaObject.const) { | |||
if (schemaObject.const !== null && schemaObject.const !== undefined) { |
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.
👍
@@ -0,0 +1,21 @@ | |||
import { tsUnionOf } from "../src/utils"; | |||
|
|||
describe("utils", () => { |
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.
These tests aren’t really necessary as they are all tested by proxy with other tests, and these are such simple functions with such simple inputs/outputs they have clear and obvious failures.
However, the coverage is appreciated and it doesn’t hurt to have. Thank you.
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.
Great fix; thank you!
@all-contributors please add @qnp for code, bug, test |
I've put up a pull request to add @qnp! 🎉 |
Changes
Fixing #1051 alongside the fact that
0
as a const value was evicted.How to Review
I wrote tests, they all pass now.
FYI, I wanted to make sure everything is working as expected, so I added tests for the
tsOneOf
util. IMHO, all the utils should be tested (but I did not have time to write these tests).Checklist
examples/
directory updated (if applicable)