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

fix: minify all common proto JSON files #1338

Merged
merged 1 commit into from
Sep 7, 2022
Merged

fix: minify all common proto JSON files #1338

merged 1 commit into from
Sep 7, 2022

Conversation

alexander-fenster
Copy link
Contributor

We ship several proto JSON files by copying them to the build/ folder before packing the package. These files are loaded into protobufjs and are not supposed to be read by a human (we ship .d.ts for humans). Since some libraries would benefit from having faster cold start time, and the size of the loaded files directly affects cold start time, let's minify these proto JSONs.

I'm exporting this small script because we can do the same in the libraries.

Size comparison:

Original files (in protos/ folder):

$ ls -lh protos/*.json
-rw-r--r--  1 fenster  primarygroup    94K Oct 19  2021 protos/compute_operations.json
-rw-r--r--  1 fenster  primarygroup    41K Jul 21  2021 protos/iam_service.json
-rw-r--r--  1 fenster  primarygroup    33K Aug  3  2021 protos/locations.json
-rw-r--r--  1 fenster  primarygroup    37K Jul 21  2021 protos/operations.json
-rw-r--r--  1 fenster  primarygroup   6.3K Aug  3  2021 protos/status.json

Minified files (in build/protos):

$ ls -lh build/protos/*.json
-rw-r--r--  1 fenster  primarygroup    32K Sep  4 01:52 build/protos/compute_operations.json
-rw-r--r--  1 fenster  primarygroup    15K Sep  4 01:52 build/protos/iam_service.json
-rw-r--r--  1 fenster  primarygroup    12K Sep  4 01:52 build/protos/locations.json
-rw-r--r--  1 fenster  primarygroup    14K Sep  4 01:52 build/protos/operations.json
-rw-r--r--  1 fenster  primarygroup   2.1K Sep  4 01:52 build/protos/status.json

@alexander-fenster alexander-fenster requested review from a team as code owners September 4, 2022 08:57
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Sep 4, 2022
@alexander-fenster alexander-fenster added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 4, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 4, 2022
Copy link

@ruyadorno ruyadorno left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Just curious on the choice of using a tools/minify.js file vs using the builtin uglify-js cli ?

@alexander-fenster
Copy link
Contributor Author

LGTM 👍

Just curious on the choice of using a tools/minify.js file vs using the builtin uglify-js cli ?

This script will eventually be called from all client libraries (because each library has its own proto JSON shipped, and I want them all to be minified). So if we had used uglify-js CLI, and later decide to change anything, it would've required changes across all libraries. I could've instead made tools/minify.ts call uglify-js CLI using a shell, but if I'm already writing a script, why not use the API :)

@alexander-fenster alexander-fenster merged commit 7569cc8 into main Sep 7, 2022
@alexander-fenster alexander-fenster deleted the uglify branch September 7, 2022 17:34
alexander-fenster added a commit that referenced this pull request Sep 7, 2022
gcf-merge-on-green bot pushed a commit that referenced this pull request Sep 7, 2022
🤖 I have created a release *beep* *boop*
---


## [3.5.0](v3.4.0...v3.5.0) (2022-09-07)


### Features

* MinifyProtoJson script exported as a bin ([#1340](#1340)) ([7956eb6](7956eb6))


### Bug Fixes

* Minify all common proto JSON files ([#1338](#1338)) ([7569cc8](7569cc8))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants