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

Presence clear methods #164

Closed
wants to merge 17 commits into from
Closed

Conversation

Santalov
Copy link

@Santalov Santalov commented Jul 29, 2022

This change only affects the users of the tsconfig option strictNullChecks, others can completely ignore it.

In typescript types of getters and setters match exactly. This is ok, cos when a field always has value (!canHaveNullValue), assigning undefined to it makes no sense: the getter of the field will always return some non-null value, so the code bellow will print 0.

let x = undefined;
msg.int64 = x; // msg.int is a proto3 int64 field
x = msg.int64;
console.log(x); // will print 0, might be confusing

The implication is that you cannot assign undefined to a field, that always returns non-null value (if you are using strictNullChecks:true). This is a problem when dealing with optional fields, so clear_* methods were introduced. The plugin generates them for all fields, that had has_* method generated. Those methods are described in the official doc https://github.com/protocolbuffers/protobuf/blob/main/docs/field_presence.md

Santalov added 17 commits July 29, 2022 11:20
Removed RecursivePartial<T>, instead introduced AsObjectPartial, which is used as fromObject method argument. Make getters, setters and AsObject typed according to canHaveNullValue predicate. Brought AsObjectPartial into line with constructor's declaration.
Change method access in RPC from property access to element access.
Add non-null assertion operators in places where an indexed access occurs.
return data && message when fromObject has no field assignments.
@thesayyn
Copy link
Owner

thesayyn commented Sep 1, 2023

Closing as there's been no activity on this.

@thesayyn thesayyn closed this Sep 1, 2023
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.

2 participants