fix: always use Map for int64 keys (#708) (#905)

* fix: always use Map for int64 keys (#708) * build: change prettier trailingComma to all
stephenh · Aug 18, 2023 · cf2fb59 · cf2fb59
1 parent 26fad31
commit cf2fb59
Show file tree

Hide file tree

Showing 11 changed files with 146 additions and 73 deletions.
diff --git a/.prettierrc b/.prettierrc
@@ -1,3 +1,4 @@
 {
-  "printWidth": 120
-}
+  "printWidth": 120,
+  "trailingComma": "all"
+}
diff --git a/integration/simple-long/simple-test.ts b/integration/simple-long/simple-test.ts
@@ -1,3 +1,4 @@
+import * as Long from "long";
 import { SimpleWithMap } from "./simple";
 
 describe("simple", () => {
@@ -6,7 +7,7 @@ describe("simple", () => {
     expect(s1).toMatchInlineSnapshot(`
       {
         "intLookup": {},
-        "longLookup": {},
+        "longLookup": Map {},
         "nameLookup": {},
       }
     `);
@@ -22,7 +23,7 @@ describe("simple", () => {
           "1": 2,
           "2": 1,
         },
-        "longLookup": {},
+        "longLookup": Map {},
         "nameLookup": {},
       }
     `);
@@ -31,21 +32,31 @@ describe("simple", () => {
   it("can fromPartial maps", () => {
     const s1 = SimpleWithMap.fromPartial({
       intLookup: { 1: 2, 2: 1 },
-      longLookup: { "1": 2, "2": 1 },
+      longLookup: new Map(),
     });
+    s1.longLookup.set(Long.fromInt(1), Long.fromInt(2));
+    s1.longLookup.set(Long.fromInt(2), Long.fromInt(1));
     expect(s1).toMatchInlineSnapshot(`
       {
         "intLookup": {
           "1": 2,
           "2": 1,
         },
-        "longLookup": {
-          "1": Long {
+        "longLookup": Map {
+          Long {
+            "high": 0,
+            "low": 1,
+            "unsigned": false,
+          } => Long {
             "high": 0,
             "low": 2,
             "unsigned": false,
           },
-          "2": Long {
+          Long {
+            "high": 0,
+            "low": 2,
+            "unsigned": false,
+          } => Long {
             "high": 0,
             "low": 1,
             "unsigned": false,
@@ -59,8 +70,10 @@ describe("simple", () => {
   it("can toJSON/fromJSON maps", () => {
     const s1 = SimpleWithMap.fromPartial({
       intLookup: { 1: 2, 2: 1 },
-      longLookup: { "1": 2, "2": 1 },
+      longLookup: new Map(),
     });
+    s1.longLookup.set(Long.fromInt(1), Long.fromInt(2));
+    s1.longLookup.set(Long.fromInt(2), Long.fromInt(1));
 
     const json = SimpleWithMap.toJSON(s1);
     expect(json).toMatchInlineSnapshot(`
@@ -83,13 +96,21 @@ describe("simple", () => {
           "1": 2,
           "2": 1,
         },
-        "longLookup": {
-          "1": Long {
+        "longLookup": Map {
+          Long {
+            "high": 0,
+            "low": 1,
+            "unsigned": false,
+          } => Long {
             "high": 0,
             "low": 2,
             "unsigned": false,
           },
-          "2": Long {
+          Long {
+            "high": 0,
+            "low": 2,
+            "unsigned": false,
+          } => Long {
             "high": 0,
             "low": 1,
             "unsigned": false,

diff --git a/integration/simple-long/simple.bin b/integration/simple-long/simple.bin
diff --git a/integration/simple-long/simple.proto b/integration/simple-long/simple.proto
@@ -14,8 +14,7 @@ message SimpleWithWrappers {
 message SimpleWithMap {
   map<string, string> nameLookup = 2;
   map<int32, int32> intLookup = 3;
-  // Ideally we'd test map<int64, int64> but we present maps as JS objects and `Long` cannot be used as a keys.
-  map<string, int64> longLookup = 4;
+  map<int64, int64> longLookup = 4;
 }
 
 message Numbers {

diff --git a/integration/simple-long/simple.ts b/integration/simple-long/simple.ts
@@ -17,8 +17,7 @@ export interface SimpleWithWrappers {
 export interface SimpleWithMap {
   nameLookup: { [key: string]: string };
   intLookup: { [key: number]: number };
-  /** Ideally we'd test map<int64, int64> but we present maps as JS objects and `Long` cannot be used as a keys. */
-  longLookup: { [key: string]: Long };
+  longLookup: Map<Long, Long>;
 }
 
 export interface SimpleWithMap_NameLookupEntry {
@@ -32,7 +31,7 @@ export interface SimpleWithMap_IntLookupEntry {
 }
 
 export interface SimpleWithMap_LongLookupEntry {
-  key: string;
+  key: Long;
   value: Long;
 }
 
@@ -190,7 +189,7 @@ export const SimpleWithWrappers = {
 };
 
 function createBaseSimpleWithMap(): SimpleWithMap {
-  return { nameLookup: {}, intLookup: {}, longLookup: {} };
+  return { nameLookup: {}, intLookup: {}, longLookup: new Map() };
 }
 
 export const SimpleWithMap = {
@@ -201,7 +200,7 @@ export const SimpleWithMap = {
     Object.entries(message.intLookup).forEach(([key, value]) => {
       SimpleWithMap_IntLookupEntry.encode({ key: key as any, value }, writer.uint32(26).fork()).ldelim();
     });
-    Object.entries(message.longLookup).forEach(([key, value]) => {
+    (message.longLookup).forEach((value, key) => {
       SimpleWithMap_LongLookupEntry.encode({ key: key as any, value }, writer.uint32(34).fork()).ldelim();
     });
     return writer;
@@ -241,7 +240,7 @@ export const SimpleWithMap = {
 
           const entry4 = SimpleWithMap_LongLookupEntry.decode(reader, reader.uint32());
           if (entry4.value !== undefined) {
-            message.longLookup[entry4.key] = entry4.value;
+            message.longLookup.set(entry4.key, entry4.value);
           }
           continue;
       }
@@ -268,11 +267,11 @@ export const SimpleWithMap = {
         }, {})
         : {},
       longLookup: isObject(object.longLookup)
-        ? Object.entries(object.longLookup).reduce<{ [key: string]: Long }>((acc, [key, value]) => {
-          acc[key] = Long.fromValue(value as Long | string);
+        ? Object.entries(object.longLookup).reduce<Map<Long, Long>>((acc, [key, value]) => {
+          acc.set(Long.fromValue(key), Long.fromValue(value as Long | string));
           return acc;
-        }, {})
-        : {},
+        }, new Map())
+        : new Map(),
     };
   },
 
@@ -296,14 +295,11 @@ export const SimpleWithMap = {
         });
       }
     }
-    if (message.longLookup) {
-      const entries = Object.entries(message.longLookup);
-      if (entries.length > 0) {
-        obj.longLookup = {};
-        entries.forEach(([k, v]) => {
-          obj.longLookup[k] = v.toString();
-        });
-      }
+    if (message.longLookup?.size) {
+      obj.longLookup = {};
+      message.longLookup.forEach((v, k) => {
+        obj.longLookup[longToNumber(k)] = v.toString();
+      });
     }
     return obj;
   },
@@ -331,15 +327,15 @@ export const SimpleWithMap = {
       },
       {},
     );
-    message.longLookup = Object.entries(object.longLookup ?? {}).reduce<{ [key: string]: Long }>(
-      (acc, [key, value]) => {
+    message.longLookup = (() => {
+      const m = new Map();
+      (object.longLookup as Map<Long, Long> ?? new Map()).forEach((value, key) => {
         if (value !== undefined) {
-          acc[key] = Long.fromValue(value);
+          m.set(key, Long.fromValue(value));
         }
-        return acc;
-      },
-      {},
-    );
+      });
+      return m;
+    })();
     return message;
   },
 };
@@ -489,13 +485,13 @@ export const SimpleWithMap_IntLookupEntry = {
 };
 
 function createBaseSimpleWithMap_LongLookupEntry(): SimpleWithMap_LongLookupEntry {
-  return { key: "", value: Long.ZERO };
+  return { key: Long.ZERO, value: Long.ZERO };
 }
 
 export const SimpleWithMap_LongLookupEntry = {
   encode(message: SimpleWithMap_LongLookupEntry, writer: _m0.Writer = _m0.Writer.create()): _m0.Writer {
-    if (message.key !== "") {
-      writer.uint32(10).string(message.key);
+    if (!message.key.isZero()) {
+      writer.uint32(8).int64(message.key);
     }
     if (!message.value.isZero()) {
       writer.uint32(16).int64(message.value);
@@ -511,11 +507,11 @@ export const SimpleWithMap_LongLookupEntry = {
       const tag = reader.uint32();
       switch (tag >>> 3) {
         case 1:
-          if (tag !== 10) {
+          if (tag !== 8) {
             break;
           }
 
-          message.key = reader.string();
+          message.key = reader.int64() as Long;
           continue;
         case 2:
           if (tag !== 16) {
@@ -535,15 +531,15 @@ export const SimpleWithMap_LongLookupEntry = {
 
   fromJSON(object: any): SimpleWithMap_LongLookupEntry {
     return {
-      key: isSet(object.key) ? String(object.key) : "",
+      key: isSet(object.key) ? Long.fromValue(object.key) : Long.ZERO,
       value: isSet(object.value) ? Long.fromValue(object.value) : Long.ZERO,
     };
   },
 
   toJSON(message: SimpleWithMap_LongLookupEntry): unknown {
     const obj: any = {};
-    if (message.key !== "") {
-      obj.key = message.key;
+    if (!message.key.isZero()) {
+      obj.key = (message.key || Long.ZERO).toString();
     }
     if (!message.value.isZero()) {
       obj.value = (message.value || Long.ZERO).toString();
@@ -558,7 +554,7 @@ export const SimpleWithMap_LongLookupEntry = {
     object: I,
   ): SimpleWithMap_LongLookupEntry {
     const message = createBaseSimpleWithMap_LongLookupEntry();
-    message.key = object.key ?? "";
+    message.key = (object.key !== undefined && object.key !== null) ? Long.fromValue(object.key) : Long.ZERO;
     message.value = (object.value !== undefined && object.value !== null) ? Long.fromValue(object.value) : Long.ZERO;
     return message;
   },
@@ -837,6 +833,25 @@ export const Numbers = {
   },
 };
 
+declare const self: any | undefined;
+declare const window: any | undefined;
+declare const global: any | undefined;
+const tsProtoGlobalThis: any = (() => {
+  if (typeof globalThis !== "undefined") {
+    return globalThis;
+  }
+  if (typeof self !== "undefined") {
+    return self;
+  }
+  if (typeof window !== "undefined") {
+    return window;
+  }
+  if (typeof global !== "undefined") {
+    return global;
+  }
+  throw "Unable to locate global object";
+})();
+
 type Builtin = Date | Function | Uint8Array | string | number | boolean | undefined;
 
 export type DeepPartial<T> = T extends Builtin ? T
@@ -849,6 +864,13 @@ type KeysOfUnion<T> = T extends T ? keyof T : never;
 export type Exact<P, I extends P> = P extends Builtin ? P
   : P & { [K in keyof P]: Exact<P[K], I[K]> } & { [K in Exclude<keyof I, KeysOfUnion<P>>]: never };
 
+function longToNumber(long: Long): number {
+  if (long.gt(Number.MAX_SAFE_INTEGER)) {
+    throw new tsProtoGlobalThis.Error("Value is larger than Number.MAX_SAFE_INTEGER");
+  }
+  return long.toNumber();
+}
+
 if (_m0.util.Long !== Long) {
   _m0.util.Long = Long as any;
   _m0.configure();

diff --git a/package.json b/package.json
@@ -54,7 +54,7 @@
     "mongodb": "^5.7.0",
     "nice-grpc": "^2.1.4",
     "object-hash": "^3.0.0",
-    "prettier": "^3.0.0",
+    "prettier": "^2.8.8",
     "protobufjs-cli": "^1.1.1",
     "reflect-metadata": "^0.1.13",
     "rxjs": "^7.8.1",

diff --git a/src/enums.ts b/src/enums.ts
@@ -41,8 +41,8 @@ export function generateEnum(
   if (options.unrecognizedEnum)
     chunks.push(code`
       ${UNRECOGNIZED_ENUM_NAME} ${delimiter} ${
-        options.stringEnums ? `"${UNRECOGNIZED_ENUM_NAME}"` : UNRECOGNIZED_ENUM_VALUE.toString()
-      },`);
+      options.stringEnums ? `"${UNRECOGNIZED_ENUM_NAME}"` : UNRECOGNIZED_ENUM_VALUE.toString()
+    },`);
 
   if (options.enumsAsLiterals) {
     chunks.push(code`} as const`);

diff --git a/src/generate-services.ts b/src/generate-services.ts
@@ -309,8 +309,8 @@ function generateCachingRpcMethod(
       const responses = requests.map(async request => {
         const data = ${inputType}.encode(request).finish()
         const response = await this.rpc.request(ctx, "${maybePrefixPackage(fileDesc, serviceDesc.name)}", "${
-          methodDesc.name
-        }", data);
+    methodDesc.name
+  }", data);
         return ${outputType}.decode(${Reader}.create(response));
       });
       return Promise.all(responses);