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

Static generation vs loading protos at runtime #401

Closed
JustinBeckwith opened this issue Jan 28, 2019 · 8 comments
Closed

Static generation vs loading protos at runtime #401

JustinBeckwith opened this issue Jan 28, 2019 · 8 comments
Labels
status: investigating The issue is under investigation, which is determined to be non-trivial. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@JustinBeckwith
Copy link
Contributor

So, I'm in no way an expert on gRPC. I've noticed that we seem to be shipping *.proto files in our libraries, and then parsing/loading them at runtime. I also notice that we tend to do a lot of this synchronously, and during construction.

What do folks think of us... not doing that? Just guessing, but if we use pbjs to generate the stubs at compile time, wouldn't that save us a fair bit of runtime loading perf?

@ofrobots @alexander-fenster @crwilcox @kinwa91

@JustinBeckwith JustinBeckwith added the type: question Request for information or clarification. Not an issue. label Jan 28, 2019
@jkwlui
Copy link
Member

jkwlui commented Jan 28, 2019

Is this in the scope of the upcoming microgenerator work? I feel like we would be able to take advantage of generated typescript protobuf code..

@jkwlui
Copy link
Member

jkwlui commented Jan 28, 2019

@JustinBeckwith JustinBeckwith added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed type: question Request for information or clarification. Not an issue. labels Feb 10, 2019
@JustinBeckwith JustinBeckwith changed the title [Discussion] Static generation vs loading protos at runtime Static generation vs loading protos at runtime Feb 10, 2019
@JustinBeckwith JustinBeckwith added priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. status: investigating The issue is under investigation, which is determined to be non-trivial. labels Feb 10, 2019
@evaogbe
Copy link
Contributor

evaogbe commented Feb 11, 2019

It looks like protobufjs doesn't support gRPC: grpc/grpc-node#528

@evaogbe
Copy link
Contributor

evaogbe commented Feb 14, 2019

To be clear, it doesn't support gRPC directly. @grpc/protoloader uses protobufjs internal.

@yoshi-automation yoshi-automation added 🚨 This issue needs some love. and removed 🚨 This issue needs some love. labels Feb 19, 2019
@noahdietz
Copy link

@eoogbe any update on this issue? Thanks!

@JustinBeckwith
Copy link
Contributor Author

I'd like to keep this open for discussion. It looks like this is something we should engage with the gRPC team.

@JustinBeckwith JustinBeckwith removed the priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. label Feb 25, 2019
@noahdietz
Copy link

@eoogbe Is this something you can take to the gRPC team for exploration or should we assign it to another for follow up?

@evaogbe evaogbe removed their assignment Sep 30, 2019
@alexander-fenster
Copy link
Contributor

We don't load protos directly anymore, instead, we use pbjs to produce a JSON and load it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: investigating The issue is under investigation, which is determined to be non-trivial. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

6 participants