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

JS generator missing jspb.generate_from_object option #96

Open
hochhaus opened this issue May 23, 2016 · 56 comments
Open

JS generator missing jspb.generate_from_object option #96

hochhaus opened this issue May 23, 2016 · 56 comments
Labels
enhancement New feature or request javascript json triaged Issue has been triaged

Comments

@hochhaus
Copy link

hochhaus commented May 23, 2016

The comment for jspb.Message.GENERATE_FROM_OBJECT states:

 *     NOTE: By default no protos actually have a fromObject method. You need to
 *     add the jspb.generate_from_object options to the proto definition to
 *     activate the feature.

However, no such option is currently implemented in the open sourced version. I see the code which generates output for this option (GenerateClassFromObject) but it is invoked 0 times.

~/protobuf$ grep -r GenerateClassFromObject .
./src/google/protobuf/compiler/js/js_generator.cc:void Generator::GenerateClassFromObject(const GeneratorOptions& options,
./src/google/protobuf/compiler/js/js_generator.h:  void GenerateClassFromObject(const GeneratorOptions& options,

Can the jspb.generate_from_object option be open sourced?

@hochhaus
Copy link
Author

Also, the jspb.generate_xid option appears to be missing as well.

/**
 * The xid of this proto type (The same for all instances of a proto). Provides
 * a way to identify a proto by stable obfuscated name.
 * @see {xid}.
 * Available if {@link jspb.generate_xid} is added as a Message option to
 * a protocol buffer.
 * @const {!xid.String|undefined} The xid or undefined if message is
 *     annotated to generate the xid.
 */
jspb.Message.prototype.messageXid;

@hochhaus
Copy link
Author

@xfxyjwf Any chance this bug could be addressed prior to the beta 4 release? It is a large issue for certain use cases of the JS implementation.

@pcj
Copy link
Contributor

pcj commented Aug 2, 2016

+1

@hochhaus
Copy link
Author

hochhaus commented Aug 2, 2016

Adding @haberman as he was able to help with a few other JS issues.

Any chance this could be looked into?

@haberman
Copy link
Member

haberman commented Aug 2, 2016

Many of the options we have internally are legacy/deprecated, so we haven't open-sourced them. The ones that we want to keep we will likely want to integrate into descriptor.proto.

Could you say more about how you want to use toObject() and fromObject()? I ask because this format is not standardized, and it is similar to (but not the same as) proto3 JSON.

Also how do you want to use xids?

@hochhaus
Copy link
Author

hochhaus commented Aug 2, 2016

Thanks Joshua. All I need is proto3 JSON support (I was not aware of the different toObject() JSON format). Also I have no current need for xid, I was only trying to be complete.

Could the proto3 JSON support be open sourced?

@haberman
Copy link
Member

haberman commented Aug 2, 2016

We don't currently have proto3 JSON support implemented for JavaScript (even internally). Inside Google most JavaScript projects use a wire format called JSPB that is specific to JavaScript and encodes protobufs as arrays indexed by field number like [123, "abc", true]. This format has code-size benefits over binary or proto3 JSON formats (basically it can be parsed with JSON.parse() without the need for any generated parsing code). But the JSPB format isn't very standardized (even inside Google) and changes a lot so we haven't open-sourced it.

For JavaScript we've implemented binary support which works across proto2/proto3 and can interoperate with other implementations. proto3 JSON support would be possible to implement, but we haven't done it yet.

@hochhaus
Copy link
Author

hochhaus commented Aug 2, 2016

@haberman Thanks for the additional context.

For what it is worth, the JSPB format that you mentioned looks very similar to the goog.proto2 PBLite format from closure-library. I've been using this for some time as part of the protoclosure project.

Given that proto3 JSON isn't implemented would it be possible to include the toObject and fromObject JSPB support?

@haberman
Copy link
Member

haberman commented Aug 3, 2016

For what it is worth, the JSPB format that you mentioned looks very similar to the goog.proto2 PBLite format from closure-library.

Indeed -- that is an example of a format that is similar but not the same as JSPB. I didn't realize that had been open-sourced.

@pcj
Copy link
Contributor

pcj commented Aug 3, 2016

I was under the impression from https://groups.google.com/d/msg/closure-library-discuss/oRZIfaoFGRQ/0FZpl-RIAgAJ that someone is working on grpc support for javascript that may or may not be supported by closure library goog.net.rpc.*. Any insight on JSON support with those efforts?

@haberman
Copy link
Member

haberman commented Aug 3, 2016

@stanley-cheung do you have any insight on @pcj's question?

@stanley-cheung
Copy link

I am not sure about the full context here, but yes we are working on a javascript client library that will use JS protobuf and the Closure library to support making RPC calls from browsers to gRPC backend via a gRPC gateway. It is still early in that project so nothing concrete is released yet. The current estimate is late Q3 the earliest, probably Q4.

@pcj
Copy link
Contributor

pcj commented Aug 3, 2016

@stanley-cheung Nice. Keep me posted, I may be interested in helping with that.

@seishun
Copy link

seishun commented Aug 11, 2016

@haberman

Could you say more about how you want to use toObject() and fromObject()?

Here's my use case. I maintain a library that interoperates with a network that uses protobufs (proto2) extensively. Since I don't want to tie my library's API to a specific protobuf implementation, its methods accept protobufs as plain objects - same goes for events that provide protobufs (more detailed description here).

There's also the issue that I don't convert to camelCase, but toObject in this implementation does and there's no way around it. I was hoping to address this after protocolbuffers/protobuf#1922 is resolved.

@alexeykomov
Copy link

alexeykomov commented Oct 21, 2016

Hello, all, I'm interested in this issue being resolved as well.

Answering @haberman on question in https://github.com/google/protobuf/issues/1591#issuecomment-237001956.
My use case is to use toObject and fromObject in order to use JSON.stringify and JSON.parse with them correspondingly, in order to transmit human readable JSON over the wire.

I see that at least fromObject being missing could be resolved by adding GenerateClassFromObject(options, printer, desc); after this line https://github.com/google/protobuf/blob/master/src/google/protobuf/compiler/js/js_generator.cc#L1841.

Is there any dangers with this solution?

@wiktortomczak
Copy link

wiktortomczak commented Mar 18, 2017

What's the status of this? Are there plans to include fromObject method in the generated JS proto message classes?

My use case is same as @alexeykomov's. Also, Alexey's solution just works. See wiktortomczak/protobuf@68ad1c8.

@someValue
Copy link

+1

@jonnyreeves
Copy link

I would also like to make use of fromObject; our use-case is to incorporate it with https://github.com/improbable-eng/ts-protoc-gen and provide a less verbose API for constructing Protobuf objects for transmission via https://github.com/improbable-eng/grpc-web

I'm happy to raise a PR for this if it will be considered by the grpc maintainers.

@shanshanzhu
Copy link

What is the status of this one? We would like to have fromObject exposed as well.

@anandolee
Copy link
Contributor

@haberman any updates?

@hartmut-co-uk
Copy link

having the same requirement as @jonnyreeves...

less verbose API for constructing Protobuf objects

yugui referenced this issue in yugui/grpc-custom-serializer Feb 19, 2018
Remaining issues:
* https://github.com/google/protobuf/issues/1591
  There's no way to unmarshal from JSON.
@metled
Copy link

metled commented Apr 19, 2018

+1, any updates?

@connor4312
Copy link

Lacking this option is is making it hard to support communication with etcd in the browser (see referenced issue) 🙁

@Teivaz
Copy link

Teivaz commented Oct 25, 2020

Any luck get this issue resolved? It's been 4 years since it was raised.

@otto-dev
Copy link

otto-dev commented Nov 15, 2020

My use case is same as @alexeykomov's. Also, Alexey's solution just works. See wiktortomczak/protobuf@68ad1c8.

I don't know what I'm looking at - So we could have this solved with one line of code?

@otto-dev
Copy link

otto-dev commented Nov 15, 2020

I've found a way to convert any plain JavaScript object reliably to its Protobuf counterpart for use with any grpc-web implementation. Includes nested messages / all types.

The answer is that protocolbuffers/protobuf does not support fromObject, but protobuf.js does with .encode.

So you can serialize your plain object into the protobuf binary format using protobuf.js, and then read that serialized binary back into another protobuf library of your choice. (improbable-eng/grpc-web in my case)

  const ProtoJsMyMessage = root.lookupType('something.MyMessage'); // protobuf.js
  const whatTheHeck = { ... }; // plain JS object containing the message properties
  const result = MyMessage.deserializeBinary(ProtoJsMyMessage.encode(whatTheHeck).finish()); // MyMessage is grpc-web

So I'm using the protobuf binary format as an interface between the two protobuf libraries, so that I can use gRPC as an interface to communicate with my backend. I'm glad that I have finally arrived at a solution that is more "agile" (/s) than just using JSON. Shoot me now.

@lucaslcode
Copy link

That should be looked at. Saying "toObject/fromObject has some issues and we are discussing it internally" really goes against the point of open source. What are the issues? toObject works well. We are ready to help with pull requests but they get ignored. Is the internal discussion 4 years long?

@Leo7654
Copy link

Leo7654 commented Nov 18, 2020

+1

@vjpai
Copy link

vjpai commented Nov 18, 2020

@murgatroid99 @nicolasnoble Can y'all comment on #96

@murgatroid99
Copy link

As far as I am aware, people have been successfully using code generated by protoc with Node gRPC just fine for a while. This requested feature would absolutely improve usability, but I assume the statement that it is "unusable" is just hyperbole.

@mkosieradzki
Copy link

As far as I am aware, people have been successfully using code generated by protoc with Node gRPC just fine for a while.

You mean people used to imperative paradigm (I bet they have backend implemented in Java :-) ). Today people practising functional paradigm have no practical means to use this library.

From my perspective lack of support for functional style is good enough reason to keep using protobuf.js.

@xiemeilong
Copy link

Add this feature, please , please, please!

@ykensuke
Copy link

+1

@kamok
Copy link

kamok commented Apr 23, 2021

Can someone explain how we would use the new fromObject method? I currently use https://github.com/agreatfool/grpc_tools_node_protoc_ts to generate TypeScript types from proto files. Does the PR from protocolbuffers/protobuf#8488 mean that the typescript generation lib need to be updated as well (simple typing change?).

@wapa5pow
Copy link

I created example repository to generate fromObject method https://github.com/wapa5pow/protoc-node . It compiles protoc with fromObject.

@sglim
Copy link

sglim commented Apr 27, 2022

I created an npm library to serialize pure javascript objects compatible with the protobuf class.
https://www.npmjs.com/package/protobuf-js-from-object

Modifying protoc is too heavy and sometimes hard to apply due to your dependency system.
So I add a simple solution to serialize a pure object to protobuf binary.

The serialized binary can be deserialized, and you can also send it via networks.
The solution is a bit hacky, but it doesn't take many resources like memory or CPU.

For example:

// Serialize
const buffer = serializeFromObject(pureObj, pb.Burger);
// Deserialize
const burger = pb.Burger.deserializeBinary(buffer);

Make sure that the pure object should have all properties as a key even if the property's value is undefined.

psigen referenced this issue in psigen/protobuf-javascript May 4, 2022
Adds the fromObject helper to the public API of generated pb classes.

This closes a 5-year-old open issue: https://github.com/protocolbuffers/protobuf/issues/1591 
by rebasing this 1-year-old open PR: protocolbuffers/protobuf#8488
based on this 4-year-old workaround: https://github.com/protocolbuffers/protobuf/issues/1591#issuecomment-414778829
@acozzette acozzette transferred this issue from protocolbuffers/protobuf May 16, 2022
@dibenede dibenede added the triaged Issue has been triaged label Sep 30, 2022
@kcarlson
Copy link

@leafstark
Copy link

https://www.npmjs.com/package/proto-msg-converter
This can be helpful

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request javascript json triaged Issue has been triaged
Projects
None yet