Transform recursively, without JSON.parse() #223
Conversation
| // 3. open nullable | ||
| if (value.nullable) { | ||
| output += "("; | ||
| } |
There was a problem hiding this comment.
cc @antonk52 sorry about doing this for you, but your PR pointed a flaw in the design. And I wanted to make sure I was solving for nullable (and it seems to be!).
So this doesn’t break, and it looks correct, but I actually didn’t add robust unit tests in this PR which we desperately need to ensure that nullable works in all cases. Are nullable tests (and potentially fixing some edge cases) something you’re interested in writing in a followup PR? 👍 / 👎 ?
There was a problem hiding this comment.
No worries man, whatever gets the job done =)
IMHO I think the existing cases covered the nullable fairly well. If you can point out a few extra test cases where the explicit test cases are needed I have no problem sending a pr later on
There was a problem hiding this comment.
Yeah, fair. Thanks for understanding.
Also, a new version is up on npm with the changes, so I’d love to see if the new @next version works for your schemas :)
| shipDate?: string | ||
| /** | ||
| * Order Status | ||
| */ |
There was a problem hiding this comment.
This PR adds comment support! 🎉
| | "individual" | ||
| | "non_profit"; | ||
| | ("company" | "government_entity" | "individual" | "non_profit") | ||
| | null; |
There was a problem hiding this comment.
I believe nullable has been added, but we can’t be sure before we write unit tests.
Ditches the
JSON.parse()approach which was limiting in 2 major ways:nullablesupport (Support Swagger v3 #112)At the end of the day, TypeScript isn’t JSON 🤷. And it turns out the original codebase was closer to what it needed to be. This PR refactors the output generation a little to be even simpler than before.