feat: compileProtos bin script#547
Conversation
Codecov Report
@@ Coverage Diff @@
## master #547 +/- ##
=========================================
- Coverage 89.88% 82.59% -7.3%
=========================================
Files 52 53 +1
Lines 3519 3544 +25
Branches 268 269 +1
=========================================
- Hits 3163 2927 -236
- Misses 290 550 +260
- Partials 66 67 +1
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #547 +/- ##
=========================================
Coverage ? 44.44%
=========================================
Files ? 10
Lines ? 693
Branches ? 4
=========================================
Hits ? 308
Misses ? 382
Partials ? 3Continue to review full report at Codecov.
|
bcoe
left a comment
There was a problem hiding this comment.
mostly nits 👍
excited to see this in action.
| if (fileStat.isFile() && file.match(PROTO_LIST_REGEX)) { | ||
| result.push(fullPath); | ||
| } else if (fileStat.isDirectory()) { | ||
| const nested = await findProtoJsonFiles(fullPath); |
There was a problem hiding this comment.
clever 😄 seems like a nice simple implementation that should work.
| '--target', | ||
| 'json', | ||
| '-p', | ||
| path.join('node_modules', 'google-gax', 'protos'), |
There was a problem hiding this comment.
does google-gax export this as a bin? it's better to execute ./node_modules/.bin/protos if possible; because this will work regardless of hoisting.
There was a problem hiding this comment.
This is a value for -p option of pbjs, it basically points to an output directory, it's not executing anything.
|
lgtm! Cool stuff Alex |
|
As discussed in the chat: we'll run this script during the generation step and will commit the resulting JSON file so that no new @JustinBeckwith - do you have any concerns, or can you put your formal ✅ on this? |
This is a step towards using static protos in client libraries (both browser and Node.js).
Background: we cannot switch to using
protocto compile JavaScript stubs in client libraries right now since it will be a big refactor, a major change, and we don't really know how deeply we depend onprotobufjs(e.g. some end users' code might actually use it).But what we can and will do is to stop loading
.protofiles in the runtime (which is (a) slow, (b) sometimes unpredictable, and (c) not webpack-able), and use theprotobufjs-supported JSON format which is:protobuf.loadSyncaccepts it, so does the@grpc/proto-loader, so no changes are needed)This script will be placed into a
binfolder and will be used by client libraries to compile protos to JSON.