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: Introduce DescMessage.field #839

Merged
merged 1 commit into from
May 13, 2024
Merged

V2: Introduce DescMessage.field #839

merged 1 commit into from
May 13, 2024

Conversation

timostamm
Copy link
Member

In some situations, it's useful to be able to refer to a field by name.

DescMessage provides all fields in the property fields: DescField[] - that's convenient to loop through all fields, but it's awkward to pick a field by name.

This PR adds another property to DescMessage:

/**
 * All fields of this message by their "localName".
 */
readonly field: Record<string, DescField>;

In generated code, the property is a type-safe record. You get autocomplete for field names, and a typo is a compile-time error:

import { UserDesc } from "./gen/example_pb.js";

UserDesc.field.firstName; // DescField
UserDesc.field.firstNam; // type error

This PR also changes the signatures of isFieldSet and clearField to make use of this new feature. Instead of having to pass in the message descriptor, the message, and the field name, both functions only require two arguments now: The message, and the field:

import { create, clearField } from "@bufbuild/protobuf";
import { UserDesc } from "./gen/example_pb.js";

const user = create(UserDesc);
clearField(user, UserDesc.field.firstName);

@@ -543,6 +543,7 @@ function addFields(
const oneof = findOneof(proto, allOneofs);
const field = newField(proto, message, reg, oneof, mapEntries);
message.fields.push(field);
message.field[field.localName] = field;
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're eagerly populating the record - it's quite simple and cheap.

} from "@bufbuild/protobuf/wkt";

/**
* Get comments on the package element in the protobuf source.
*/
export function getPackageComments(desc: DescFile): DescComments {
return findComments(desc.proto.sourceCodeInfo, [
FieldNumber.FileDescriptorProto_Package,
FileDescriptorProtoDesc.field.package.number,
Copy link
Member Author

Choose a reason for hiding this comment

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

That's a nice practical example of where this feature can be useful. I imagine it will also be useful to construct FieldMasks.

Comment on lines +37 to +41
field: DescField,
): boolean {
const field = messageDesc.fields.find((f) => f.localName === fieldName);
if (field) {
return unsafeIsSet(message, field);
}
return false;
return (
field.parent.typeName == message.$typeName && unsafeIsSet(message, field)
);
Copy link
Member Author

Choose a reason for hiding this comment

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

Same as before, a foreign field is never set, and clearing a foreign field is a no-op.

@timostamm timostamm changed the title Introduce DescMessage.field V2: Introduce DescMessage.field May 13, 2024
@timostamm timostamm merged commit b81a141 into v2 May 13, 2024
8 checks passed
@timostamm timostamm deleted the tstamm/DescMessage.field branch May 13, 2024 13:56
@timostamm timostamm mentioned this pull request May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants