Add support for custom options. about protobuf.js HOT 10 CLOSED

Thank you for your feedback!

Currently there are only a few options that are actually understood (packed and default) and all the other options are parsed but are not yet accessible within the emitted classes. I am also not sure if implementing custom options the google-way is worth the trouble.

Could you explain the exact scenario of what you are trying to accomplish?

from protobuf.js.

At work we have built an RPC framework on top of protocol buffers. Custom options are used to identify what protocols messages belong to, etc.

I'm hoping to use ProtoBuf.js to demonstrate that javascript is a viable choice for writing clients and servers for the framework.

from protobuf.js.

What types of custom options are you using? Would it be sufficient just to export each option's value on top of the emitted classes, e.g. if you had a "message Test" with some options, to be able to Test.getOpt(key) which will return a string with the options value?

from protobuf.js.

As of ProtoBuf.js 0.12.5, there is now an $options property exposed on every namespace object obtained from Builder#build(...). For example the "options.proto" test file will expose My.$options and My.Test.$options.

Such a property includes all options set for the namespace as key-value pairs (not type-resolved). However, in order to make this option non-enumerable in the namespace, it requires an ECMAScript 5 / JavaScript 1.8.5 compatible environment to use it (ref).

For compatibility, enclosing an option name in (...) just ignores the paranthesis.

Please let me know if this solves your issue.

from protobuf.js.

Thanks for the quick update, you actually committed your fix while I was writing a reply to your previous message.

Unfortunately the changes are not quite enough. While ignoring the parenthesis might make it possible to parse some of my protobuf files, it does not help parsing the file that declares the custom option:

$ node bin/proto2js custom-options.proto

/tmp/ProtoBuf.js/ProtoBuf.js:570
                        throw(new Error("Illegal top level declaration: "+toke
                              ^
Error: Illegal top level declaration: extend
    at ProtoBuf.DotProto.Parser.Parser.parse (/tmp/ProtoBuf.js/ProtoBuf.js:570:31)
    at Object.<anonymous> (/tmp/ProtoBuf.js/bin/proto2js:57:18)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Function.Module.runMain (module.js:497:10)
    at startup (node.js:119:16)
    at node.js:903:3

Ideally, ProtoBuf.js would support all the syntax and information of custom options, but that would probably require a non-insignificant amount of effort.

It just occurred to me that a possible alternative would be able to decode the file descriptor sets produced by protoc, and get the information out of them.

It doesn't quite work since there is something in descriptor.proto that throws ProtoBuf.js off:

$ node bin/proto2js ~/node_modules/protobuf/protobuf/src/google/protobuf/descriptor.proto

/tmp/ProtoBuf.js/ProtoBuf.js:714
                        throw(new Error("Illegal token in message "+msg.name+"
                              ^
Error: Illegal token in message FieldDescriptorProto: ;
    at ProtoBuf.DotProto.Parser.Parser._parseMessage (/tmp/ProtoBuf.js/ProtoBuf.js:714:31)
    at ProtoBuf.DotProto.Parser.Parser.parse (/tmp/ProtoBuf.js/ProtoBuf.js:559:30)
    at Object.<anonymous> (/tmp/ProtoBuf.js/bin/proto2js:57:18)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Function.Module.runMain (module.js:497:10)
    at startup (node.js:119:16)
    at node.js:903:3

Should I file an issue about that?

Thanks again!

from protobuf.js.

This issue is still open, so I'll look what I can do about it here :). Would ignoring the "extend" block be enough?

The problem with all the syntax is, that it will require lots of code that actually isn't useful in a real-world scenario. This might be ok for a C-library, but not in JavaScript-space where bandwidth is also an important factor.

from protobuf.js.

ProtoBuf.js 0.12.6 now simply ignores:

extend something {
    ...
}

And:

import "google/protobuf/something.proto";

As a result, your definitions should now compile fine. Of course this is not the same behaviour as in the official protoc implementation, but it should be fully compatible and not restrict you on the options level in any way.

The low-level bootstrapping stuff from protoc is simply not required by ProtoBuf.js as it uses its own magic :)

from protobuf.js.

Sadly, my .proto files still cause problems for ProtoBuf.js. I spent some time looking at custom options and they are quite tricky.

I suggest that you undo the changes you have done (except for those enabling block comments), and leave custom option support for later. I could attempt it, but I fear that it would take too long. If I do it anyway I will submit a pull request.

Thank you again for the time you have spent on this. You are very generous and it is much appreciated.

BTW, I'm not sure what's proper GitHub etiquette, should I close the slips, or should I leave that for your?

from protobuf.js.

Relax :). I've merged your test for customOptions and fixed all related issues. Try 0.12.7.

from protobuf.js.

At work we have built an RPC framework on top of protocol buffers. Custom options are used to identify what protocols messages belong to, etc.

I'm hoping to use ProtoBuf.js to demonstrate that javascript is a viable choice for writing clients and servers for the framework.

i have the same case, how you resolve ?

from protobuf.js.