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

Automatically wrapping & unwrapping wrappers.proto types? #677

Open
RodH257 opened this Issue Feb 12, 2017 · 10 comments

Comments

Projects
None yet
7 participants
@RodH257

RodH257 commented Feb 12, 2017

protobuf.js version: 6.6.3

Hey, first off this library is really great - love the typescript integration! Thanks for building it.

I'm trying to work out how to retain 'null' values across the wire. We have some instances where a value can be null and it means something different to empty string or 0 value. Google has the wrappers https://github.com/google/protobuf/blob/master/src/google/protobuf/wrappers.proto that let you do this, and protobuf.js handles retaining the null well, however it doesn't automatically wrap and unwrap the values

Lets say I have

message ChangeEvent {
  string source = 1;
  google.protobuf.StringValue code = 2;
}

I'd like to be able to pass in:

const changeEventWithCode = {
  source = 'test',
  code = 'code',
}

const changeEventWithoutCode = {
  source = 'test',
  code = null,
}

and have them both encode & decode to the same thing. However it seems if I want to set the code string, I have to do:

const changeEventWithCode = {
  source = 'test',
  code = {
    value: 'code',
  },
}

I was hoping fromObject and toObject would handle this, but they don't - is there any way I can hook in some customisation to do this, or would this be a good feature to add to the framework?

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Feb 12, 2017

There is no special handling for Wrappers, Any, Duration, Timestamp etc. currently.

What could be done, though, is to set up custom classes for these types that are capable of handling these cases. Haven't thought this through, yet, unfortunately.

@RodH257

This comment has been minimized.

RodH257 commented Feb 16, 2017

I was thinking along those lines as well, I was having a bit of difficulty working out how to get the custom classes injected in when using codegen, but I'll have another crack

@dcodeIO dcodeIO added the enhancement label Feb 23, 2017

dcodeIO added a commit that referenced this issue Apr 11, 2017

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Apr 11, 2017

This adds (hidden) options to specify custom implementations of fromObject and toObject. Basically, when there is an option __fromObject, it is used instead of the generated converter with this.fromObject referencing the original function. Likewise, a __toObject option does the same but for toObject.

There is a commented out example here: 0c6e639#diff-2c36c683dff5798fefed561dbec17803R54

Additional functionality can now be implemented right within src/common.js where these types are provided.

dcodeIO added a commit that referenced this issue Apr 11, 2017

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Apr 11, 2017

Has it's own module now.

Adding wrappers to the module translates to reflection. Static code integration is still todo, but possible.

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Apr 11, 2017

If anyone is interested in tackling other common types, feel free to send a PR! Otherwise, if there is a need for a specific type, but a PR is out of reach, feel free to leave a comment.

@alexleigh

This comment has been minimized.

alexleigh commented Mar 19, 2018

I'm having trouble writing wrappers to support google/protobuf/wrappers types. Suppose I have a message like the following:

message HasStringValue {
  google.protobuf.StringValue string_value = 1;
}

And I try to call HasStringValue.fromObject with an object like the following:

{
  stringValue: "hello world"
}

The desired result is that the string in this object gets turned into a StringValue message. However, this method results in the following error:

TypeError: .HasStringValue.stringValue: object expected

I tried to override this behavior both by providing a wrapper for .google.protobuf.StringValue and also by writing a custom constructor for .google.protobuf.StringValue. But neither my wrapper's fromObject nor my custom constructor gets invoked. It seems the error is thrown when building the outer HasStringValue message, and so the custom building logic for StringValue is never reached. Could I do something different to get protobuf wrappers to work?

@kurayama

This comment has been minimized.

kurayama commented May 29, 2018

This error comes from https://github.com/dcodeIO/protobuf.js/blob/master/src/converter.js#L36 which gets thrown before fromObject even has a chance to be called.

@dcodeIO Is there any way around this? Couldn't a preprocess step be called before this step? Or rename fromObject/toObject to serialize/deserialize and assume the argument could be anything.

@7chenko

This comment has been minimized.

7chenko commented Jun 26, 2018

Any word on a solution for well-known types support in static code generation? I'm using protobufjs to read/write JSON that is generated by C# Protobuf code, which produces canonical JSON for well-known types throughout. I am forced to write brittle hardcoded preprocessing before every fromObject and toJSON calls to wrap/unwrap the values.

@sthomp

This comment has been minimized.

sthomp commented Jun 28, 2018

Im also interested in static code generation for wrapper types. Currently, my solution is to manually overwrite to/fromObject. For example, if I have a proto message:

message MyMessage {
  google.protobuf.StringValue string_value = 1;
}

Then I can overwrite the statically generated to/fromObject functions:

const myMessageToObject = proto.MyMessage.toObject;
proto.MyMessage.toObject = (message: proto.MyMessage, options?: $protobuf.IConversionOptions): { [k: string]: any } => {
  const req = myMessageToObject(message);
  if (options && options.json && message.stringValue) {
    req.stringValue = message.stringValue.value;
  }
  return req;
};

const myMessageFromObject = proto.MyMessage.fromObject;
proto.MyMessage.fromObject = (object: { [k: string]: any }): proto.MyMessage => {
  if (object.stringValue && typeof object.stringValue === 'string') {
    object.stringValue = {
      value: String(object.stringValue),
    };
  }
  return myMessageFromObject(object);
};

This isn't ideal because I could have many message types, each of which I need to overwrite.

Would it make sense for this (or similar) logic to be baked into converter.js? As it stands (at least in Go), jsonpb special cases wrapper types which makes protobufjs incompatible with our server code.

@mickeyreiss

This comment has been minimized.

mickeyreiss commented Oct 6, 2018

+1 - it would be great to add a mode where WKTs are generated per the JSON proto3 spec.

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