bufbuild · timostamm · May 13, 2024 · May 13, 2024
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         | 126,685 b      | 66,254 b | 15,996 b |
+| protobuf-es         | 126,744 b      | 66,286 b | 16,005 b |
 | protobuf-javascript | 394,384 b  | 288,654 b | 45,122 b |
diff --git a/packages/protobuf-test/src/codegenv1/boot.test.ts b/packages/protobuf-test/src/codegenv1/boot.test.ts
@@ -18,24 +18,22 @@ import { UpstreamProtobuf } from "upstream-protobuf";
 import { join as joinPath } from "node:path";
 import { readFileSync } from "fs";
 import { clearField, equals, fromBinary, toBinary } from "@bufbuild/protobuf";
-import type {
-  DescriptorProto,
-  FileDescriptorProto,
-} from "@bufbuild/protobuf/wkt";
 import {
+  type DescriptorProto,
+  type FileDescriptorProto,
   DescriptorProtoDesc,
-  FieldDescriptorProtoDesc,
   FileDescriptorProtoDesc,
   FileDescriptorSetDesc,
+  FieldDescriptorProtoDesc,
   FieldOptionsDesc,
 } from "@bufbuild/protobuf/wkt";
 import assert from "node:assert";
 import {
+  boot,
   bootFileDescriptorProto,
   createFileDescriptorProtoBoot,
   embedFileDesc,
 } from "@bufbuild/protobuf/codegenv1";
-import { boot } from "@bufbuild/protobuf/codegenv1";
 
 describe("boot()", () => {
   test("hydrates google/protobuf/descriptor.proto", async () => {
@@ -95,12 +93,12 @@ describe("bootFileDescriptorProto()", () => {
       d.messageType.forEach(stripLikeBoot);
       return;
     }
-    clearField(DescriptorProtoDesc, d, "reservedRange");
-    clearField(DescriptorProtoDesc, d, "reservedName");
+    clearField(d, DescriptorProtoDesc.field.reservedRange);
+    clearField(d, DescriptorProtoDesc.field.reservedName);
     for (const f of d.field) {
-      clearField(FieldDescriptorProtoDesc, f, "jsonName");
+      clearField(f, FieldDescriptorProtoDesc.field.jsonName);
       if (f.options) {
-        clearField(FieldOptionsDesc, f.options, "featureSupport");
+        clearField(f.options, FieldOptionsDesc.field.featureSupport);
       }
     }
     for (const n of d.nestedType) {

diff --git a/packages/protobuf-test/src/create.test.ts b/packages/protobuf-test/src/create.test.ts
@@ -540,7 +540,7 @@ describe("create()", () => {
             expect(created.either).toStrictEqual(filled.either);
             break;
           default:
-            expect(isFieldSet(desc, created, name)).toBe(true);
+            expect(isFieldSet(created, desc.field[name])).toBe(true);
             expect(created[name]).toStrictEqual(filled[name]);
         }
       });
@@ -561,7 +561,7 @@ describe("create()", () => {
             expect(created.either).toStrictEqual(filled.either);
             break;
           default:
-            expect(isFieldSet(desc, created, name)).toBe(true);
+            expect(isFieldSet(created, desc.field[name])).toBe(true);
             expect(created[name]).toStrictEqual(filled[name]);
         }
       });
@@ -582,7 +582,7 @@ describe("create()", () => {
             expect(created.either).toStrictEqual(filled.either);
             break;
           default:
-            expect(isFieldSet(desc, created, name)).toBe(true);
+            expect(isFieldSet(created, desc.field[name])).toBe(true);
             expect(created[name]).toStrictEqual(filled[name]);
         }
       });

diff --git a/packages/protobuf-test/src/fields.test.ts b/packages/protobuf-test/src/fields.test.ts
@@ -13,7 +13,6 @@
 // limitations under the License.
 
 import { beforeEach, describe, expect, test } from "@jest/globals";
-import type { DescMessage } from "@bufbuild/protobuf";
 import { clearField, create, isFieldSet } from "@bufbuild/protobuf";
 import * as proto3_ts from "./gen/ts/extra/proto3_pb.js";
 import * as proto2_ts from "./gen/ts/extra/proto2_pb.js";
@@ -26,69 +25,71 @@ import {
 } from "./helpers-edition2023.js";
 
 describe("isFieldSet()", () => {
-  test("accepts field names", () => {
+  test("returns true for set field", () => {
     const msg = create(proto3_ts.Proto3MessageDesc);
-    isFieldSet(proto3_ts.Proto3MessageDesc, msg, "singularStringField");
-    isFieldSet(proto3_ts.Proto3MessageDesc, msg, "optionalStringField");
-    isFieldSet(proto3_ts.Proto3MessageDesc, msg, "repeatedStringField");
-    isFieldSet(proto3_ts.Proto3MessageDesc, msg, "mapStringStringField");
-    isFieldSet(proto3_ts.Proto3MessageDesc, msg, "oneofBoolField");
+    msg.optionalStringField = "abc";
+    const set = isFieldSet(
+      msg,
+      proto3_ts.Proto3MessageDesc.field.optionalStringField,
+    );
+    expect(set).toBe(true);
   });
-  test("rejects unknown field names", () => {
+  test("returns true for unset field", () => {
     const msg = create(proto3_ts.Proto3MessageDesc);
-    // @ts-expect-error TS2345
-    isFieldSet(proto3_ts.Proto3MessageDesc, msg, "not a field name");
+    const set = isFieldSet(
+      msg,
+      proto3_ts.Proto3MessageDesc.field.optionalStringField,
+    );
+    expect(set).toBe(false);
   });
-  test("rejects oneof names", () => {
+  test("returns false for foreign field", () => {
     const msg = create(proto3_ts.Proto3MessageDesc);
-    // @ts-expect-error TS2345
-    isFieldSet(proto3_ts.Proto3MessageDesc, msg, "either");
-  });
-  test("accepts string for anonymous message", () => {
-    const desc: DescMessage = proto3_ts.Proto3MessageDesc;
-    const msg = create(desc);
-    const set = isFieldSet(desc, msg, "not a field name");
+    msg.optionalStringField = "abc";
+    const set = isFieldSet(
+      msg,
+      proto2_ts.Proto2MessageDesc.field.optionalStringField,
+    );
     expect(set).toBe(false);
   });
   describe("with proto3", () => {
     const desc = proto3_ts.Proto3MessageDesc;
     test.each(desc.fields)("%s is initially unset", (field) => {
       const msg = create(desc);
-      const set = isFieldSet(desc as DescMessage, msg, field.localName);
+      const set = isFieldSet(msg, field);
       expect(set).toBe(false);
     });
     test.each(fillProto3MessageNames())("%s is set", (name) => {
       const msg = create(desc);
       fillProto3Message(msg);
-      const set = isFieldSet(desc, msg, name);
+      const set = isFieldSet(msg, desc.field[name]);
       expect(set).toBe(true);
     });
   });
   describe("with proto2", () => {
     const desc = proto2_ts.Proto2MessageDesc;
     test.each(desc.fields)("%s is initially unset", (field) => {
       const msg = create(desc);
-      const set = isFieldSet(desc as DescMessage, msg, field.localName);
+      const set = isFieldSet(msg, field);
       expect(set).toBe(false);
     });
     test.each(fillProto2MessageNames())("%s is set", (name) => {
       const msg = create(desc);
       fillProto2Message(msg);
-      const set = isFieldSet(desc, msg, name);
+      const set = isFieldSet(msg, desc.field[name]);
       expect(set).toBe(true);
     });
   });
   describe("with edition2023", () => {
     const desc = edition2023_ts.Edition2023MessageDesc;
     test.each(desc.fields)("%s is initially unset", (field) => {
       const msg = create(desc);
-      const set = isFieldSet(desc as DescMessage, msg, field.localName);
+      const set = isFieldSet(msg, field);
       expect(set).toBe(false);
     });
     test.each(fillEdition2023MessageNames())("%s is set", (name) => {
       const msg = create(desc);
       fillEdition2023Message(msg);
-      const set = isFieldSet(desc, msg, name);
+      const set = isFieldSet(msg, desc.field[name]);
       expect(set).toBe(true);
     });
   });
@@ -105,9 +106,9 @@ describe("clearField()", () => {
       fillProto3Message(msg);
     });
     test.each(fillProto3MessageNames())("%s", (name) => {
-      expect(isFieldSet(desc, msg, name)).toBe(true);
-      clearField(desc, msg, name);
-      expect(isFieldSet(desc, msg, name)).toBe(false);
+      expect(isFieldSet(msg, desc.field[name])).toBe(true);
+      clearField(msg, desc.field[name]);
+      expect(isFieldSet(msg, desc.field[name])).toBe(false);
       switch (name) {
         case "oneofBoolField":
           expect(msg.either).toStrictEqual(zero.either);
@@ -135,9 +136,9 @@ describe("clearField()", () => {
       fillProto2Message(msg);
     });
     test.each(fillProto2MessageNames())("%s", (name) => {
-      expect(isFieldSet(desc, msg, name)).toBe(true);
-      clearField(desc, msg, name);
-      expect(isFieldSet(desc, msg, name)).toBe(false);
+      expect(isFieldSet(msg, desc.field[name])).toBe(true);
+      clearField(msg, desc.field[name]);
+      expect(isFieldSet(msg, desc.field[name])).toBe(false);
       switch (name) {
         case "oneofBoolField":
           expect(msg.either).toStrictEqual(zero.either);
@@ -162,9 +163,9 @@ describe("clearField()", () => {
       fillEdition2023Message(msg);
     });
     test.each(fillEdition2023MessageNames())("%s", (name) => {
-      expect(isFieldSet(desc, msg, name)).toBe(true);
-      clearField(desc, msg, name);
-      expect(isFieldSet(desc, msg, name)).toBe(false);
+      expect(isFieldSet(msg, desc.field[name])).toBe(true);
+      clearField(msg, desc.field[name]);
+      expect(isFieldSet(msg, desc.field[name])).toBe(false);
       switch (name) {
         case "oneofBoolField":
           expect(msg.either).toStrictEqual(zero.either);

diff --git a/packages/protobuf-test/src/generate-code.test.ts b/packages/protobuf-test/src/generate-code.test.ts
@@ -225,8 +225,13 @@ describe("ts generated code is equal to js generated code", () => {
           if (id !== undefined) {
             return id;
           }
-          if ("toString" in value) {
-            id = String(value) + "@" + seen.size;
+          if (
+            "toString" in value &&
+            typeof value.toString == "function" &&
+            Object.prototype.hasOwnProperty.call(value, "toString")
+          ) {
+            // eslint-disable-next-line @typescript-eslint/no-base-to-string -- we're not calling Object.toString
+            id = value.toString() + "@" + seen.size;
           } else {
             id = "unknown@" + seen.size;
           }
@@ -237,3 +242,11 @@ describe("ts generated code is equal to js generated code", () => {
     );
   }
 });
+
+describe("GenDescMessage.field", () => {
+  test("is type safe", () => {
+    proto3_ts.Proto3MessageDesc.field.optionalStringField;
+    // @ts-expect-error TS2339: Property foo does not exist on type
+    proto3_ts.Proto3MessageDesc.field.foo;
+  });
+});
diff --git a/packages/protobuf-test/src/helpers.ts b/packages/protobuf-test/src/helpers.ts
@@ -12,11 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import type { DescMessage } from "@bufbuild/protobuf";
 import { UpstreamProtobuf } from "upstream-protobuf";
 import { createFileRegistry } from "@bufbuild/protobuf/reflect";
-import * as proto3_ts from "./gen/ts/extra/proto3_pb.js";
-import type { DescField } from "@bufbuild/protobuf";
 import { fromBinary } from "@bufbuild/protobuf";
 import type { FileDescriptorSet } from "@bufbuild/protobuf/wkt";
 import { FileDescriptorSetDesc } from "@bufbuild/protobuf/wkt";
@@ -95,40 +92,3 @@ export async function compileMethod(proto: string) {
   assert(firstMethod);
   return firstMethod;
 }
-
-export function getFieldByLocalName(desc: DescMessage, name: string): DescField;
-export function getFieldByLocalName(
-  desc: DescMessage,
-  name: string,
-  fieldKind: "message",
-): DescField & { fieldKind: "message" };
-export function getFieldByLocalName(
-  desc: DescMessage,
-  name: string,
-  fieldKind: "list",
-): DescField & { fieldKind: "list" };
-export function getFieldByLocalName(
-  desc: DescMessage,
-  name: string,
-  fieldKind: "map",
-): DescField & { fieldKind: "map" };
-export function getFieldByLocalName(
-  desc: DescMessage,
-  name: string,
-  fieldKind?: string,
-): DescField {
-  const field = proto3_ts.Proto3MessageDesc.fields.find(
-    (f) => f.localName === name,
-  );
-  if (!field) {
-    throw new Error(`getFieldByLocalName: ${name} not found`);
-  }
-  if (fieldKind !== undefined) {
-    if (field.fieldKind != fieldKind) {
-      throw new Error(
-        `getFieldByLocalName: ${name} is not a ${fieldKind} field`,
-      );
-    }
-  }
-  return field;
-}
diff --git a/packages/protobuf-test/src/reflect/reflect-list.test.ts b/packages/protobuf-test/src/reflect/reflect-list.test.ts
@@ -19,54 +19,38 @@ import {
   reflect,
   isReflectMessage,
 } from "@bufbuild/protobuf/reflect";
-import { getFieldByLocalName } from "../helpers.js";
 import * as proto3_ts from "../gen/ts/extra/proto3_pb.js";
 import { create, protoInt64 } from "@bufbuild/protobuf";
 import { UserDesc } from "../gen/ts/extra/example_pb.js";
+import assert from "node:assert";
 
 describe("reflectList()", () => {
   test("creates ReflectList", () => {
-    const f = getFieldByLocalName(
-      proto3_ts.Proto3MessageDesc,
-      "repeatedStringField",
-      "list",
-    );
+    const f = proto3_ts.Proto3MessageDesc.field.repeatedStringField;
+    assert(f.fieldKind == "list");
     const list = reflectList(f);
     expect(typeof list.field).toBe("function");
     expect(isReflectList(list)).toBe(true);
   });
   test("creates ReflectList with unsafe input", () => {
-    const f = getFieldByLocalName(
-      proto3_ts.Proto3MessageDesc,
-      "repeatedStringField",
-      "list",
-    );
+    const f = proto3_ts.Proto3MessageDesc.field.repeatedStringField;
+    assert(f.fieldKind == "list");
     const list = reflectList(f, [1, 2, 3]);
     expect(isReflectList(list)).toBe(true);
   });
 });
 
 describe("ReflectList", () => {
-  const repeatedStringField = getFieldByLocalName(
-    proto3_ts.Proto3MessageDesc,
-    "repeatedStringField",
-    "list",
-  );
-  const repeatedInt64Field = getFieldByLocalName(
-    proto3_ts.Proto3MessageDesc,
-    "repeatedInt64Field",
-    "list",
-  );
-  const repeatedInt64JsStringField = getFieldByLocalName(
-    proto3_ts.Proto3MessageDesc,
-    "repeatedInt64JsStringField",
-    "list",
-  );
-  const repeatedMessageField = getFieldByLocalName(
-    proto3_ts.Proto3MessageDesc,
-    "repeatedMessageField",
-    "list",
-  );
+  const {
+    repeatedStringField,
+    repeatedInt64Field,
+    repeatedInt64JsStringField,
+    repeatedMessageField,
+  } = proto3_ts.Proto3MessageDesc.field;
+  assert(repeatedStringField.fieldKind == "list");
+  assert(repeatedInt64Field.fieldKind == "list");
+  assert(repeatedInt64JsStringField.fieldKind == "list");
+  assert(repeatedMessageField.fieldKind == "list");
   const n0 = protoInt64.zero;
   const n1 = protoInt64.parse(1);
   const n2 = protoInt64.parse(2);