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

Proper generator #3

Open
seishun opened this issue Aug 8, 2020 · 7 comments
Open

Proper generator #3

seishun opened this issue Aug 8, 2020 · 7 comments

Comments

@seishun
Copy link

seishun commented Aug 8, 2020

Patching protoc output seems hacky and error-prone. How about providing a proper plugin?

The most straightforward way would be to modify js_generator.cc, but that requires building a binary. Not convenient. Alternatively, this Node.js port could be used as the basis for a Node.js/Deno plugin.

@keithamus
Copy link

Working on an implementation here: https://github.com/keithamus/deno-protod if you want to check it out.

@seishun
Copy link
Author

seishun commented Sep 4, 2020

@keithamus is this proto3-only?

@keithamus
Copy link

For now it is. The parser can consume proto2 files and output ASTs for Proto2, but I just don't have the motivation to get proto2 groups and extensions translated to typescript.

Right now it intentionally throws if you feed it proto2. PRs welcome to change that behaviour. If you want to get groups and extensions working too then great!

@marcushultman
Copy link
Owner

My opinion is that, since typescript is a superset of javascript, little work is needed as long as protoc can produce some javascript that is similar to the javascript that Deno can execute directly. I think it would be much better to work with https://github.com/protocolbuffers/protobuf directly, and especially implement the ES6 imports. While at it, I would advocate for a Compiler Option to force file extensions. That would make the protoc output be immediately consumable by Deno.

For type declarations, I haven't really made up my mind. Great editors and IDEs can still provide great suggestions and completion for JS.

@seishun
Copy link
Author

seishun commented Sep 4, 2020

I think it would be much better to work with https://github.com/protocolbuffers/protobuf directly, and especially implement the ES6 imports.

That would be ideal but they are reluctant to accept any significant third-party contributions.

@keithamus
Copy link

keithamus commented Sep 4, 2020

Personally for me, I'd rather work on a separate implementation because the output that https://github.com/protocolbuffers/protobuf creates is far from idiomatic JS, let alone it being idiomatic TypeScript. For example their code uses the google closure compiler module system. I don't see that changing any time soon.

There are direct wins for having first-class TS output; a good example is enums which have a very close representation between proto and TS, so we use the power of TS enums for much simpler code.

Ultimately I think it also benefits the protobuf community to have separate implementations. It was a good while before proto had a complete ENBF grammar and it shows in the complexity of proto2 files. Having competing parsers/output can provide space for innovation and provide good upstream feedback as to where wins can be made.

(Here's my parser for deno https://github.com/keithamus/deno-protoc-parser. I encourage alternative implementations to https://github.com/keithamus/deno-protod)

(Ps: hopefully this doesn't come across as self promotion, that isn't intended. The intent is to share some good discourse on the matter. Don't intend to waste people's time or self promote)

@marcushultman
Copy link
Owner

Don't get me wrong, there is definitely a place for idiomatic TypeScript. It's most likely not output from protoc anytime soon and even if it did, it wouldn't come from the JavaScript generator. This repo is not it though. I don't see this solution competing with plugins that does output more idiomatic TypeScript (competing => being an alternative if you will). This was just an attempt to get the basic functionality in place to work with protocol buffers shortly after Deno v1 landed as the existing solutions did not have enough feature coverage for me personally. 🙂

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

No branches or pull requests

3 participants