Skip to content
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

Allow values to be 'null' for types with a defined custom wrapper #1272

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alec-mccormick
Copy link

@alec-mccormick alec-mccormick commented Jul 31, 2019

This PR would give the ability to pass null for a property as long as a custom wrapper is defined for it. If a protobuf wrapper exists then the converter only checks for undefined rather than undefined or null.

This is a breaking change because it changes behavior for objects serialized with null values on fields that have an existing protobuf wrapper. This could potentially be addressed with an added configuration option to enable this behavior.

Use Case

We would like to make message fields 'nullable' so that the grpc client can overwrite columns in our database with null. The only way to currently accomplish this is make every message field a non-primitive type that we can check for null values. We would like to add protobuf wrappers to handle the serialization and deserialization of the fields but currently this is not possible because all null values are filtered out.

Message:

message MyString {
  bool null = 1;
  string value = 2;
}

message MyMessage {
  MyString foo = 1;
}

Protobuf Wrapper:

{
  '.MyString': {
    fromObject(value) {
      if (value === null) {
        return this.create({ null: true });
      }
      
      return this.create({ value });
    },
    toObject(message) {
      return message.null ? null : message.value;
    }
  }
};
// Currently treated the same as undefined even with the custom wrapper
// Server will receive the object {} rather than { foo: null }
myService.sendApiCall({ foo: null }); 

This use case is also dependent on Pull Request 1271 in order to be able to pass string | null as the property value and not fail the object check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant