mapbox / mapbox/pbf

Package information is missing in the [imported] messages.

Open
#129 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

question
Dominant language
JavaScript
Stars
881
Forks
113
PR merge metrics
No merged PRs in 30d

Description

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 ?

- does pbf supports "FieldType references" other than the basic ones ? _(I don't know how to call it, maybe Message or Struct would be more exact than "FieldType references")_
- does it support package directive ?
- does it resolve import directive (whatever the nested level) ? So how does it deal with duplicate name functions (but in other packages)**

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with compile.js, especially compileFieldRead and compileFieldWrite, and inspect KNArchives.proto around its package and import directives. Run the pbf command shown in the issue against the referenced protos to reproduce the unexpected type and generated output. Done means the supported handling of message field types, packages, imports, and names across packages is clarified or implemented with appropriate tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
backend-api-design, tooling
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.