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

Migrate to protobuf-es from protobuf-ts #641

Open
jlewi opened this issue Jul 30, 2024 · 3 comments
Open

Migrate to protobuf-es from protobuf-ts #641

jlewi opened this issue Jul 30, 2024 · 3 comments

Comments

@jlewi
Copy link
Contributor

jlewi commented Jul 30, 2024

RunMe is using the protobuf-ts plugin.
Foyle is using the protobuf-es plugin.

Filing this issue so we can track it and eventually figure out what to do about it. Don't think we need to do anything right now.

Per the Migration guide it looks like protobuf-es is the newer thing that is officially maintained by buf. It looks like the main contributor to protobuf-ts is also contributing to protobuf-es.

@sourishkrout Did you have a reason you chose protobuf-ts over protobuf-es?

I introduced protobuf-es into Foyle to support Ghost cells jlewi/foyle#167. I went with protobuf-es because I wanted to use the connect protocol to handle bidirectional streaming for ghost cells rather than gRPC. I didn't want to use regular grpc because grpc doesn't work in the browser and I didn't want to introduce more code that would be a blocker to running the frontend as a PWA in the browser (#616).

The main downside to using both is we potentially have to convert between the two types (e.g. via serializing to JSON) which entails some overhead. Since this would only happen right now with Ghost Cells which is yet to be complete and would be an experimental feature I don't think we need to worry about it yet.

@sourishkrout
Copy link
Member

I don't remember a specific reason to not use protobuf-es other than it either wasn't available or mature enough at the time. Just from glancing over the project it does indeed look like protobuf-es is the natural successor of protobuf-ts. I believe Buf wound up hiring the original author of protobuf-ts so it would make sense to bring it under the buf umbrella.

I'd like to get through the runnerv2 migration first but after we could "schedule" a PoC and subsequent migration.

Re PWA, while it's not a short-term priority, I'd like to understand what it would take to run optionally Runme as a web. Both for rendering read-only saved Runme DevOps notebooks and potentially to use a "remote kernel". In this extension mode, using the connect protocol is likely of advantage: https://code.visualstudio.com/api/extension-guides/web-extensions

@sourishkrout
Copy link
Member

sourishkrout commented Jul 30, 2024

Per the Migration guide it looks like protobuf-es is the newer thing that is officially maintained by buf. It looks like the main contributor to protobuf-ts is also contributing to protobuf-es.

Just for reference. Here's a stable link https://github.com/bufbuild/protobuf-es/blob/v1.10.0/docs/migrating.md. The v2 release about ~1h ago rejiggered the docs and the old link will 404. I'm sure the migration docs got absorbed into the larger MANUAL.md.

@jlewi
Copy link
Contributor Author

jlewi commented Aug 1, 2024

A couple things to be aware of.
It looks protobuf-es just released version 2.0 which breaks backwards compatibility. (I think this is to support protobuf editions). This introduces a dependency on buf/protobuf 2.0 which is inconsistent with the 1.0 package which RunMe is currently using. (See bufbuild/protobuf-es#956). So we might want to plan on doing a single migration to 2.0.

I think the semantics of streaming in the connect SDK area also different. It uses AsyncIterable

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

2 participants