diff --git a/packages/protobuf-bench/README.md b/packages/protobuf-bench/README.md index 426da7776..5d938e1d3 100644 --- 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 | 127,054 b | 65,501 b | 15,983 b | +| protobuf-es | 126,666 b | 65,334 b | 15,946 b | | protobuf-javascript | 394,384 b | 288,654 b | 45,122 b | diff --git a/packages/protobuf-test/src/reflect/registry.test.ts b/packages/protobuf-test/src/reflect/registry.test.ts index 2ace91588..9c91b630e 100644 --- a/packages/protobuf-test/src/reflect/registry.test.ts +++ b/packages/protobuf-test/src/reflect/registry.test.ts @@ -802,73 +802,6 @@ describe("DescField", () => { ).toBe(false); }); }); - describe("optional", () => { - test("false for proto2 required scalar", async () => { - const field = await compileField(` - syntax="proto2"; - message M { required int32 f1 = 1; } - `); - expect( - field.fieldKind == "scalar" || - field.fieldKind == "message" || - field.fieldKind == "enum" - ? field.optional - : undefined, - ).toBe(false); - }); - test("true for proto2 optional scalar", async () => { - const field = await compileField(` - syntax="proto2"; - message M { required int32 f1 = 1; } - `); - expect( - field.fieldKind == "scalar" || - field.fieldKind == "message" || - field.fieldKind == "enum" - ? field.optional - : undefined, - ).toBe(false); - }); - test("true for proto3 optional scalar", async () => { - const field = await compileField(` - syntax="proto3"; - message M { optional int32 f1 = 1; } - `); - expect( - field.fieldKind == "scalar" || - field.fieldKind == "message" || - field.fieldKind == "enum" - ? field.optional - : undefined, - ).toBe(true); - }); - test("false for features.field_presence = EXPLICIT", async () => { - const field = await compileField(` - edition="2023"; - message M { int32 f1 = 1 [features.field_presence = EXPLICIT]; } - `); - expect( - field.fieldKind == "scalar" || - field.fieldKind == "message" || - field.fieldKind == "enum" - ? field.optional - : undefined, - ).toBe(false); - }); - test("false for features.field_presence = IMPLICIT", async () => { - const field = await compileField(` - edition="2023"; - message M { int32 f1 = 1 [features.field_presence = IMPLICIT]; } - `); - expect( - field.fieldKind == "scalar" || - field.fieldKind == "message" || - field.fieldKind == "enum" - ? field.optional - : undefined, - ).toBe(false); - }); - }); describe("longType", () => { test("returns default LongType.BIGINT for option omitted", async () => { const { fields } = await compileMessage(` @@ -1171,7 +1104,6 @@ describe("DescField", () => { field.packed; // exclusive to singular - field.optional; const def: undefined = field.getDefaultValue(); // exclusive to map @@ -1194,7 +1126,6 @@ describe("DescField", () => { field.packed; // exclusive to singular - field.optional; const def: string | number | bigint | boolean | Uint8Array | undefined = field.getDefaultValue(); @@ -1229,7 +1160,6 @@ describe("DescField", () => { field.packed; // exclusive to singular - field.optional; const def: number | undefined = field.getDefaultValue(); // exclusive to map diff --git a/packages/protobuf/src/desc-types.ts b/packages/protobuf/src/desc-types.ts index b08f90e65..6b3b45fcc 100644 --- a/packages/protobuf/src/desc-types.ts +++ b/packages/protobuf/src/desc-types.ts @@ -325,10 +325,6 @@ type descFieldSingularCommon = { * This does not include synthetic oneofs for proto3 optionals. */ readonly oneof: DescOneof | undefined; - /** - * Whether this field was declared with `optional` in the protobuf source. - */ - readonly optional: boolean; /** * Presence of the field. * See https://protobuf.dev/programming-guides/field_presence/ diff --git a/packages/protobuf/src/reflect/registry.ts b/packages/protobuf/src/reflect/registry.ts index da0dae067..bc42b72b3 100644 --- a/packages/protobuf/src/reflect/registry.ts +++ b/packages/protobuf/src/reflect/registry.ts @@ -375,8 +375,6 @@ const TYPE_ENUM: FieldDescriptorProto_Type.ENUM = 14; // bootstrap-inject google.protobuf.FieldDescriptorProto.Label.LABEL_REPEATED: const $name: FieldDescriptorProto_Label.$localName = $number; const LABEL_REPEATED: FieldDescriptorProto_Label.REPEATED = 3; -// bootstrap-inject google.protobuf.FieldDescriptorProto.Label.LABEL_OPTIONAL: const $name: FieldDescriptorProto_Label.$localName = $number; -const LABEL_OPTIONAL: FieldDescriptorProto_Label.OPTIONAL = 1; // bootstrap-inject google.protobuf.FieldDescriptorProto.Label.LABEL_REQUIRED: const $name: FieldDescriptorProto_Label.$localName = $number; const LABEL_REQUIRED: FieldDescriptorProto_Label.REQUIRED = 2; @@ -909,7 +907,6 @@ function newField( } } field.presence = getFieldPresence(proto, oneof, parentOrFile); - field.optional = isOptionalField(field as DescField | DescExtension); return field as DescField | DescExtension; } @@ -1080,21 +1077,6 @@ function getFieldPresence( return resolveFeature("fieldPresence", { proto, parent }); } -/** - * Did the user use the `optional` keyword? - */ -function isOptionalField(field: DescField | DescExtension): boolean { - const edition = (field.kind == "extension" ? field.file : field.parent.file) - .edition; - if (edition == EDITION_PROTO2) { - return !field.oneof && field.proto.label == LABEL_OPTIONAL; - } - if (edition == EDITION_PROTO3) { - return field.proto.proto3Optional; - } - return false; -} - /** * Pack this repeated field? */ diff --git a/packages/protoc-gen-es/src/protoc-gen-es-plugin.ts b/packages/protoc-gen-es/src/protoc-gen-es-plugin.ts index aa5b0b5c2..b1786b0f8 100644 --- a/packages/protoc-gen-es/src/protoc-gen-es-plugin.ts +++ b/packages/protoc-gen-es/src/protoc-gen-es-plugin.ts @@ -27,12 +27,7 @@ import type { Printable, Target, } from "@bufbuild/protoplugin/ecmascript"; -import { - arrayLiteral, - fieldUsesPrototype, - functionCall, - getFieldTypeInfo, -} from "./util"; +import { arrayLiteral, functionCall, fieldTypeScriptType } from "./util"; import { version } from "../package.json"; export const protocGenEs = createEcmaScriptPlugin({ @@ -92,7 +87,7 @@ function generateTs(schema: Schema) { const { GenDescExtension, extDesc } = f.runtime.codegen; const name = f.importDesc(desc).name; const E = f.importShape(desc.extendee); - const V = getFieldTypeInfo(desc).typing; + const V = fieldTypeScriptType(desc).typing; const call = functionCall(extDesc, [f.importDesc(file), ...pathInFileDesc(desc)]); f.print(f.jsDoc(desc)); f.print(f.exportDecl("const", name), ": ", GenDescExtension, "<", E, ", ", V, ">", " = ", pure); @@ -219,7 +214,7 @@ function generateDts(schema: Schema) { const { GenDescExtension } = f.runtime.codegen; const name = f.importDesc(desc).name; const E = f.importShape(desc.extendee); - const V = getFieldTypeInfo(desc).typing; + const V = fieldTypeScriptType(desc).typing; f.print(f.jsDoc(desc)); f.print(f.exportDecl("declare const", name), ": ", GenDescExtension, "<", E, ", ", V, ">;"); f.print(); @@ -310,7 +305,7 @@ function generateMessageShape(f: GeneratedFile, message: DescMessage, target: Ex f.print(` } | {`); } f.print(f.jsDoc(field, " ")); - const { typing } = getFieldTypeInfo(field); + const { typing } = fieldTypeScriptType(field); f.print(` value: `, typing, `;`); f.print(` case: "`, localName(field), `";`); } @@ -318,11 +313,11 @@ function generateMessageShape(f: GeneratedFile, message: DescMessage, target: Ex break; default: { f.print(f.jsDoc(member, " ")); - const {typing, optional} = getFieldTypeInfo(member); - if (fieldUsesPrototype(member) || !optional) { - f.print(" ", localName(member), ": ", typing, ";"); - } else { + const { typing, optional } = fieldTypeScriptType(member); + if (optional) { f.print(" ", localName(member), "?: ", typing, ";"); + } else { + f.print(" ", localName(member), ": ", typing, ";"); } break; } diff --git a/packages/protoc-gen-es/src/util.ts b/packages/protoc-gen-es/src/util.ts index 8affcaaa9..cc835e492 100644 --- a/packages/protoc-gen-es/src/util.ts +++ b/packages/protoc-gen-es/src/util.ts @@ -18,44 +18,19 @@ import { ScalarType, scalarTypeScriptType, } from "@bufbuild/protobuf/reflect"; -import { Edition, isWrapperDesc } from "@bufbuild/protobuf/wkt"; +import { isWrapperDesc } from "@bufbuild/protobuf/wkt"; import type { Printable } from "@bufbuild/protoplugin/ecmascript"; -/** - * Tells whether a field uses the prototype chain for field presence. - * Behavior must match with the counterpart in @bufbuild/protobuf. - */ -export function fieldUsesPrototype(field: DescField): field is DescField & { - fieldKind: "scalar" | "enum"; - oneof: undefined; - repeated: false; -} { - if (field.parent.file.edition != Edition.EDITION_PROTO2) { - return false; - } - if (field.fieldKind != "scalar" && field.fieldKind != "enum") { - return false; - } - if (field.oneof) { - return false; - } - // proto2 singular scalar and enum fields use an initial value on the prototype chain - return true; -} - -export function getFieldTypeInfo(field: DescField | DescExtension): { +export function fieldTypeScriptType(field: DescField | DescExtension): { typing: Printable; optional: boolean; - typingInferrableFromZeroValue: boolean; } { const typing: Printable = []; - let typingInferrableFromZeroValue: boolean; let optional = false; switch (field.fieldKind) { case "scalar": typing.push(scalarTypeScriptType(field.scalar, field.longType)); - optional = field.optional; - typingInferrableFromZeroValue = true; + optional = field.proto.proto3Optional; break; case "message": { if (!field.oneof && isWrapperDesc(field.message)) { @@ -68,7 +43,6 @@ export function getFieldTypeInfo(field: DescField | DescExtension): { }); } optional = true; - typingInferrableFromZeroValue = true; break; } case "enum": @@ -76,12 +50,10 @@ export function getFieldTypeInfo(field: DescField | DescExtension): { kind: "es_shape_ref", desc: field.enum, }); - optional = field.optional; - typingInferrableFromZeroValue = true; + optional = field.proto.proto3Optional; break; case "list": optional = false; - typingInferrableFromZeroValue = false; switch (field.listKind) { case "enum": typing.push( @@ -140,12 +112,11 @@ export function getFieldTypeInfo(field: DescField | DescExtension): { break; } typing.push("{ [key: ", keyType, "]: ", valueType, " }"); - typingInferrableFromZeroValue = false; optional = false; break; } } - return { typing, optional, typingInferrableFromZeroValue }; + return { typing, optional }; } export function functionCall( diff --git a/packages/protoplugin-test/src/source-code-info.test.ts b/packages/protoplugin-test/src/source-code-info.test.ts index 1a2bc1bd8..368092b7f 100644 --- a/packages/protoplugin-test/src/source-code-info.test.ts +++ b/packages/protoplugin-test/src/source-code-info.test.ts @@ -127,7 +127,7 @@ describe("getComments()", () => { }); describe("getDeclarationString()", () => { - test("for field built-in option", async () => { + test("for field with json_name option", async () => { const field = await compileField(` syntax="proto3"; message M { @@ -138,7 +138,18 @@ describe("getDeclarationString()", () => { 'string scalar_field = 1 [json_name = "scalarFieldJsonName"]', ); }); - test("for field with labels", async () => { + test("for field with jstype option", async () => { + const field = await compileField(` + syntax="proto3"; + message M { + string scalar_field = 1 [jstype = JS_NORMAL]; + } + `); + expect(getDeclarationString(field)).toBe( + "string scalar_field = 1 [jstype = JS_NORMAL]", + ); + }); + test("for repeated field", async () => { const field = await compileField(` syntax="proto3"; message M { @@ -149,6 +160,28 @@ describe("getDeclarationString()", () => { "repeated double double_field = 1", ); }); + test("for proto2 optional field", async () => { + const field = await compileField(` + syntax="proto3"; + message M { + optional double double_field = 1; + } + `); + expect(getDeclarationString(field)).toBe( + "optional double double_field = 1", + ); + }); + test("for proto2 required field", async () => { + const field = await compileField(` + syntax="proto2"; + message M { + required double double_field = 1; + } + `); + expect(getDeclarationString(field)).toBe( + "required double double_field = 1", + ); + }); test("for map field", async () => { const field = await compileField(` syntax="proto3"; diff --git a/packages/protoplugin/src/source-code-info.ts b/packages/protoplugin/src/source-code-info.ts index cdbae71e9..61281096e 100644 --- a/packages/protoplugin/src/source-code-info.ts +++ b/packages/protoplugin/src/source-code-info.ts @@ -154,7 +154,6 @@ export function getDeclarationString( } return str; } - const file = desc.kind === "extension" ? desc.file : desc.parent.file; const parts: string[] = []; function typeName(f: DescField | DescExtension) { if (f.message) { @@ -169,14 +168,10 @@ export function getDeclarationString( case "scalar": case "enum": case "message": - if ( - file.edition === Edition.EDITION_PROTO2 && - isFieldSet(FieldDescriptorProtoDesc, desc.proto, "label") && - desc.proto.label == FieldDescriptorProto_Label.REQUIRED - ) { + if (fieldHasRequiredKeyword(desc)) { parts.push("required"); } - if (desc.optional) { + if (fieldHasOptionalKeyword(desc)) { parts.push("optional"); } parts.push(typeName(desc)); @@ -231,6 +226,37 @@ export function getDeclarationString( return parts.join(" "); } +/** + * Whether this field was declared with `required` in the protobuf source. + */ +function fieldHasRequiredKeyword(field: DescField | DescExtension): boolean { + const edition = (field.kind == "extension" ? field.file : field.parent.file) + .edition; + return ( + edition == Edition.EDITION_PROTO2 && + field.proto.label == FieldDescriptorProto_Label.REQUIRED + ); +} + +/** + * Whether this field was declared with `optional` in the protobuf source. + * Note that message fields are always optional. It is impossible to determine + * whether the keyword was used. + */ +function fieldHasOptionalKeyword(field: DescField | DescExtension): boolean { + const edition = (field.kind == "extension" ? field.file : field.parent.file) + .edition; + if (edition == Edition.EDITION_PROTO2) { + return ( + !field.oneof && field.proto.label == FieldDescriptorProto_Label.OPTIONAL + ); + } + if (edition == Edition.EDITION_PROTO3) { + return field.proto.proto3Optional; + } + return false; +} + /** * Find comments. */