-
Notifications
You must be signed in to change notification settings - Fork 349
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
feat: adds support for emitting default scalar values in json #919
Conversation
Hey @wgottschalk ! Thanks for the PR! Apologies for the delay reviewing, have just been busy... Fwiw I'm kind of dumb-founded that this so-obvious-in-retrospect slice point for "output default or not" already existed...it's been awhile since I was deep in this code, but 🤷 , that's why we have smart & motivated contributors like yourself I guess! 🎉 Looks like maybe there is a small options test failing, just b/c of the new option you added, I assume. Also do you mind running format/prettier on Otherwise it looks great, and I'll merge once the build is passing! |
Nice, good idea to follow that existing approach.
I think the encoders are probably fine to leave as-is, b/c they output a binary format that a) no one sees, and b) b/c it's binary most implementations pretty strictly follow it, vs. the JSON encoding where it's got this "someone people want defaults/some don't" nuance. By extensions, what are you referring to? Sorry, just not following/remembering. |
Just updated the snapshots and ran formatting on the directory. |
There's a function called generateExtension which has a call to notDefaultChecked Line 1519 in df0c596
Lines 1617 to 1624 in df0c596
|
Looks great @wgottschalk , thanks! |
# [1.158.0](v1.157.1...v1.158.0) (2023-09-24) ### Features * adds support for emitting default scalar values in json ([#919](#919)) ([01f529f](01f529f))
🎉 This PR is included in version 1.158.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Adds a new option for emitting default fields to support #907
Note that I only added support for the
toJSON
method. I designed the option to be easily modified to accept an array of fields similar tosnakeToCamel
. I didn't support the encoders or extensions because I'm not sure how to properly test those methods. If there's any code samples, I'd be happy to take a look and implement that