Allow number[] for bytes fields in constructor (#533)

Follow up work to #511. This doesn't change the types but loosens the runtime to also accept `number[]` alongside `Uint8Array` values for `bytes` fields. --------- Co-authored-by: Timo Stamm <[email protected]>
bufbuild · Jul 26, 2023 · a6af1c1 · a6af1c1
1 parent d3f19c1
commit a6af1c1
Show file tree

Hide file tree

Showing 10 changed files with 119 additions and 13 deletions.
diff --git a/packages/protobuf-bench/README.md b/packages/protobuf-bench/README.md
@@ -10,5 +10,5 @@ server would usually do.
 
 | code generator      | bundle size             | minified               | compressed         |
 |---------------------|------------------------:|-----------------------:|-------------------:|
-| protobuf-es         | 87,781 b      | 37,517 b | 9,777 b |
+| protobuf-es         | 88,672 b      | 37,773 b | 9,865 b |
 | protobuf-javascript | 394,384 b  | 288,761 b | 45,123 b |
diff --git a/packages/protobuf-test/extra/msg-oneof.proto b/packages/protobuf-test/extra/msg-oneof.proto
@@ -19,6 +19,7 @@ message OneofMessage {
     oneof scalar {
         int32 value = 1;
         string error = 2;
+        bytes bytes = 3;
     }
     oneof message {
         OneofMessageFoo foo = 11;

diff --git a/packages/protobuf-test/src/gen/js/extra/msg-oneof_pb.d.ts b/packages/protobuf-test/src/gen/js/extra/msg-oneof_pb.d.ts
diff --git a/packages/protobuf-test/src/gen/js/extra/msg-oneof_pb.js b/packages/protobuf-test/src/gen/js/extra/msg-oneof_pb.js
diff --git a/packages/protobuf-test/src/gen/ts/extra/msg-oneof_pb.ts b/packages/protobuf-test/src/gen/ts/extra/msg-oneof_pb.ts
diff --git a/packages/protobuf-test/src/msg-maps.test.ts b/packages/protobuf-test/src/msg-maps.test.ts
@@ -105,4 +105,21 @@ describeMT({ ts: TS_MapsMessage, js: JS_MapsMessage }, (messageType) => {
     const got = { ...messageType.fromJson(exampleJson) };
     expect(got).toStrictEqual(exampleFields);
   });
+  test("allow number[] for bytes field", () => {
+    const bytes = [0xff];
+    const got = {
+      ...new messageType({
+        ...defaultFields,
+        strBytesField: {
+          a: bytes as any, //eslint-disable-line @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-explicit-any
+        },
+      }),
+    };
+    expect(got).toStrictEqual({
+      ...defaultFields,
+      strBytesField: {
+        a: new Uint8Array(bytes),
+      },
+    });
+  });
 });
diff --git a/packages/protobuf-test/src/msg-oneof.test.ts b/packages/protobuf-test/src/msg-oneof.test.ts
@@ -65,4 +65,17 @@ describeMT({ ts: TS_OneofMessage, js: JS_OneofMessage }, (messageType) => {
     const got = { ...messageType.fromJson(exampleJson) };
     expect(got).toStrictEqual(exampleFields);
   });
+  test("allows number[] for bytes field", () => {
+    const bytes = [0xff];
+    const got = {
+      ...new messageType({
+        ...defaultFields,
+        scalar: { case: "bytes", value: bytes as any }, //eslint-disable-line @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-explicit-any
+      }),
+    };
+    expect(got).toStrictEqual({
+      ...defaultFields,
+      scalar: { case: "bytes", value: new Uint8Array(bytes) },
+    });
+  });
 });
diff --git a/packages/protobuf-test/src/msg-scalar.test.ts b/packages/protobuf-test/src/msg-scalar.test.ts
@@ -106,6 +106,19 @@ describeMT(
       const got = { ...messageType.fromJson(exampleJson) };
       expect(got).toStrictEqual(exampleFields);
     });
+    test("allow number[] for bytes field", () => {
+      const bytes = [0xff];
+      const got = {
+        ...new messageType({
+          ...defaultFields,
+          bytesField: bytes as any, //eslint-disable-line @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-explicit-any
+        }),
+      };
+      expect(got).toStrictEqual({
+        ...defaultFields,
+        bytesField: new Uint8Array(bytes),
+      });
+    });
   }
 );
 
@@ -198,5 +211,18 @@ describeMT(
       const got = { ...messageType.fromJson(exampleJson) };
       expect(got).toStrictEqual(exampleFields);
     });
+    test("allow number[] for bytes field", () => {
+      const bytes = [0xff];
+      const got = {
+        ...new messageType({
+          ...defaultFields,
+          bytesField: [bytes] as any, //eslint-disable-line @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-explicit-any
+        }),
+      };
+      expect(got).toStrictEqual({
+        ...defaultFields,
+        bytesField: [new Uint8Array(bytes)],
+      });
+    });
   }
 );
diff --git a/packages/protobuf-test/src/wkt-wrappers.test.ts b/packages/protobuf-test/src/wkt-wrappers.test.ts
@@ -70,6 +70,15 @@ describeMT(
         const want = `{"boolValueField":true}`;
         expect(got).toBe(want);
       });
+      test("allow number[] for bytes field", () => {
+        const bytes = [0xff];
+        const got = {
+          ...new messageType({
+            bytesValueField: bytes as any, //eslint-disable-line @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-explicit-any
+          }),
+        };
+        expect(got.bytesValueField).toStrictEqual(new Uint8Array(bytes));
+      });
     });
     describe("oneof fields", () => {
       const w = new messageType({

diff --git a/packages/protobuf/src/private/util-common.ts b/packages/protobuf/src/private/util-common.ts
@@ -17,7 +17,6 @@ import { Message } from "../message.js";
 import type { AnyMessage, PartialMessage, PlainMessage } from "../message.js";
 import type { MessageType } from "../message-type.js";
 import { ScalarType } from "../field.js";
-import type { FieldInfo } from "../field.js";
 import type { Util } from "./util.js";
 import { scalarEquals } from "./scalars.js";
 
@@ -55,18 +54,36 @@ export function makeUtilCommon(): Omit<Util, "newFieldList" | "initFields"> {
               !(val instanceof sourceField.T)
             ) {
               val = new sourceField.T(val);
+            } else if (
+              sourceField &&
+              sourceField.kind === "scalar" &&
+              sourceField.T === ScalarType.BYTES
+            ) {
+              val = toU8Arr(val);
             }
             t[localName] = { case: sk, value: val };
             break;
           case "scalar":
           case "enum":
-            t[localName] = s[localName];
+            let copy = s[localName];
+            if (member.T === ScalarType.BYTES) {
+              copy = member.repeated
+                ? (copy as ArrayLike<number>[]).map(toU8Arr)
+                : toU8Arr(copy);
+            }
+            t[localName] = copy;
             break;
           case "map":
             switch (member.V.kind) {
               case "scalar":
               case "enum":
-                Object.assign(t[localName], s[localName]);
+                if (member.V.T === ScalarType.BYTES) {
+                  for (const [k, v] of Object.entries(s[localName])) {
+                    t[localName][k] = toU8Arr(v as ArrayLike<number>);
+                  }
+                } else {
+                  Object.assign(t[localName], s[localName]);
+                }
                 break;
               case "message":
                 const messageType = member.V.T;
@@ -91,7 +108,14 @@ export function makeUtilCommon(): Omit<Util, "newFieldList" | "initFields"> {
             } else if (s[localName] !== undefined) {
               const val = s[localName];
               if (mt.fieldWrapper) {
-                t[localName] = val;
+                if (
+                  // We can't use BytesValue.typeName as that will create a circular import
+                  mt.typeName === "google.protobuf.BytesValue"
+                ) {
+                  t[localName] = toU8Arr(val);
+                } else {
+                  t[localName] = val;
+                }
               } else {
                 t[localName] = val instanceof mt ? val : new mt(val);
               }
@@ -186,19 +210,19 @@ export function makeUtilCommon(): Omit<Util, "newFieldList" | "initFields"> {
         const source = (message as AnyMessage)[member.localName];
         let copy: any;
         if (member.repeated) {
-          copy = (source as any[]).map((e) => cloneSingularField(member, e));
+          copy = (source as any[]).map(cloneSingularField);
         } else if (member.kind == "map") {
           copy = any[member.localName];
           for (const [key, v] of Object.entries(source)) {
-            copy[key] = cloneSingularField(member.V, v);
+            copy[key] = cloneSingularField(v);
           }
         } else if (member.kind == "oneof") {
           const f = member.findField(source.case);
           copy = f
-            ? { case: source.case, value: cloneSingularField(f, source.value) }
+            ? { case: source.case, value: cloneSingularField(source.value) }
             : { case: undefined };
         } else {
-          copy = cloneSingularField(member, source);
+          copy = cloneSingularField(source);
         }
         any[member.localName] = copy;
       }
@@ -208,10 +232,7 @@ export function makeUtilCommon(): Omit<Util, "newFieldList" | "initFields"> {
 }
 
 // clone a single field value - i.e. the element type of repeated fields, the value type of maps
-function cloneSingularField(
-  field: FieldInfo | (FieldInfo & { kind: "map" })["V"],
-  value: any
-): any {
+function cloneSingularField(value: any): any {
   if (value === undefined) {
     return value;
   }
@@ -225,3 +246,8 @@ function cloneSingularField(
   }
   return value;
 }
+
+// converts any ArrayLike<number> to Uint8Array if necessary.
+function toU8Arr(input: ArrayLike<number>) {
+  return input instanceof Uint8Array ? input : new Uint8Array(input);
+}