Pack packable repeated fields and read packed (#167)

tatethurston · Apr 29, 2022 · ddaa883 · ddaa883
1 parent 6eba6d6
commit ddaa883
Show file tree

Hide file tree

Showing 12 changed files with 3,495 additions and 1,484 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Changelog
 
+## v0.0.55
+
+- Protobuf messages now always pack packable repeated fields when serializing and can read packed or unpacked when deserializing. This will slightly decrease the size of some protobuf messages over the wire, and enable better interop with messages encoded by other protobuf serializers.
+
 ## v0.0.54
 
 - Fixes a regression introduced in v0.0.53 that caused JSON.parse to be invoked twice for JSON clients.

diff --git a/package.json b/package.json
@@ -22,7 +22,7 @@
     "lint:package:fix": "prettier-package-json --write ./package.json",
     "package:build": "yarn install && yarn clean && yarn build:commonjs && yarn build:module && chmod +x dist/compiler.js dist/cli/index.js dist/test-protos/gen-test-input.js && cp src/compiler.cmd dist/compiler.cmd && yarn package:prune && yarn package:copy:files",
     "package:copy:files": "cp ./LICENSE ./README.md dist/ && cp ./public.package.json dist/package.json",
-    "package:prune": "find dist -name test.* | xargs rm -f",
+    "package:prune": "find dist -name test.* -delete",
     "regen": "yarn test-serialization && yarn test -u && yarn examples:check && yarn examples:buf && yarn clientcompat",
     "test": "yarn jest src/*",
     "test-serialization": "(cd src/test-serialization && yarn twirpscript)",
@@ -31,27 +31,27 @@
   },
   "sideEffects": false,
   "dependencies": {
-    "@types/google-protobuf": "^3.15.3",
+    "@types/google-protobuf": "^3.15.6",
     "google-protobuf": "^3.20.1",
     "prettier": "^2.6.0"
   },
   "devDependencies": {
-    "@babel/preset-env": "^7.16.11",
+    "@babel/preset-env": "^7.17.10",
     "@babel/preset-typescript": "^7.16.7",
     "@types/jest": "^27.4.0",
-    "@types/node": "^17.0.27",
+    "@types/node": "^17.0.30",
     "@types/prettier": "^2.4.3",
-    "@types/react": "^18.0.7",
+    "@types/react": "^18.0.8",
     "@typescript-eslint/eslint-plugin": "^5.21.0",
     "@typescript-eslint/parser": "^5.21.0",
     "babel-loader": "^8.2.5",
     "codecov": "^3.8.3",
     "eslint": "^8.14.0",
     "eslint-config-prettier": "^8.1.0",
     "husky": "4.3.8",
-    "jest": "^28.0.0",
+    "jest": "^28.0.3",
     "prettier-package-json": "^2.1.3",
-    "typescript": "^4.5.5",
+    "typescript": "^4.6.4",
     "webpack": "^5.67.0",
     "webpack-cli": "^4.9.2"
   },

diff --git a/public.package.json b/public.package.json
@@ -1,6 +1,6 @@
 {
   "name": "twirpscript",
-  "version": "0.0.54",
+  "version": "0.0.55",
   "description": "Protobuf RPC framework for JavaScript and TypeScript",
   "license": "MIT",
   "author": "Tate <tatethurston@gmail.com>",
@@ -16,7 +16,7 @@
   "sideEffects": false,
   "types": "index.d.ts",
   "dependencies": {
-    "@types/google-protobuf": "^3.15.3",
+    "@types/google-protobuf": "^3.15.6",
     "google-protobuf": "^3.20.1",
     "prettier": "^2.6.0"
   },

diff --git a/src/autogenerate/index.ts b/src/autogenerate/index.ts
@@ -408,23 +408,41 @@ function writeSerializers(types: ProtoTypes[], isTopLevel: boolean): string {
                         }
                         res += `reader.readMessage(msg.${field.name}, ${field.tsType}._readMessage);`;
                       }
-                    } else if (field.read === "readEnum") {
-                      if (field.repeated) {
-                        res += `msg.${field.name}.push(${field.tsType}._fromInt(reader.${field.read}()));`;
-                      } else {
-                        res += `msg.${field.name} = ${field.tsType}._fromInt(reader.${field.read}());`;
-                      }
-                    } else if (field.tsType === "bigint") {
-                      if (field.repeated) {
-                        res += `msg.${field.name}.push(BigInt(reader.${field.read}()));`;
-                      } else {
-                        res += `msg.${field.name} = BigInt(reader.${field.read}());`;
-                      }
                     } else {
+                      let converter;
+                      if (field.read === "readEnum") {
+                        converter = `${field.tsType}._fromInt`;
+                      } else if (field.tsType === "bigint") {
+                        converter = "BigInt";
+                      }
                       if (field.repeated) {
-                        res += `msg.${field.name}.push(reader.${field.read}());`;
+                        if (converter) {
+                          if (field.readPacked) {
+                            res += `if (reader.isDelimited()) {`;
+                            res += `msg.${field.name}.push(...reader.${field.readPacked}().map(${converter}));`;
+                            res += `} else {`;
+                            res += `msg.${field.name}.push(${converter}(reader.${field.read}()));`;
+                            res += `}`;
+                          } else {
+                            res += `msg.${field.name}.push(${converter}(reader.${field.read}()));`;
+                          }
+                        } else {
+                          if (field.readPacked) {
+                            res += `if (reader.isDelimited()) {`;
+                            res += `msg.${field.name}.push(...reader.${field.readPacked}());`;
+                            res += `} else {`;
+                            res += `msg.${field.name}.push(reader.${field.read}());`;
+                            res += `}`;
+                          } else {
+                            res += `msg.${field.name}.push(reader.${field.read}());`;
+                          }
+                        }
                       } else {
-                        res += `msg.${field.name} = reader.${field.read}();`;
+                        if (converter) {
+                          res += `msg.${field.name} = ${converter}(reader.${field.read}());`;
+                        } else {
+                          res += `msg.${field.name} = reader.${field.read}();`;
+                        }
                       }
                     }
                     res += "break;\n}";

diff --git a/src/runtime/client/mocks.ts → src/runtime/client/test.mocks.ts b/src/runtime/client/mocks.ts → src/runtime/client/test.mocks.ts
diff --git a/src/runtime/client/test.ts b/src/runtime/client/test.ts
@@ -1,6 +1,6 @@
 import { describe, it } from "@jest/globals";
 import { client, JSONrequest, PBrequest, RpcTransportResponse } from ".";
-import { mockRpcTransportResponse } from "./mocks";
+import { mockRpcTransportResponse } from "./test.mocks";
 
 const mockRpcTransport = jest.fn(() =>
   Promise.resolve(mockRpcTransportResponse)

diff --git a/src/runtime/error/test.ts b/src/runtime/error/test.ts
@@ -1,6 +1,6 @@
 import { describe, it } from "@jest/globals";
 import { twirpErrorFromResponse } from ".";
-import { mockRpcTransportResponse } from "../client/mocks";
+import { mockRpcTransportResponse } from "../client/test.mocks";
 
 describe("twirpErrorFromResponse", () => {
   it("Twirp error when Twirp error", async () => {