Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V2: Return zero values from ReflectMessage.get #813

Merged
merged 2 commits into from
Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/protobuf-bench/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
10 changes: 9 additions & 1 deletion packages/protobuf-test/src/reflect/reflect-list.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()", () => {
Expand Down Expand Up @@ -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", () => {
Expand Down
15 changes: 14 additions & 1 deletion packages/protobuf-test/src/reflect/reflect-map.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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", () => {
Expand Down
32 changes: 25 additions & 7 deletions packages/protobuf-test/src/reflect/reflect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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";
Expand Down Expand Up @@ -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/);
Expand Down
13 changes: 13 additions & 0 deletions packages/protobuf/src/fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
146 changes: 142 additions & 4 deletions packages/protobuf/src/reflect/reflect-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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" };

/**
Expand Down Expand Up @@ -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" };

/**
Expand Down Expand Up @@ -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
);

Expand Down
Loading
Loading