V2: Return zero values from ReflectMessage.get (#813)

packages/protobuf-bench/README.md

@@ -10,5 +10,5 @@ server would usually do.
 
 | code generator      | bundle size             | minified               | compressed         |
 |---------------------|------------------------:|-----------------------:|-------------------:|
-| protobuf-es         | 126,944 b      | 65,418 b | 15,948 b |
+| protobuf-es         | 126,937 b      | 65,433 b | 15,946 b |
 | protobuf-javascript | 394,384 b  | 288,654 b | 45,122 b |

packages/protobuf-test/src/reflect/reflect-list.test.ts

@@ -17,10 +17,11 @@ import {
   isReflectList,
   reflectList,
   reflect,
+  isReflectMessage,
 } from "@bufbuild/protobuf/reflect";
 import { getFieldByLocalName } from "../helpers.js";
 import * as proto3_ts from "../gen/ts/extra/proto3_pb.js";
-import { protoInt64 } from "@bufbuild/protobuf";
+import { create, protoInt64 } from "@bufbuild/protobuf";
 import { UserDesc } from "../gen/ts/extra/example_pb.js";
 
 describe("reflectList()", () => {
@@ -98,6 +99,13 @@ describe("ReflectList", () => {
       const list = reflectList(repeatedInt64JsStringField, local);
       expect(list.get(0)).toBe(n1);
     });
+    test("returns ReflectMessage for message list", () => {
+      const list = reflectList(repeatedMessageField, [
+        create(proto3_ts.Proto3MessageDesc),
+      ]);
+      const val = list.get(0);
+      expect(isReflectMessage(val)).toBe(true);
+    });
   });
   describe("add()", () => {
     test("adds item", () => {

packages/protobuf-test/src/reflect/reflect-map.test.ts

@@ -15,9 +15,15 @@
 import { describe, expect, test } from "@jest/globals";
 import { getFieldByLocalName } from "../helpers.js";
 import * as proto3_ts from "../gen/ts/extra/proto3_pb.js";
-import { isReflectMap, reflectMap, reflect } from "@bufbuild/protobuf/reflect";
+import {
+  isReflectMap,
+  reflectMap,
+  reflect,
+  isReflectMessage,
+} from "@bufbuild/protobuf/reflect";
 import { protoInt64 } from "@bufbuild/protobuf";
 import { UserDesc } from "../gen/ts/extra/example_pb.js";
+import { create } from "@bufbuild/protobuf";
 
 describe("reflectMap()", () => {
   test("creates ReflectMap", () => {
@@ -105,6 +111,13 @@ describe("ReflectMap", () => {
       const map = reflectMap(mapInt64Int64Field, { "1": n11 });
       expect(map.get(n1)).toBeDefined();
     });
+    test("returns ReflectMessage for message map", () => {
+      const map = reflectMap(mapInt32MessageField, {
+        a: create(proto3_ts.Proto3MessageDesc),
+      });
+      const val = map.get("a");
+      expect(isReflectMessage(val)).toBe(true);
+    });
   });
   describe("keys()", () => {
     test("returns iterable keys", () => {

packages/protobuf-test/src/reflect/reflect.test.ts

@@ -181,9 +181,30 @@ describe("ReflectMessage", () => {
       };
       expect(r.get(f)).toBe(false);
     });
-    test("returns undefined for unselected oneof field", () => {
-      const f = getFieldByLocalName(desc, "oneofBoolField");
-      expect(r.get(f)).toBeUndefined();
+    describe("returns zero value for unset", () => {
+      test("scalar oneof field", () => {
+        const f = getFieldByLocalName(desc, "oneofBoolField");
+        expect(r.get(f)).toBe(false);
+      });
+      test("optional string field", () => {
+        const f = getFieldByLocalName(desc, "optionalStringField");
+        expect(r.get(f)).toBe("");
+      });
+      test("optional enum field", () => {
+        const f = getFieldByLocalName(desc, "optionalEnumField");
+        expect(r.get(f)).toBe(proto3_ts.Proto3Enum.UNSPECIFIED);
+      });
+      test("message field", () => {
+        const f = getFieldByLocalName(desc, "singularMessageField", "message");
+        const v = r.get(f);
+        expect(isReflectMessage(v)).toBe(true);
+        if (isReflectMessage(v)) {
+          for (const f of v.fields) {
+            expect(v.isSet(f)).toBe(false);
+          }
+        }
+        expect(r.isSet(f)).toBe(false);
+      });
     });
     test("returns ReflectMessage with zero message for unset message field", () => {
       const f = getFieldByLocalName(desc, "singularMessageField", "message");
@@ -195,10 +216,6 @@ describe("ReflectMessage", () => {
         }
       }
     });
-    test("returns undefined for unset optional string field", () => {
-      const f = getFieldByLocalName(desc, "optionalStringField");
-      expect(r.get(f)).toBeUndefined();
-    });
     test("returns bigint for jstype=JS_STRING", () => {
       const f = getFieldByLocalName(desc, "singularInt64JsStringField");
       msg.singularInt64JsStringField = "123";
@@ -356,6 +373,7 @@ describe("ReflectMessage", () => {
     });
     describe("returns error setting undefined", () => {
       test.each(desc.fields)("for proto3 field $name", (f) => {
+        // @ts-expect-error TS2345
         const err = r.set(f, undefined);
         expect(err).toBeDefined();
         expect(err?.message).toMatch(/^expected .*, got undefined/);

packages/protobuf/src/fields.ts

@@ -19,6 +19,19 @@ import type { DescMessage } from "./desc-types.js";
 
 /**
  * Returns true if the field is set.
+ *
+ * - Scalar and enum fields with implicit presence (proto3):
+ *   Set if not a zero value.
+ *
+ * - Scalar and enum fields with explicit presence (proto2, oneof):
+ *   Set if a value was set when creating or parsing the message, or when a
+ *   value was assigned to the field's property.
+ *
+ * - Message fields:
+ *   Set if the property is not undefined.
+ *
+ * - List and map fields:
+ *   Set if not empty.
  */
 export function isFieldSet<Desc extends DescMessage>(
   messageDesc: Desc,

packages/protobuf/src/reflect/reflect-types.ts

@@ -18,48 +18,172 @@ import { unsafeLocal } from "./unsafe.js";
 import type { Message, UnknownField } from "../types.js";
 import type { LongType, ScalarValue } from "./scalar.js";
 
+/**
+ * ReflectMessage provides dynamic access and manipulation of a message.
+ */
 export interface ReflectMessage {
+  /**
+   * The underlying message instance.
+   */
   readonly message: Message;
+
+  /**
+   * The descriptor for the message.
+   */
   readonly desc: DescMessage;
+
+  /**
+   * The fields of the message. This is a shortcut to message.fields.
+   */
   readonly fields: readonly DescField[];
+
+  /**
+   * The fields of the message, sorted by field number ascending.
+   */
   readonly sortedFields: readonly DescField[];
+
+  /**
+   * Oneof groups of the message. This is a shortcut to message.oneofs.
+   */
   readonly oneofs: readonly DescOneof[];
+
+  /**
+   * Fields and oneof groups for this message. This is a shortcut to message.members.
+   */
   readonly members: readonly (DescField | DescOneof)[];
 
+  /**
+   * Find a field by number.
+   */
   findNumber(number: number): DescField | undefined;
 
+  /**
+   * Returns true if the field is set.
+   *
+   * - Scalar and enum fields with implicit presence (proto3):
+   *   Set if not a zero value.
+   *
+   * - Scalar and enum fields with explicit presence (proto2, oneof):
+   *   Set if a value was set when creating or parsing the message, or when a
+   *   value was assigned to the field's property.
+   *
+   * - Message fields:
+   *   Set if the property is not undefined.
+   *
+   * - List and map fields:
+   *   Set if not empty.
+   */
   isSet(field: DescField): boolean;
 
+  /**
+   * Resets the field, so that isSet() will return false.
+   */
   clear(field: DescField): void;
 
+  /**
+   * Return the selected field of a oneof group.
+   */
   oneofCase(oneof: DescOneof): DescField | undefined;
 
+  /**
+   * Returns the field value. Values are converted or wrapped to make it easier
+   * to manipulate messages.
+   *
+   * - Scalar fields:
+   *   Returns the value, but converts 64-bit integer fields with the option
+   *   `jstype=JS_STRING` to a bigint value.
+   *   If the field is not set, the default value is returned. If no default
+   *   value is set, the zero value is returned.
+   *
+   * - Enum fields:
+   *   Returns the numeric value. If the field is not set, the default value is
+   *   returned. If no default value is set, the zero value is returned.
+   *
+   * - Message fields:
+   *   Returns a ReflectMessage. If the field is not set, a new message is
+   *   returned, but not set on the field.
+   *
+   * - List fields:
+   *   Returns a ReflectList object.
+   *
+   * - Map fields:
+   *   Returns a ReflectMap object.
+   *
+   * Note that get() never returns `undefined`. To determine whether a field is
+   * set, use isSet().
+   */
   get<Field extends DescField>(field: Field): ReflectGetValue<Field>;
 
+  /**
+   * Set a field value.
+   *
+   * Expects values in the same form that get() returns:
+   *
+   * - Scalar fields:
+   *   64-bit integer fields with the option `jstype=JS_STRING` as a bigint value.
+   *
+   * - Message fields:
+   *   ReflectMessage.
+   *
+   * - List fields:
+   *   ReflectList.
+   *
+   * - Map fields:
+   *   ReflectMap.
+   *
+   * Returns an error if the value is invalid for the field. `undefined` is not
+   * a valid value. To reset a field, use clear().
+   */
   set<Field extends DescField>(
     field: Field,
     value: ReflectSetValue<Field>,
   ): FieldError | undefined;
 
+  /**
+   * Add an item to a list field.
+   */
   addListItem<Field extends DescField & { fieldKind: "list" }>(
     field: Field,
     value: NewListItem<Field>,
   ): FieldError | undefined;
 
+  /**
+   * Set a map entry.
+   */
   setMapEntry<Field extends DescField & { fieldKind: "map" }>(
     field: Field,
     key: MapEntryKey,
     value: NewMapEntryValue<Field>,
   ): FieldError | undefined;
 
+  /**
+   * Returns the unknown fields of the message.
+   */
   getUnknown(): UnknownField[] | undefined;
 
+  /**
+   * Sets the unknown fields of the message, overwriting any previous values.
+   */
   setUnknown(value: UnknownField[]): void;
 
   [unsafeLocal]: Message;
 }
 
+/**
+ * ReflectList provides dynamic access and manipulation of a list field on a
+ * message.
+ *
+ * ReflectList is iterable - you can loop through all items with a for...of loop.
+ *
+ * Values are converted or wrapped to make it easier to manipulate them:
+ * - Scalar 64-bit integer fields with the option `jstype=JS_STRING` are
+ *   converted to bigint.
+ * - Messages are wrapped in a ReflectMessage.
+ */
 export interface ReflectList<V = unknown> extends Iterable<V> {
+  /**
+   * Returns the list field.
+   */
   field(): DescField & { fieldKind: "list" };
 
   /**
@@ -102,8 +226,22 @@ export interface ReflectList<V = unknown> extends Iterable<V> {
   [unsafeLocal]: unknown[];
 }
 
+/**
+ * ReflectMap provides dynamic access and manipulation of a map field on a
+ * message.
+ *
+ * ReflectMap is iterable - you can loop through all entries with a for...of loop.
+ *
+ * Keys and values are converted or wrapped to make it easier to manipulate them:
+ * - A map field is a record object on a message, where keys are always strings.
+ *   ReflectMap converts keys to their closest possible type in TypeScript.
+ * - Messages are wrapped in a ReflectMessage.
+ */
 export interface ReflectMap<K extends MapEntryKey = MapEntryKey, V = unknown>
   extends ReadonlyMap<K, V> {
+  /**
+   * Returns the map field.
+   */
   field(): DescField & { fieldKind: "map" };
 
   /**
@@ -139,19 +277,19 @@ type ReflectGetValue<Field extends DescField = DescField> = (
     never
   ) :
   Field extends { fieldKind: "list" } ? ReflectList :
-  Field extends { fieldKind: "enum" } ? number | undefined :
+  Field extends { fieldKind: "enum" } ? number :
   Field extends { fieldKind: "message" } ? ReflectMessage :
-  Field extends { fieldKind: "scalar"; scalar: infer T } ? ScalarValue<T> | undefined:
+  Field extends { fieldKind: "scalar"; scalar: infer T } ? ScalarValue<T> :
   never
 );
 
 // prettier-ignore
 type ReflectSetValue<Field extends DescField = DescField> = (
   Field extends { fieldKind: "map" } ? ReflectMap :
   Field extends { fieldKind: "list" } ? ReflectList :
-  Field extends { fieldKind: "enum" } ? number | undefined :
+  Field extends { fieldKind: "enum" } ? number :
   Field extends { fieldKind: "message" } ? ReflectMessage :
-  Field extends { fieldKind: "scalar"; scalar: infer T } ? ScalarValue<T> | undefined:
+  Field extends { fieldKind: "scalar"; scalar: infer T } ? ScalarValue<T> :
   never
 );