Skip object check for types with a defined custom wrapper.

[alec-mccormick]

This PR would give the ability to pass a primitive value for a non primitive type as long as a custom wrapper is defined for it.

Use Case

We would like to be able to define fields that can be multiple different primitive types, however the library currently will throw an error if a message field resolves to a type and the value is not an object. If a custom wrapper is defined for that type, then it should be assumed that the wrapper will handle serialization.

Message:

message StringOrDouble {
  oneof kind {
    string stringValue = 1;
    double doubleValue = 2;
  }  
}

message MyMessage {
  StringOrDouble value = 1;
}

Protobuf Wrapper:

{
  '.StringOrDouble': {
    fromObject(value) {
      return (typeof value === 'string') 
        ? this.create({ stringValue: value }) 
        : this.create({ doubleValue: value });
    },
    toObject(message) {
      return (typeof message.stringValue === 'string') 
        ? message.stringValue 
        : message.doubleValue;
    }
  }
}

[pgherveou]

I am not an owner, but tested this patch works perfectly for defining StringValue wrapper for examples
see #677 (comment)

[pgherveou]

@alexander-fenster, @JustinBeckwith, do you think you guys or another contributors could review and merge this?

[pgherveou]

@nicolasnoble any chance you could look at this PR? It is a quick fix and solves some interesting issues. see my comment here see #677 (comment)

[dardanos]

Please merge this 🙏

[betmix-matt]

@dcodeIO or @alexander-fenster any chance this could get some love?

It's a dependency of work arounds like:
#677

[Sandeep-Hirani]

@dcodeIO @alexander-fenster @bcoe Review please

[stephenh]

I know pings are annoying @dcodeIO / @alexander-fenster / @gitjuba but FWIW I'm the maintainer of ts-proto, and have pre-reviewed / :+1: this super-small patch.

We'd like to see this merged b/c it allows us to support modeling StringValue/etc. in TypeScript messages as just string | undefined.

Currently we already support this for our own encoder methods, but a number of our users use NestJS, which goes through grpcjs to @grpcjs/proto-loader, and proto-loader really wants to go through protobufjs's converter.js-generated encoders.

Usually we can use protobufjs's wrappers config to interject our own fromObject / toObject to map our "idiomatic TS"-isms back to what protobufjs supports.

Except for non-object primitives, due to this one extra-cautious typeof ... !== object check that this PR removes, only if the type has a wrapper configured, i.e. an indication that the user knows what they are doing, and hence the type ... !== object check imo should be safe to remove, and this PR safe to merge.

Lmk if you have questions or concerns. Thanks!

[gigi]

Hi guys! Is there any chance that this can be merged?

[Marko298]

We really need this change

[zamarawka]

Any way to get it merged?

[hyunjinjeong]

This simple PR that will help many people hasn't been merged or even reviewed for 5 years now 😢