mapbox / pbf

A low-level, lightweight protocol buffers implementation in JavaScript.
BSD 3-Clause "New" or "Revised" License
796 stars 106 forks source link

Package information is missing in the [imported] messages. #129

Open v1nce opened 3 years ago

v1nce commented 3 years ago

Hi,

First, thank you for this usefull tool. I run pbf file.proto --browser > file.proto.js on simple file and it worked flawlessly.

Then I go for something more complex with no success. I downloaded https://github.com/psobot/keynote-parser/protos and ran pbf KNArchives.proto --browser > KNArchives.proto.js

It throwed "Unexpected type". I looked at the faulty js (compile.js) and go for a dirty fix return prefix + '("'+field.type+'").read' + suffix; instead of throw new Error('Unexpected type: ' + field.type); in the compileFieldRead and compileFieldWrite.

Then it compiled again but the output (500k lines) was not what I expected. First I was happy because it means the tool supports "import" directive. I think it will not output 500k LOC for a 700 LOC input. So it looked like I'll only have to run the command on top level .proto file and not on every .proto. Great.

Then I have a look at the detail of output. It looked like it does not support the package xxx; directive. (see KNArchives.proto line 10 for such directive) This seems strange to me, because it supports the import directive. Supporting one without supporting the other sounds odd to me because package seems easier to implement than import (I may be wrong) The second things that puzzles me is the quickfix I had to write. Supporting "import" without supporting FieldType is odd.

So for those 2 reasons I think I should be doing something wrong.
**Could you clarify the following points ?

v1nce commented 3 years ago

I dug a little deeper so here are my temporary conclusions :

Yes there is support for PACKAGE. You fill find it in the json structure that is passed to the compile function

var proto = {
    "syntax": 2,
    "package": "TSK",
    "imports": ["TSPMessages.proto"],
    "enums": [...],
    "messages": [......],
    "options": {},
    "extends": []
}

But the support is incomplete Here we see that the TSK package imports definitions from TSPMessages.proto. So where are the messages from the TSPMessages ? There are in the proto.messages. How can you tell apart messages from package TSK and those from package TSP (the underlying package in TSPMessages) ? You can't. The package information is lost. It's nowhere in a message.

Here is the JSON of a message "messages": [{ "name": "TreeNode", "options": {}, "enums": [], "extends": [], "messages": [], "fields": [], "extensions": null }

See ? No Package information.