feat: add option noDefaultsForOptionals
#1051
Conversation
robmonie
changed the title
noDefaultsForOptionalsnoDefaultsForOptionals
src/main.ts
Outdated
| @@ -1249,10 +1248,14 @@ function generateDecode(ctx: Context, fullName: string, messageDesc: DescriptorP | |||
| ${messageProperty} = ${generateMapType ? "new Map()" : "{}"}; | |||
| }` | |||
| : ""; | |||
|
|
|||
| const ifValueCheck = `${varName}.value !== undefined ${withAndMaybeCheckIsNotNull(options, `${varName}.value`)}`; | |||
| const maybeIfKeyCheck = `${options.noDefaultsForOptionals ? ` && ${varName}.key !== undefined ${withAndMaybeCheckIsNotNull(options, `${varName}.key`)}`: ''}` | |||
There was a problem hiding this comment.
This change to map exists solely to satisfy the types. Now that both key and value are potentially undefined, assignment against an undefined key isn't valid so a check against both value and key is required. Is there possibly a better way to think about this?
integration/extension-import/base.ts
Outdated
| @@ -0,0 +1,82 @@ | |||
| // Code generated by protoc-gen-ts_proto. DO NOT EDIT. | |||
There was a problem hiding this comment.
This and other related files in this folder we generated from the existing base.proto as part of performing a build for tests.
|
Hi @robmonie ! Sorry about that |
|
I looked into this a bit more, and huh, I had not heard of Wire/didn't know about their "primitives are always boxed" behavior. That is certainly different! Although ngl I think I prefer it; protobuf's optional behavior has been such a bug bear over the years. Normally I'd want to keep this out of ts-proto, but the change is so relatively tiny, to our codebase, that 🤷 I guess it seems fine to me. @robmonie just curious, but does Square actually have public-facing APIs that use the Wire protocol/behavior, or have you just seen/used Wire's behavior, and prefer it, and so would like to use it on your own/non-Square projects? Just curious! |
|
Thanks for looking at this @stephenh. I'm not sure whether there are any externally facing apis that use Wire, however pretty much all the internal Apis, Events etc are based on protos written in Kotlin which use Wire as an internal standard. This makes interfacing with unsupported languages difficult. The biggest issue comes from the default proto behaviour of not encoding default values such as I'll do a rebase shortly and if there are any improvements you think I can make, let me know. |
robmonie
force-pushed
the
no-defaults-for-optionals
branch
3 times, most recently
from
bfaf8db
to
0e920f1
Compare
robmonie
force-pushed
the
no-defaults-for-optionals
branch
from
0e920f1
to
815d91f
Compare
|
Looks great @robmonie , thanks! |
# [1.177.0](v1.176.3...v1.177.0) (2024-06-07) ### Features * add option `noDefaultsForOptionals` ([#1051](#1051)) ([41d1020](41d1020))
|
🎉 This PR is included in version 1.177.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
The PR added new tests, and we need to re-generate the proto files.
Adds a new option -
noDefaultsForOptionalsthat allows for compatibility with Square/Block's Wire Protobuf implementation with respect to defaulting of optional fields.Note: As per the documentation added to the readme, this option provides non-standard protobuf behaviour so it would be understandable if this isn't something you'd like in the library. I'm hoping with the appropriate warning in the docs that you'll consider this as as option.