zig-protobuf icon indicating copy to clipboard operation
zig-protobuf copied to clipboard

Fix json oneof shadowing

Open jaronoff97 opened this issue 1 month ago • 2 comments

fixes #147

This is just a proposal and a first pass with a very mid implementation, mostly did this to unblock myself and see what others thought!

jaronoff97 avatar Dec 03 '25 21:12 jaronoff97

@Arwalk @menduz what's the best way to add in options to the package? I looked into adding them as params to the existing functions, but a lot of the current methods are meant to align with the std.json package which makes adding them challenging. How would you expect the options to work? I want to be sure i'm not introducing more complexity than is needed here and there are a few ways this could be accomplished.

jaronoff97 avatar Dec 05 '25 15:12 jaronoff97

yeah just tried to change the std.options to be a wrapped version with this new option and it was a decently thorny refactor. Open to suggestions on how to proceed! Given we were aligned that this may be a bug in implementation, another option would be to just remove the option entirely and set this as the default behavior, though that is a breaking change. Let me know what you all want to do :) thank you!

jaronoff97 avatar Dec 05 '25 16:12 jaronoff97

Hey there, thanks again @jaronoff97 for looking into this.

First, i'll invoke the robustness principle: decoding should be able to accept the input with or without shadowing. So there should be no need for new parameters on this part.

For encoding, there may be a way to pass a tuple of (StructToEncode, JsonPbOptions) in the source parameter, forwarding theses options along the way. This would require adding new parameters to the json encode/decode interfaces. This is just a very quick analysis, if i ever take to try to do it i'll keep you informed.

Does this gives you enough pointers to continue trying?

Thanks a lot for your time.

Arwalk avatar Dec 07 '25 18:12 Arwalk