Skip to content
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 | 127,064 b | 65,515 b | 15,970 b |
| protobuf-es | 127,054 b | 65,501 b | 15,983 b |
| protobuf-javascript | 394,384 b | 288,654 b | 45,122 b |
6 changes: 3 additions & 3 deletions packages/protobuf-test/src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ export async function compileFileDescriptorSet(
return fromBinary(FileDescriptorSetDesc, bytes);
}

export async function compileFile(proto: string) {
export async function compileFile(proto: string, name = "input.proto") {
upstreamProtobuf = upstreamProtobuf ?? new UpstreamProtobuf();
const bytes = await upstreamProtobuf.compileToDescriptorSet(
{
"input.proto": proto,
[name]: proto,
},
{
includeImports: true,
Expand All @@ -49,7 +49,7 @@ export async function compileFile(proto: string) {
);
const fds = fromBinary(FileDescriptorSetDesc, bytes);
const reg = createFileRegistry(fds);
const file = reg.getFile("input.proto");
const file = reg.getFile(name);
assert(file);
return file;
}
Expand Down
41 changes: 20 additions & 21 deletions packages/protobuf-test/src/reflect/registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {
import {
compileEnum,
compileField,
compileFile,
compileFileDescriptorSet,
compileMessage,
compileMethod,
Expand Down Expand Up @@ -502,31 +503,16 @@ describe("createFileRegistry()", function () {

describe("DescFile", () => {
test("proto2 syntax", async () => {
const fileDescriptorSet = await compileFileDescriptorSet({
"a.proto": `syntax="proto2";`,
});
const reg = createFileRegistry(fileDescriptorSet);
const descFile = reg.getFile("a.proto");
expect(descFile).toBeDefined();
expect(descFile?.edition).toBe(Edition.EDITION_PROTO2);
const file = await compileFile(`syntax="proto2";`);
expect(file.edition).toBe(Edition.EDITION_PROTO2);
});
test("proto3 syntax", async () => {
const fileDescriptorSet = await compileFileDescriptorSet({
"a.proto": `syntax="proto3";`,
});
const reg = createFileRegistry(fileDescriptorSet);
const descFile = reg.getFile("a.proto");
expect(descFile).toBeDefined();
expect(descFile?.edition).toBe(Edition.EDITION_PROTO3);
const file = await compileFile(`syntax="proto3";`);
expect(file.edition).toBe(Edition.EDITION_PROTO3);
});
test("edition 2023", async () => {
const fileDescriptorSet = await compileFileDescriptorSet({
"a.proto": `edition = "2023";`,
});
const reg = createFileRegistry(fileDescriptorSet);
const descFile = reg.getFile("a.proto");
expect(descFile).toBeDefined();
expect(descFile?.edition).toBe(Edition.EDITION_2023);
const file = await compileFile(`edition = "2023";`);
expect(file.edition).toBe(Edition.EDITION_2023);
});
test("dependencies", async () => {
const fileDescriptorSet = await compileFileDescriptorSet({
Expand All @@ -542,6 +528,19 @@ describe("DescFile", () => {
expect(a?.dependencies.length).toBe(2);
expect(a?.dependencies.map((f) => f.name)).toStrictEqual(["b", "c"]);
});
describe("name", () => {
test("is proto file name without .proto suffix", async () => {
const file = await compileFile(`syntax="proto3";`, "foo/bar/baz.proto");
expect(file.name).toBe("foo/bar/baz");
});
test("strips only last .proto", async () => {
const file = await compileFile(
`syntax="proto3";`,
"foo.proto/baz.proto.proto",
);
expect(file.name).toBe("foo.proto/baz.proto");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without the change above, name would be "foo/baz.proto.proto".

});
});
});

describe("DescEnum", () => {
Expand Down
6 changes: 3 additions & 3 deletions packages/protobuf/src/reflect/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -443,15 +443,15 @@ function addFile(proto: FileDescriptorProto, reg: MutableRegistry): void {
proto,
deprecated: proto.options?.deprecated ?? false,
edition: getFileEdition(proto),
name: proto.name.replace(/\.proto/, ""),
name: proto.name.replace(/\.proto$/, ""),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, this removed the first sub-string.

dependencies: findFileDependencies(proto, reg),
enums: [],
messages: [],
extensions: [],
services: [],
toString(): string {
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions -- we asserted above
return `file ${this.proto.name}`;
return `file ${proto.name}`;
},
};
const mapEntriesStore = new Map<string, DescMessage>();
Expand Down Expand Up @@ -829,7 +829,7 @@ function newField(
keyField.scalar != ScalarType.FLOAT &&
keyField.scalar != ScalarType.DOUBLE,
);
const valueField = mapEntry.fields.find((f) => f.proto.number === 2);
const valueField = mapEntry.fields.find((f) => f.number === 2);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use DescField.number here, no need to go through FieldDescriptorProto.

assert(valueField);
assert(valueField.fieldKind != "list" && valueField.fieldKind != "map");
field.mapKey = keyField.scalar;
Expand Down