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

Automatically wrapping & unwrapping wrappers.proto types? #677

Open
RodH257 opened this issue Feb 12, 2017 · 18 comments
Open

Automatically wrapping & unwrapping wrappers.proto types? #677

RodH257 opened this issue Feb 12, 2017 · 18 comments

Comments

@RodH257
Copy link

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link

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
Copy link

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
Copy link

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
Copy link

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
Copy link

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

@pgherveou
Copy link

This PR #1271 seems to resolve the issue mentioned by @kurayama (#677 (comment)) and make it possible to write wrapper like this one

const valueTypeWrapper: Protobuf.IWrapper = {
  fromObject(object) {
    return this.fromObject({ value: object })
  },

  toObject(message, options) {
    const object = this.toObject(message, options)
    return object && object.value
  }
}

Protobuf.wrappers['.google.protobuf.DoubleValue'] = valueTypeWrapper
Protobuf.wrappers['.google.protobuf.StringValue'] = valueTypeWrapper
Protobuf.wrappers['.google.protobuf.Int64Value'] = valueTypeWrapper
Protobuf.wrappers['.google.protobuf.BoolValue'] = valueTypeWrapper
Protobuf.wrappers['.google.protobuf.UInt64Value'] = valueTypeWrapper
Protobuf.wrappers['.google.protobuf.UInt32Value'] = valueTypeWrapper
Protobuf.wrappers['.google.protobuf.FloatValue'] = valueTypeWrapper
Protobuf.wrappers['.google.protobuf.BytesValue'] = valueTypeWrapper
Protobuf.wrappers['.google.protobuf.Int32Value'] = valueTypeWrapper

Would be great if an owner could review & merge it

@TimBailey-pnk
Copy link

@dcodeIO Thoughts on this above? Would allow me to manually add the wrappers that are stalled in this #929 ?

@ntindall
Copy link

ntindall commented Oct 6, 2019

#893 #1258 is also relevant to this issue.

@GregHattJr
Copy link

This PR #1271 seems to resolve the issue mentioned by @kurayama (#677 (comment)) and make it possible to write wrapper like this one

const valueTypeWrapper: Protobuf.IWrapper = {
  fromObject(object) {
    return this.fromObject({ value: object })
  },

  toObject(message, options) {
    const object = this.toObject(message, options)
    return object && object.value
  }
}

Protobuf.wrappers['.google.protobuf.DoubleValue'] = valueTypeWrapper
Protobuf.wrappers['.google.protobuf.StringValue'] = valueTypeWrapper
Protobuf.wrappers['.google.protobuf.Int64Value'] = valueTypeWrapper
Protobuf.wrappers['.google.protobuf.BoolValue'] = valueTypeWrapper
Protobuf.wrappers['.google.protobuf.UInt64Value'] = valueTypeWrapper
Protobuf.wrappers['.google.protobuf.UInt32Value'] = valueTypeWrapper
Protobuf.wrappers['.google.protobuf.FloatValue'] = valueTypeWrapper
Protobuf.wrappers['.google.protobuf.BytesValue'] = valueTypeWrapper
Protobuf.wrappers['.google.protobuf.Int32Value'] = valueTypeWrapper

Would be great if an owner could review & merge it

I'm wondering where you are defining those wrappers / applying them to your PB generated JS files?

@Bakkenrak
Copy link

Any chance of getting PR #1271 or something similar merged? 🙏 This is generally also a use case for us.

@vikmn
Copy link

vikmn commented Jul 2, 2020

Is there any update on this? Are there any examples on how the wrapper module could be used for this purpose?

@yinzara
Copy link

yinzara commented Jan 11, 2021

Is there any way to get the typescript generated types from pbts to use different types for the generated types? While the above fix works great for pbjs, the TypeScript types for the fields now don't correspond to the real types.

@zamarawka
Copy link

faced with this issue inside of nest.js -> no way to fix it
so huge pain

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

No branches or pull requests