Fix unintended serialization of the SelectedIndex for Input nodes #12087
Conversation
saintentropy
changed the title
| @@ -92,7 +92,7 @@ public class NodeInputData | |||
| /// The index of the selected item. | |||
| /// </summary> | |||
| [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] | |||
| public int SelectedIndex { get; set; } | |||
| public int? SelectedIndex { get; set; } | |||
There was a problem hiding this comment.
it's likely this is an API break.
I don't know if we care - but I think it fits in:
❌ DISALLOWED: Changing the type of a member
The return value of a method or the type of a property or field cannot be modified. For example, the signature of a method that returns an Object cannot be changed to return a String, or vice versa.
this may cause binary compatibility issues with any code that referenced this member, can you check?
There was a problem hiding this comment.
I think you are right. That was my reading of it as well. I do think we should probably make the change though. I will double check.
There was a problem hiding this comment.
@mjkkirschner Yep it does fail if you directly read or set the SelectedIndex but I think we should fix this.
There was a problem hiding this comment.
Hey - finally time to do this?
There was a problem hiding this comment.
Yep let's do this. Just updated the branch @mjkkirschner @QilongTang
Purpose
This sets the SelectedIndex to nullable so that it does not serialize with every input type. This key / value should only be serialized when the input type is a DropDown.
Declarations
Check these if you believe they are true
*.resxfilesReviewers
@mjkkirschner @nate-peters
FYIs
@BogdanZavu