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

Replace instanceof Message usages #729

Merged
merged 19 commits into from
Mar 2, 2024
6 changes: 1 addition & 5 deletions docs/migrating.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ to Protobuf-ES.
|------------------------|-------------|---------------------|-------------|
| Initializers | ✅ | ❌ | ✅ |
| Plain properties | ✅ | ❌ | ✅ |
| `instanceof` | ✅ | ✅ | ❌ |
| JSON format | ✅ | ❌ | ✅ |
| Binary format | ✅ | ✅ | ✅ |
| TypeScript | ✅ | ❌ | ✅ |
Expand Down Expand Up @@ -423,12 +422,9 @@ declare var message: Example;

```diff
- Example.is(message);
+ message instanceof Example;
+ isMessage(message, Example);
```

Note that `instanceof` has much better performance characteristics than `is()`.
For that reason, we do not provide an equivalent to `isAssignable()`.


### Reflection

Expand Down
31 changes: 30 additions & 1 deletion docs/runtime_api.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ provided by the library.
- [Cloning messages](#cloning-messages)
- [Comparing messages](#comparing-messages)
- [Serializing messages](#serializing-messages)
- [Identifying messages](#identifying-messages)
- [Enumerations](#enumerations)
- [Extensions](#extensions)
- [Extensions and JSON](#extensions-and-json)
Expand Down Expand Up @@ -314,6 +315,33 @@ JSON.
To learn about serialization options and other details related to serialization,
see the section about [advanced serialization](#advanced-serialization).

### Identifying messages

To check whether a given object is a message, use the function [`isMessage`][src-isMessage].

`isMessage` is _mostly_ equivalent to the `instanceof` operator. For
example, `isMessage(foo, MyMessage)` is the same as `foo instanceof MyMessage`,
and `isMessage(foo)` is the same as `foo instanceof Message`.

The advantage of `isMessage` is that it compares identity by the message type
name, not by class identity. This makes it robust against the dual package
hazard and similar situations, where the same message is duplicated.

To determine if an object is any subtype of `Message`, pass that object to the
function. To determine if an object is a specific type of `Message`, pass the
object as well as the type.

```typescript
import { isMessage } from "@bufbuild/protobuf";

const user = new User({
firstName: "Homer",
});

isMessage(user); // true
isMessage(user, User); // true
isMessage(user, OtherMessageType); // false
```

## Enumerations

Expand Down Expand Up @@ -1100,7 +1128,7 @@ function to your users that processes this message:
```ts
export function sendUser(user: PartialMessage<User>) {
// convert partial messages into their full representation if necessary
const u = user instanceof User ? user : new User(user);
const u = isMessage(user, User) ? user : new User(user);
// process further...
const bytes = u.toBinary();
}
Expand Down Expand Up @@ -1178,6 +1206,7 @@ Note that any message is assignable to `AnyMessage`.
[src-PlainMessage]: https://github.com/bufbuild/protobuf-es/blob/9b8efb4f4eb8ff8ce9f56798e769914ee2069cd1/packages/protobuf/src/message.ts#L137
[src-AnyMessage]: https://github.com/bufbuild/protobuf-es/blob/9b8efb4f4eb8ff8ce9f56798e769914ee2069cd1/packages/protobuf/src/message.ts#L25
[src-toPlainMessage]: https://github.com/bufbuild/protobuf-es/blob/51573c39ff38a9b43b6f7c22ba6b5ba40fa3ec3a/packages/protobuf/src/to-plain-message.ts#L29
[src-isMessage]: https://github.com/bufbuild/protobuf-es/blob/3864c00709c444d5cf2cef694345b9beea7b3ed9/packages/protobuf/src/is-message.ts#L31
[@bufbuild/protobuf]: https://www.npmjs.com/package/@bufbuild/protobuf
[@bufbuild/protoplugin]: https://www.npmjs.com/package/@bufbuild/protoplugin
[pkg-protoplugin]: https://www.npmjs.com/package/@bufbuild/protoplugin
Expand Down
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 | 96,999 b | 41,438 b | 10,763 b |
| protobuf-es | 97,510 b | 41,660 b | 10,845 b |
| protobuf-javascript | 394,384 b | 288,654 b | 45,122 b |
3 changes: 2 additions & 1 deletion packages/protobuf-test/src/edition-feature-resolver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
FeatureSet_Utf8Validation,
FeatureSetDefaults,
FeatureSetDefaults_FeatureSetEditionDefault,
isMessage,
protoInt64,
ScalarType,
} from "@bufbuild/protobuf";
Expand Down Expand Up @@ -633,7 +634,7 @@ describe("FeatureResolver", function () {
...descExtensions: DescExtension[]
): FeatureSet;
function getDefaults(edition: Edition, ...rest: unknown[]) {
if (rest[0] instanceof FeatureSetDefaults) {
if (isMessage(rest[0], FeatureSetDefaults)) {
const compiledFeatureSetDefaults = rest[0];
const resolver = FeatureResolver.create(
edition,
Expand Down
65 changes: 65 additions & 0 deletions packages/protobuf-test/src/is-message.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// Copyright 2021-2024 Buf Technologies, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import { describe, expect, test } from "@jest/globals";
import { User as TS_User } from "./gen/ts/extra/example_pb.js";
import { User as JS_User } from "./gen/js/extra/example_pb.js";
import { isMessage } from "@bufbuild/protobuf";

describe("isMessage", () => {
smaye81 marked this conversation as resolved.
Show resolved Hide resolved
test("subclass of Message", () => {
const user = new TS_User({
firstName: "Homer",
lastName: "Simpson",
});

expect(isMessage(user)).toBeTruthy();
expect(isMessage(user, TS_User)).toBeTruthy();
});
test("returns false if expected Message property is not a function", () => {
const user = new TS_User({
firstName: "Homer",
lastName: "Simpson",
});
// @ts-expect-error: Setting to a boolean to force a failure
user.toJson = false;

expect(isMessage(user, TS_User)).toBeFalsy();
});
test("null returns false", () => {
expect(isMessage(null)).toBeFalsy();
expect(isMessage(null, TS_User)).toBeFalsy();
});
test("non-object returns false", () => {
expect(isMessage("test")).toBeFalsy();
expect(isMessage("test", TS_User)).toBeFalsy();
});
test("mixed instances", () => {
const user = new TS_User({
firstName: "Homer",
lastName: "Simpson",
});

expect(isMessage(user, JS_User)).toBeTruthy();
});
test("type guard works as expected", () => {
const user: unknown = new TS_User();
if (isMessage(user)) {
expect(user.toJsonString()).toBeDefined();
}
if (isMessage(user, TS_User)) {
expect(user.firstName).toBeDefined();
}
});
});
11 changes: 3 additions & 8 deletions packages/protobuf-test/src/mixing-instances.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,7 @@
// limitations under the License.

import { describe, expect, test } from "@jest/globals";
import {
MessageFieldMessage as TS_MessageFieldMessage,
MessageFieldMessage_TestMessage as TS_MessageFieldMessage_TestMessage,
} from "./gen/ts/extra/msg-message_pb.js";
import { MessageFieldMessage as TS_MessageFieldMessage } from "./gen/ts/extra/msg-message_pb.js";
import { MessageFieldMessage_TestMessage as JS_MessageFieldMessage_TestMessage } from "./gen/js/extra/msg-message_pb.js";

describe("mixing message instances", () => {
Expand All @@ -37,14 +34,12 @@ describe("mixing message instances", () => {
});

describe("mixing message instances in the constructor", () => {
test("normalizes by creating a new instance", () => {
test("matches expected message", () => {
const test = new JS_MessageFieldMessage_TestMessage({ name: "foo" });
const message = new TS_MessageFieldMessage({
messageField: test,
});
expect(message.messageField?.name).toBe("foo");
expect(message.messageField).toBeInstanceOf(
TS_MessageFieldMessage_TestMessage,
);
expect(message.messageField).toBe(test);
});
});
12 changes: 6 additions & 6 deletions packages/protobuf/src/create-descriptor-set.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import type { BinaryReadOptions, BinaryWriteOptions } from "./binary-format.js";
import type { FeatureResolverFn } from "./private/feature-set.js";
import { createFeatureResolver } from "./private/feature-set.js";
import { LongType, ScalarType } from "./scalar.js";
import { isMessage } from "./is-message";

/**
* Create a DescriptorSet, a convenient interface for working with a set of
Expand All @@ -76,12 +77,11 @@ export function createDescriptorSet(
extensions: new Map<string, DescExtension>(),
mapEntries: new Map<string, DescMessage>(),
};
const fileDescriptors =
input instanceof FileDescriptorSet
? input.file
: input instanceof Uint8Array
? FileDescriptorSet.fromBinary(input).file
: input;
const fileDescriptors = isMessage(input, FileDescriptorSet)
? input.file
: input instanceof Uint8Array
? FileDescriptorSet.fromBinary(input).file
: input;
const resolverByEdition = new Map<Edition, FeatureResolverFn>();
for (const proto of fileDescriptors) {
const edition =
Expand Down
3 changes: 2 additions & 1 deletion packages/protobuf/src/create-registry-from-desc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ import type {
import { createDescriptorSet } from "./create-descriptor-set.js";
import type { Extension } from "./extension.js";
import type { ExtensionFieldSource } from "./private/extensions.js";
import { isMessage } from "./is-message";

// well-known message types with specialized JSON representation
const wkMessages = [
Expand Down Expand Up @@ -111,7 +112,7 @@ export function createRegistryFromDescriptors(
IExtensionRegistry &
IServiceTypeRegistry {
const set: DescriptorSet =
input instanceof Uint8Array || input instanceof FileDescriptorSet
input instanceof Uint8Array || isMessage(input, FileDescriptorSet)
? createDescriptorSet(input)
: input;
const enums = new Map<string, EnumType>();
Expand Down
18 changes: 12 additions & 6 deletions packages/protobuf/src/is-message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,22 @@ import type { AnyMessage } from "./message.js";
import { Message } from "./message.js";

/**
* Check whether the given object is an instance of the given message type.
*
* This function is equivalent to the `instanceof` operator. For example,
* `isMessage(foo, MyMessage)` is the same as `foo instanceof MyMessage`, and
* `isMessage(foo)` is the same as `foo instanceof Message`.
* Check whether the given object is any subtype of Message or is a specific
* Message by passing the type.
*
* Just like `instanceof`, `isMessage` narrows the type. The advantage of
* `isMessage` is that it compares identity by the message type name, not by
* class identity. This makes it robust against the dual package hazard and
* similar situations, where the same message is duplicated.
*
* This function is _mostly_ equivalent to the `instanceof` operator. For
* example, `isMessage(foo, MyMessage)` is the same as `foo instanceof MyMessage`,
* and `isMessage(foo)` is the same as `foo instanceof Message`. In most cases,
* `isMessage` should be preferred over `instanceof`.
*
* However, due to the fact that `isMessage` does not use class identity, there
* are subtle differences between this function and `instanceof`. Notably,
* calling `isMessage` on an explicit type of Message will return false.
*/
export function isMessage<T extends Message<T> = AnyMessage>(
arg: unknown,
Expand All @@ -46,7 +52,7 @@ export function isMessage<T extends Message<T> = AnyMessage>(
const actualType = (arg as { getType(): unknown }).getType();
if (
actualType === null ||
typeof actualType != "object" ||
typeof actualType != "function" ||
!("typeName" in actualType) ||
typeof actualType.typeName != "string"
) {
Expand Down
3 changes: 2 additions & 1 deletion packages/protobuf/src/private/binary-format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { assert } from "./assert.js";
import { isFieldSet } from "./reflect.js";
import type { ScalarValue } from "../scalar.js";
import { LongType, ScalarType } from "../scalar.js";
import { isMessage } from "../is-message";

/* eslint-disable prefer-const,no-case-declarations,@typescript-eslint/no-explicit-any,@typescript-eslint/no-unsafe-argument,@typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-member-access,@typescript-eslint/no-unsafe-call,@typescript-eslint/no-unsafe-return */

Expand Down Expand Up @@ -210,7 +211,7 @@ function readField(
readMessageField(reader, new messageType(), options, field),
);
} else {
if (target[localName] instanceof Message) {
if (isMessage(target[localName])) {
readMessageField(reader, target[localName], options, field);
} else {
target[localName] = readMessageField(
Expand Down
3 changes: 2 additions & 1 deletion packages/protobuf/src/private/field-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { Message } from "../message.js";
import type { MessageType } from "../message-type.js";
import type { DescExtension, DescField } from "../descriptor-set.js";
import { ScalarType } from "../scalar.js";
import { isMessage } from "../is-message";

/* eslint-disable @typescript-eslint/no-explicit-any -- unknown fields are represented with any */

Expand All @@ -40,7 +41,7 @@ export function wrapField<T extends Message<T>>(
type: MessageType<T>,
value: any,
): T {
if (value instanceof Message || !type.fieldWrapper) {
if (isMessage(value) || !type.fieldWrapper) {
return value as T;
}
return type.fieldWrapper.wrapField(value);
Expand Down
3 changes: 2 additions & 1 deletion packages/protobuf/src/private/json-format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import { scalarZeroValue } from "./scalars.js";
import { isScalarZeroValue } from "./scalars.js";
import type { ScalarValue } from "../scalar.js";
import { LongType, ScalarType } from "../scalar.js";
import { isMessage } from "../is-message";

/* eslint-disable no-case-declarations,@typescript-eslint/no-unsafe-argument,@typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-member-access,@typescript-eslint/no-unsafe-call */

Expand Down Expand Up @@ -385,7 +386,7 @@ function readField(
return;
}
let currentValue = target[localName] as Message | undefined;
if (currentValue instanceof Message) {
if (isMessage(currentValue)) {
currentValue.fromJson(jsonValue, options);
} else {
target[localName] = currentValue = messageType.fromJson(
Expand Down
9 changes: 5 additions & 4 deletions packages/protobuf/src/private/util-common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import type { MessageType } from "../message-type.js";
import type { Util } from "./util.js";
import { scalarEquals } from "./scalars.js";
import { ScalarType } from "../scalar.js";
import { isMessage } from "../is-message";

/* eslint-disable @typescript-eslint/no-explicit-any,@typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-member-access,@typescript-eslint/no-unsafe-return,@typescript-eslint/no-unsafe-argument,no-case-declarations */

Expand Down Expand Up @@ -52,7 +53,7 @@ export function makeUtilCommon(): Omit<Util, "newFieldList" | "initFields"> {
if (
sourceField &&
sourceField.kind == "message" &&
!(val instanceof sourceField.T)
!isMessage(val, sourceField.T)
) {
val = new sourceField.T(val);
} else if (
Expand Down Expand Up @@ -104,7 +105,7 @@ export function makeUtilCommon(): Omit<Util, "newFieldList" | "initFields"> {
const mt = member.T;
if (member.repeated) {
t[localName] = (s[localName] as any[]).map((val) =>
val instanceof mt ? val : new mt(val),
isMessage(val, mt) ? val : new mt(val),
);
} else {
const val = s[localName];
Expand All @@ -118,7 +119,7 @@ export function makeUtilCommon(): Omit<Util, "newFieldList" | "initFields"> {
t[localName] = val;
}
} else {
t[localName] = val instanceof mt ? val : new mt(val);
t[localName] = isMessage(val, mt) ? val : new mt(val);
}
}
break;
Expand Down Expand Up @@ -239,7 +240,7 @@ function cloneSingularField(value: any): any {
if (value === undefined) {
return value;
}
if (value instanceof Message) {
if (isMessage(value)) {
return value.clone();
}
if (value instanceof Uint8Array) {
Expand Down
Loading
Loading