-
Notifications
You must be signed in to change notification settings - Fork 13
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
feature: Protos #237
feature: Protos #237
Conversation
managed: | ||
enabled: true | ||
plugins: | ||
- plugin: buf.build/community/stephenh-ts-proto |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason to use this particular plugin? It generates a lot of duplicated data which will blow up our bundle size. From the another hand default plugin only generates schemas + types which can be used together with fromBinary
method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with David on this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@totraev @jrwbabylonlab
We can configure our protobuf files generation in multiple ways. The reason I chose this approach is
- it's the easiest, generates everything
- it was already established and working on
btc-staking-dashboard
application
It leads to https://i.imgur.com/lK7Xr86.png, generates all the types for the dependencies, etc.
We can play around with config on https://github.com/babylonlabs-io/babylon-proto-ts
@totraev I am not sure whether or not usage of .fromBinary
will be handy or that the current approach bloats the bundle size. But you can create an issue on that repo describing the pros of the suggested approach in comparison with the current that is already set up
cp -r "$BABYLON_PROTO" "$PROTO_DIR" | ||
echo "Compiling Babylon proto files to TS files" | ||
cd "$SCRIPTS_DIR" && npx buf generate proto | ||
cd ../.. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: i think this can be simplified to npx buf generate "$PROTO_DIR"
?
|
||
# Clean up | ||
rm -r "$PROTO_DIR" | ||
rm -rf "$BABYLON_REPO_DIR" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be nice t perform this clean up even if any of the above command were failed/existed.
try with trap
?
trap 'rm -rf "$PROTO_DIR" "$BABYLON_REPO_DIR"' EXIT
Minor update - this PR might be not needed as we move to the npm library and a separate repository |
Closing this in favor of |
src/proto
babylon
at the top level and runnpm run build:proto