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.

Sign up for GitHub

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

Jump to bottom

fix: minify proto JSON files #1771

Merged
merged 2 commits into from Sep 13, 2022
Merged

fix: minify proto JSON files #1771

merged 2 commits into from Sep 13, 2022

Conversation

alexander-fenster
Copy link
Member

Use the minifyProtoJson script from google-gax to minify proto JSON files. No one reads those auto-generated files anyway.

@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. api: firestore Issues related to the googleapis/nodejs-firestore API. labels Sep 12, 2022
@alexander-fenster alexander-fenster added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Sep 12, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Sep 12, 2022
@alexander-fenster alexander-fenster removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 12, 2022
@alexander-fenster
Copy link
Member Author

Before:

$ ls -lh dev/protos/*.json
-rw-r--r--  1 fenster  primarygroup    76K Sep 12 13:58 dev/protos/admin_v1.json
-rw-r--r--  1 fenster  primarygroup    89K Sep 12 13:58 dev/protos/v1.json
-rw-r--r--  1 fenster  primarygroup    79K Sep 12 13:58 dev/protos/v1_admin.json
-rw-r--r--  1 fenster  primarygroup    88K Sep 12 13:58 dev/protos/v1beta1.json

After:

$ ls -lh dev/protos/*.json
-rw-r--r--  1 fenster  primarygroup    28K Sep 12 13:59 dev/protos/admin_v1.json
-rw-r--r--  1 fenster  primarygroup    32K Sep 12 13:59 dev/protos/v1.json
-rw-r--r--  1 fenster  primarygroup    29K Sep 12 13:59 dev/protos/v1_admin.json
-rw-r--r--  1 fenster  primarygroup    31K Sep 12 13:59 dev/protos/v1beta1.json

MarkDuckworth
MarkDuckworth approved these changes Sep 12, 2022
View reviewed changes
Copy link
Contributor

@MarkDuckworth MarkDuckworth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM? Will this affect debuggability?

@product-auto-label product-auto-label bot added size: xs Pull request size is extra small. and removed size: xl Pull request size is extra large. labels Sep 12, 2022
@alexander-fenster
Copy link
Member Author

Oh, actually, I just realized I can do the same as I did in google-gax here. Let's minify the proto JSONs only in the build folder, so that they are minified in the distribution (for npm pack), but are readable in the repository. Updated the PR. Thanks for reviewing!

@alexander-fenster
Copy link
Member Author

Verifying that it actually works as intended:

$ npm run compile

> @google-cloud/firestore@6.1.0 precompile
> gts clean

version: 16
Removing build ...

> @google-cloud/firestore@6.1.0 compile
> tsc -p .


> @google-cloud/firestore@6.1.0 postcompile
> node scripts/init-directories.js && cp -r dev/protos build && cp dev/src/v1beta1/*.json build/src/v1beta1/ && cp dev/src/v1/*.json build/src/v1/ && cp dev/conformance/test-definition.proto build/conformance && cp dev/conformance/conformance-tests/*.json build/conformance/conformance-tests && minifyProtoJson

Minifying admin_v1.json...
Minifying v1.json...
Minifying v1_admin.json...
Minifying v1beta1.json...
Minified all proto JSON files successfully.

$ npm pack
npm notice 
npm notice 📦  @google-cloud/firestore@6.1.0
npm notice === Tarball Contents === 
npm notice 11.4kB  LICENSE                                                     
npm notice 8.2kB   README.md                                              
...
npm notice 29.1kB  build/protos/admin_v1.json
...
npm notice 29.8kB  build/protos/v1_admin.json                                  
npm notice 32.3kB  build/protos/v1.json                                        
npm notice 32.0kB  build/protos/v1beta1.json                                   
...
npm notice === Tarball Details === 
npm notice name:          @google-cloud/firestore                 
npm notice version:       6.1.0                                   
npm notice filename:      @google-cloud/firestore-6.1.0.tgz       
npm notice package size:  623.0 kB                                
npm notice unpacked size: 5.9 MB                                  
npm notice shasum:        be5309cb9e198a7c7465831d6dd54c32e2829cf7
npm notice integrity:     sha512-hXXmXPJtDlcy+[...]mmsRBKFFAF8Ng==
npm notice total files:   159                                     
npm notice 
google-cloud-firestore-6.1.0.tgz

Note the size of npm pack-ed package in the main branch:

npm notice package size:  630.5 kB                                
npm notice unpacked size: 6.1 MB

@alexander-fenster alexander-fenster merged commit 6393fe7 into main Sep 13, 2022
@release-please release-please bot mentioned this pull request Sep 13, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MarkDuckworth MarkDuckworth MarkDuckworth approved these changes

Assignees
No one assigned
Labels
api: firestore Issues related to the googleapis/nodejs-firestore API. size: xs Pull request size is extra small.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@alexander-fenster @MarkDuckworth