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

When defaults=false, arrays are initialized as null #597

Closed
r-tock opened this Issue Dec 29, 2016 · 11 comments

Comments

Projects
None yet
2 participants
@r-tock
Contributor

r-tock commented Dec 29, 2016

V5 behavior has been that arrays are always initialized. Can we get the same behavior back via an JSONConversionOption. We don't want non-arrays to be initialized to defaults, but there are several places where arrays need to be initialized to []

#MigrationPains

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Dec 30, 2016

Not exactly sure that there should be an option to handle just array defaults while excluding everything else. I feel like this is too much of a special case.

@dcodeIO dcodeIO added the unsure label Dec 30, 2016

@r-tock

This comment has been minimized.

Contributor

r-tock commented Dec 30, 2016

I understand the feel of special case. But because of v5 there are several places where this is done and even at our scale of code size it's a nightmare.

For arrays in every language other than protobufjs arrays are never null. Which makes the null representation here stick out as a exception. And this is independent of proto2/3

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Dec 30, 2016

So, the array field values are explicitly set to null instead of being just undefined? That would indeed be a bug.

@r-tock

This comment has been minimized.

Contributor

r-tock commented Dec 30, 2016

I mean set to null instead of default initialized []

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Dec 30, 2016

Can you provide additional information on your use case or an example? I am somewhat puzzled.

@r-tock

This comment has been minimized.

Contributor

r-tock commented Dec 30, 2016

Let's say a parsed proto object looks like

Message MyProto {
 optional int32 some_int = 1;
 repeated int32 some_array = 2;
 repeated int32 other_array = 3;
}

Let's say it parsed a datastream and looked like this.
{someArray:[0, 1] }
obj.otherArray will be []

But on converting to JSON
We get {someArray:[0, 1] }
However when I enable defaults, I get {someInt :0, someArray:[0,1], otherArray: [] } this is fine as this is a proto3 behavior
But former is not. When converting to JSON without defaults prroto2 usually gives you default initialized array and you expect { someArray:[0,1], otherArray: [] }

dcodeIO added a commit that referenced this issue Dec 30, 2016

@r-tock

This comment has been minimized.

Contributor

r-tock commented Dec 30, 2016

👍

@r-tock r-tock closed this Dec 30, 2016

@r-tock r-tock reopened this Dec 30, 2016

@r-tock

This comment has been minimized.

Contributor

r-tock commented Dec 30, 2016

Reopening to address 40074bb#comments

dcodeIO added a commit that referenced this issue Dec 30, 2016

@r-tock

This comment has been minimized.

Contributor

r-tock commented Jan 3, 2017

@dcodeIO Hope you get a chance to address this again, last remaining blocker for us.

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Jan 3, 2017

Have you tried again with master, lately?

dcodeIO added a commit that referenced this issue Jan 3, 2017

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Jan 3, 2017

Closing this for now as 6.4.0, which is now released, should solve the issue. Feel free to reopen the issue if necessary.

@dcodeIO dcodeIO closed this Jan 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment