Skip to content

Commit

Permalink
Replace LongType with a boolean
Browse files Browse the repository at this point in the history
  • Loading branch information
timostamm committed May 22, 2024
1 parent 633eaa4 commit aed8182
Show file tree
Hide file tree
Showing 17 changed files with 101 additions and 138 deletions.
10 changes: 5 additions & 5 deletions packages/bundle-size/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ usually do. We repeat this for an increasing number of files.
<!--- TABLE-START -->
| code generator | files | bundle size | minified | compressed |
|-----------------|----------|------------------------:|-----------------------:|-------------------:|
| protobuf-es | 1 | 124,299 b | 64,356 b | 15,017 b |
| protobuf-es | 4 | 126,494 b | 65,860 b | 15,694 b |
| protobuf-es | 8 | 129,272 b | 67,631 b | 16,231 b |
| protobuf-es | 16 | 139,780 b | 75,612 b | 18,547 b |
| protobuf-es | 32 | 167,675 b | 97,639 b | 24,024 b |
| protobuf-es | 1 | 123,984 b | 64,216 b | 14,981 b |
| protobuf-es | 4 | 126,179 b | 65,722 b | 15,659 b |
| protobuf-es | 8 | 128,957 b | 67,493 b | 16,165 b |
| protobuf-es | 16 | 139,465 b | 75,474 b | 18,514 b |
| protobuf-es | 32 | 167,360 b | 97,499 b | 23,962 b |
| protobuf-javascript | 1 | 339,613 b | 255,820 b | 42,481 b |
| protobuf-javascript | 4 | 366,281 b | 271,092 b | 43,912 b |
| protobuf-javascript | 8 | 388,324 b | 283,409 b | 45,038 b |
Expand Down
12 changes: 6 additions & 6 deletions packages/bundle-size/chart.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
21 changes: 10 additions & 11 deletions packages/protobuf-test/src/registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {
type DescMessage,
type DescOneof,
type DescService,
LongType,
ScalarType,
} from "@bufbuild/protobuf";
import { protoCamelCase } from "@bufbuild/protobuf/reflect";
Expand Down Expand Up @@ -951,8 +950,8 @@ describe("DescField", () => {
).toBe(false);
});
});
describe("longType", () => {
test("returns default LongType.BIGINT for option omitted", async () => {
describe("longAsString", () => {
test("returns default false for option omitted", async () => {
const { fields } = await compileMessage(`
syntax="proto3";
message M {
Expand All @@ -978,17 +977,17 @@ describe("DescField", () => {
field.fieldKind == "scalar" ||
(field.fieldKind == "list" && field.listKind == "scalar")
) {
expect(field.longType).toBe(LongType.BIGINT);
expect(field.longAsString).toBe(false);
}
}
});
test.each([
{ jstype: "JS_NORMAL", longType: "BIGINT" },
{ jstype: "JS_NUMBER", longType: "BIGINT" },
{ jstype: "JS_STRING", longType: "STRING" },
{ jstype: "JS_NORMAL", longAsString: false },
{ jstype: "JS_NUMBER", longAsString: false },
{ jstype: "JS_STRING", longAsString: true },
] as const)(
"returns default LongType.$longType for jstype=$jstype",
async ({ jstype, longType }) => {
async ({ jstype, longAsString }) => {
const { fields } = await compileMessage(`
syntax="proto3";
message M {
Expand All @@ -1014,7 +1013,7 @@ describe("DescField", () => {
field.fieldKind == "scalar" ||
(field.fieldKind == "list" && field.listKind == "scalar")
) {
expect(field.longType).toBe(LongType[longType]);
expect(field.longAsString).toBe(longAsString);
}
}
},
Expand Down Expand Up @@ -1314,7 +1313,7 @@ describe("DescField", () => {
}
case "scalar": {
// exclusive to scalar (list and singular)
field.longType;
field.longAsString;

// exclusive to list
// @ts-expect-error TS2339
Expand Down Expand Up @@ -1378,7 +1377,7 @@ describe("DescField", () => {

switch (field.listKind) {
case "scalar": {
field.longType;
field.longAsString;
const scalar: ScalarType = field.scalar;
const message: undefined = field.message;
const enumeration: undefined = field.enum;
Expand Down
5 changes: 2 additions & 3 deletions packages/protobuf/src/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
type DescField,
type DescMessage,
type DescOneof,
LongType,
ScalarType,
} from "./descriptors.js";
import type { Message, MessageInitShape, MessageShape } from "./types.js";
Expand Down Expand Up @@ -273,11 +272,11 @@ function createZeroField(
}
const defaultValue = field.getDefaultValue();
if (defaultValue !== undefined) {
return field.fieldKind == "scalar" && field.longType == LongType.STRING
return field.fieldKind == "scalar" && field.longAsString
? defaultValue.toString()
: defaultValue;
}
return field.fieldKind == "scalar"
? scalarZeroValue(field.scalar, field.longType)
? scalarZeroValue(field.scalar, field.longAsString)
: field.enum.values[0].number;
}
47 changes: 12 additions & 35 deletions packages/protobuf/src/descriptors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,35 +77,6 @@ export enum ScalarType {
SINT64 = 18, // Uses ZigZag encoding.
}

/**
* JavaScript representation of fields with 64 bit integral types (int64, uint64,
* sint64, fixed64, sfixed64).
*
* This is a subset of google.protobuf.FieldOptions.JSType, which defines JS_NORMAL,
* JS_STRING, and JS_NUMBER. Protobuf-ES uses BigInt by default, but will use
* String if `[jstype = JS_STRING]` is specified.
*
* ```protobuf
* uint64 field_a = 1; // BigInt
* uint64 field_b = 2 [jstype = JS_NORMAL]; // BigInt
* uint64 field_b = 2 [jstype = JS_NUMBER]; // BigInt
* uint64 field_b = 2 [jstype = JS_STRING]; // String
* ```
*/
export enum LongType {
/**
* Use JavaScript BigInt.
*/
BIGINT = 0,

/**
* Use JavaScript String.
*
* Field option `[jstype = JS_STRING]`.
*/
STRING = 1,
}

/**
* A union of all descriptors, discriminated by a `kind` property.
*/
Expand Down Expand Up @@ -419,10 +390,13 @@ type descFieldScalar<T extends ScalarType = ScalarType> = T extends T
*/
readonly scalar: T;
/**
* JavaScript type for 64 bit integral types (int64, uint64,
* sint64, fixed64, sfixed64).
* By default, 64-bit integral types (int64, uint64, sint64, fixed64,
* sfixed64) are represented with BigInt.
*
* If the field option `jstype = JS_STRING` is set, this property
* is true, and 64-bit integral types are represented with String.
*/
readonly longType: LongType;
readonly longAsString: boolean;
/**
* The message type, if it is a message field.
*/
Expand Down Expand Up @@ -517,10 +491,13 @@ type descFieldListScalar<T extends ScalarType = ScalarType> = T extends T
*/
readonly scalar: T;
/**
* JavaScript type for 64 bit integral types (int64, uint64,
* sint64, fixed64, sfixed64).
* By default, 64-bit integral types (int64, uint64, sint64, fixed64,
* sfixed64) are represented with BigInt.
*
* If the field option `jstype = JS_STRING` is set, this property
* is true, and 64-bit integral types are represented with String.
*/
readonly longType: LongType;
readonly longAsString: boolean;
}
: never;

Expand Down
2 changes: 1 addition & 1 deletion packages/protobuf/src/extensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ export function createExtensionContainer<Desc extends DescExtension>(
if (isWrapperDesc(desc)) {
return scalarZeroValue(
desc.fields[0].scalar,
desc.fields[0].longType,
desc.fields[0].longAsString,
) as ExtensionValueShape<Desc>;
}
return create(desc) as ExtensionValueShape<Desc>;
Expand Down
11 changes: 3 additions & 8 deletions packages/protobuf/src/from-binary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import {
type DescField,
type DescMessage,
LongType,
ScalarType,
} from "./descriptors.js";
import { type DescField, type DescMessage, ScalarType } from "./descriptors.js";
import type { MessageShape } from "./types.js";
import type { MapEntryKey, ReflectMessage } from "./reflect/index.js";
import { scalarZeroValue } from "./reflect/scalar.js";
Expand Down Expand Up @@ -200,12 +195,12 @@ function readMapEntry(
}
}
if (key === undefined) {
key = scalarZeroValue(field.mapKey, LongType.BIGINT);
key = scalarZeroValue(field.mapKey, false);
}
if (val === undefined) {
switch (field.mapKind) {
case "scalar":
val = scalarZeroValue(field.scalar, LongType.BIGINT);
val = scalarZeroValue(field.scalar, false);
break;
case "enum":
val = field.enum.values[0].number;
Expand Down
3 changes: 1 addition & 2 deletions packages/protobuf/src/from-json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
type DescField,
type DescMessage,
type DescOneof,
LongType,
ScalarType,
} from "./descriptors.js";
import type { JsonValue } from "./json-value.js";
Expand Down Expand Up @@ -478,7 +477,7 @@ function readScalar(
): ScalarValue | typeof tokenNull {
if (json === null) {
if (nullAsZeroValue) {
return scalarZeroValue(type, LongType.BIGINT);
return scalarZeroValue(type, false);
}
return tokenNull;
}
Expand Down
5 changes: 2 additions & 3 deletions packages/protobuf/src/reflect/reflect-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {
type DescField,
type DescMessage,
type DescOneof,
LongType,
} from "../descriptors.js";
import { FieldError } from "./error.js";
import { unsafeLocal } from "./unsafe.js";
Expand Down Expand Up @@ -284,7 +283,7 @@ export type ReflectGetValue<Field extends DescField = DescField> = (
Field extends { fieldKind: "map" } ? (
Field extends { mapKind: "message" } ? ReflectMap<MapEntryKey, ReflectMessage> :
Field extends { mapKind: "enum" } ? ReflectMap<MapEntryKey, enumVal> :
Field extends { mapKind: "scalar"; scalar: infer T } ? ReflectMap<MapEntryKey, ScalarValue<T, LongType.BIGINT>> :
Field extends { mapKind: "scalar"; scalar: infer T } ? ReflectMap<MapEntryKey, ScalarValue<T>> :
never
) :
Field extends { fieldKind: "list" } ? ReflectList :
Expand Down Expand Up @@ -325,6 +324,6 @@ export type ReflectAddListItemValue<Field extends DescField & { fieldKind: "list
export type ReflectSetMapEntryValue<Field extends DescField & { fieldKind: "map" }> = (
Field extends { mapKind: "enum" } ? enumVal :
Field extends { mapKind: "message" } ? ReflectMessage :
Field extends { mapKind: "scalar"; scalar: infer T } ? ScalarValue<T, LongType.BIGINT> :
Field extends { mapKind: "scalar"; scalar: infer T } ? ScalarValue<T> :
never
);
17 changes: 8 additions & 9 deletions packages/protobuf/src/reflect/reflect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {
type DescField,
type DescMessage,
type DescOneof,
LongType,
ScalarType,
} from "../descriptors.js";
import type { Message, MessageShape, UnknownField } from "../types.js";
Expand Down Expand Up @@ -171,7 +170,7 @@ class ReflectMessageImpl<Desc extends DescMessage> implements ReflectMessage {
case "scalar":
return (
value === undefined
? scalarZeroValue(field.scalar, LongType.BIGINT)
? scalarZeroValue(field.scalar, false)
: longToReflect(field, value)
) as ReflectGetValue<Field>;
case "enum":
Expand All @@ -190,7 +189,7 @@ class ReflectMessageImpl<Desc extends DescMessage> implements ReflectMessage {
return err;
}
}
let local: unknown = value;
let local: unknown;
if (isReflectMap(value) || isReflectList(value)) {
local = value[unsafeLocal];
} else if (isReflectMessage(value)) {
Expand Down Expand Up @@ -571,8 +570,8 @@ function longToReflect(field: DescField, value: unknown): unknown {
case ScalarType.SFIXED64:
case ScalarType.SINT64:
if (
"longType" in field &&
field.longType == LongType.STRING &&
"longAsString" in field &&
field.longAsString &&
typeof value == "string"
) {
value = protoInt64.parse(value);
Expand All @@ -581,8 +580,8 @@ function longToReflect(field: DescField, value: unknown): unknown {
case ScalarType.FIXED64:
case ScalarType.UINT64:
if (
"longType" in field &&
field.longType == LongType.STRING &&
"longAsString" in field &&
field.longAsString &&
typeof value == "string"
) {
value = protoInt64.uParse(value);
Expand All @@ -598,15 +597,15 @@ function longToLocal(field: DescField, value: unknown) {
case ScalarType.INT64:
case ScalarType.SFIXED64:
case ScalarType.SINT64:
if ("longType" in field && field.longType == LongType.STRING) {
if ("longAsString" in field && field.longAsString) {
value = String(value);
} else if (typeof value == "string" || typeof value == "number") {
value = protoInt64.parse(value);
}
break;
case ScalarType.FIXED64:
case ScalarType.UINT64:
if ("longType" in field && field.longType == LongType.STRING) {
if ("longAsString" in field && field.longAsString) {
value = String(value);
} else if (typeof value == "string" || typeof value == "number") {
value = protoInt64.uParse(value);
Expand Down
Loading

0 comments on commit aed8182

Please sign in to comment.