Skip to content

feat: added useNullAsOptional option #1017

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

Merged
merged 3 commits into from
Mar 30, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
52 changes: 52 additions & 0 deletions README.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,58 @@ Generated code will be placed in the Gradle build directory.

- With `--ts_proto_opt=comments=false`, comments won't be copied from the proto files to the generated code.

- With `--ts_proto_opt=useNullAsOptional=true`, `undefined` values will be converted to `null`, and if you use `optional` label in your `.proto` file, the field implicitly will have `undefined` type as well. for example:

```protobuf
message ProfileInfo {
int32 id = 1;
string bio = 2;
string phone = 3;
}

message Department {
int32 id = 1;
string name = 2;
}

message User {
int32 id = 1;
string username = 2;
/*
ProfileInfo will be optional in typescript, implicitly the type will be ProfileInfo | null | undefined
this is needed in cases where you don't wanna provide any value for the profile.
*/
optional ProfileInfo profile = 3;

/*
Department only accepts a Department type or null, so this means you have to pass it null if there is no value available.
*/
Department department = 4;
}
```

the generated interfaces will be:

```typescript
export interface ProfileInfo {
id: number;
bio: string;
phone: string;
}

export interface Department {
id: number;
name: string;
}

export interface User {
id: number;
username: string;
profile?: ProfileInfo | null; // check this one
department: Department | null; // check this one
}
```

### NestJS Support

We have a great way of working together with [nestjs](https://docs.nestjs.com/microservices/grpc). `ts-proto` generates `interfaces` and `decorators` for you controller, client. For more information see the [nestjs readme](NESTJS.markdown).
Expand Down
1 change: 1 addition & 0 deletions integration/use-null-as-optional/parameters.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
useNullAsOptional=true
Binary file not shown.
23 changes: 23 additions & 0 deletions integration/use-null-as-optional/use-null-as-optional.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
syntax = "proto3";

package useNullAsOptional;

message ProfileInfo {
int32 id = 1;
string bio = 2;
string phone = 3;
}

message User {
int32 id = 1;
string username = 2;
optional ProfileInfo profile = 3;
}

message UserById {
int32 id = 1;
}

service HeroService {
rpc FindOneHero (UserById) returns (User) {}
}
297 changes: 297 additions & 0 deletions integration/use-null-as-optional/use-null-as-optional.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,297 @@
/* eslint-disable */
import * as _m0 from "protobufjs/minimal";

export const protobufPackage = "useNullAsOptional";

export interface ProfileInfo {
id: number;
bio: string;
phone: string;
}

export interface User {
id: number;
username: string;
profile?: ProfileInfo | null;
Copy link
Owner

Choose a reason for hiding this comment

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

Is this ProfileInfo | null type actually accurate?

Afaict the .decode and .fromJSON methods are not ever setting profile to null ... so if it's never null, why are we adding it to the type?

Fwiw I was expecting this to be like profile: ProfileInfo | null and then we'd like specifically insert null as the "empty" value at runtime, in the decode/fromJSON methods.

Or is the point that we can accept a User.profile: null object that was created from an ORM, with | null in the type, and just not write it on the wire / still treat it as undefined?

Will the ORM be fine with accepting undefined if we decode something off the wire it and give it profile: undefined instead of profile: null?

Copy link
Contributor Author

@TheMichio TheMichio Mar 21, 2024

Choose a reason for hiding this comment

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

I had the same doubt and confusion too 🥲
my goal is to give the user a chance to have better alignment with tools like ORMs, let me give an example with drizzle ORM:
let's say we have two tables user and profile, we can have two scenarios in which we might include profile relation in our select query or not, the resuling type for the query would be something like:

  1. with profile relation included:
    let user = await this.db.query.users.findFirst({
      where: eq(users.id, id),
      with: {
        profile: true,
      },
    });

the type for user would be:

let user: {
    id: number;
    email: string;
    username: string;
    password: string;
    profile: {
        id: number;
        name: string | null;
        bio: string | null;
        lastName: string | null;
    } | null;
} | undefined
  1. without profile relation included:
    let user = await this.db.query.users.findFirst({
      where: eq(users.id, id),
    });

the type for user would be:

let user: {
    id: number;
    email: string;
    username: string;
    password: string;
} | undefined

so the explicit null type on the profile is needed for the situation where we don't have any profile relation for user. and the optional part on profile? is needed for the situation where we are NOT querying the relation ( we don't care about it ) and it's not even there.
the ideal generated interface by ts-proto IMHO would be:

export interface Profile {
  id: number;
  name?: string | null;
  lastName?: string | null;
  bio?: string | null;
}

export interface User {
  id: number;
  email: string;
  username: string;
  profile?: Profile | null;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for the exactOptionalPropertyTypes on tsc type checking we can make the type ProfileInfo | null | undefined, to make the typechecker happy as well.
please let me know what you think, thanks thanks 🙏
this is the best I could come up with to handle the undefined, null, optional situation a little bit 😅

Copy link
Owner

Choose a reason for hiding this comment

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

Hi @TheMichio ; I guess my confusion with the two findFirst examples is that I'm not sure it's a good idea to try and have a single protobuf message type be used for both of those queries simulatenously.

I.e. it sounds like you're trying to make a User "domain model type" / shared type that is going to be used at multiple points in your application, and probably returned from multiple wire calls? Like maybe 1 RPC call has GetJustUser that does the first findMany but another RPC call has GetUserWithProfile.

As you pointed out, these have fundamentally different types coming from the ORM, but I guess shouldn't they also have fundamentally different types when being put on the wire? I.e. it seems weird for the caller of GetJustUser to have a message type that says "you might have a profile" when they would never have a profile.

Dunno, honestly I'm going back/forth on accepting the PR; it's really great you got it to work, and all of the code technically looks great, but it adds some implementation complexity that I'm not 100% sure is best for the long-term.

...

Hm, I guess we have #869 open for awhile, and also it does look like things like profile: null` are the default, which I'd missed in the first review--okay, seems good enough for me then. Thank you!

}

export interface UserById {
id: number;
}

function createBaseProfileInfo(): ProfileInfo {
return { id: 0, bio: "", phone: "" };
}

export const ProfileInfo = {
encode(message: ProfileInfo, writer: _m0.Writer = _m0.Writer.create()): _m0.Writer {
if (message.id !== 0) {
writer.uint32(8).int32(message.id);
}
if (message.bio !== "") {
writer.uint32(18).string(message.bio);
}
if (message.phone !== "") {
writer.uint32(26).string(message.phone);
}
return writer;
},

decode(input: _m0.Reader | Uint8Array, length?: number): ProfileInfo {
const reader = input instanceof _m0.Reader ? input : _m0.Reader.create(input);
let end = length === undefined ? reader.len : reader.pos + length;
const message = createBaseProfileInfo();
while (reader.pos < end) {
const tag = reader.uint32();
switch (tag >>> 3) {
case 1:
if (tag !== 8) {
break;
}

message.id = reader.int32();
continue;
case 2:
if (tag !== 18) {
break;
}

message.bio = reader.string();
continue;
case 3:
if (tag !== 26) {
break;
}

message.phone = reader.string();
continue;
}
if ((tag & 7) === 4 || tag === 0) {
break;
}
reader.skipType(tag & 7);
}
return message;
},

fromJSON(object: any): ProfileInfo {
return {
id: isSet(object.id) ? globalThis.Number(object.id) : 0,
bio: isSet(object.bio) ? globalThis.String(object.bio) : "",
phone: isSet(object.phone) ? globalThis.String(object.phone) : "",
};
},

toJSON(message: ProfileInfo): unknown {
const obj: any = {};
if (message.id !== 0) {
obj.id = Math.round(message.id);
}
if (message.bio !== "") {
obj.bio = message.bio;
}
if (message.phone !== "") {
obj.phone = message.phone;
}
return obj;
},

create<I extends Exact<DeepPartial<ProfileInfo>, I>>(base?: I): ProfileInfo {
return ProfileInfo.fromPartial(base ?? ({} as any));
},
fromPartial<I extends Exact<DeepPartial<ProfileInfo>, I>>(object: I): ProfileInfo {
const message = createBaseProfileInfo();
message.id = object.id ?? 0;
message.bio = object.bio ?? "";
message.phone = object.phone ?? "";
return message;
},
};

function createBaseUser(): User {
return { id: 0, username: "", profile: null };
}

export const User = {
encode(message: User, writer: _m0.Writer = _m0.Writer.create()): _m0.Writer {
if (message.id !== 0) {
writer.uint32(8).int32(message.id);
}
if (message.username !== "") {
writer.uint32(18).string(message.username);
}
if (message.profile !== undefined && message.profile !== null) {
ProfileInfo.encode(message.profile, writer.uint32(26).fork()).ldelim();
}
return writer;
},

decode(input: _m0.Reader | Uint8Array, length?: number): User {
const reader = input instanceof _m0.Reader ? input : _m0.Reader.create(input);
let end = length === undefined ? reader.len : reader.pos + length;
const message = createBaseUser();
while (reader.pos < end) {
const tag = reader.uint32();
switch (tag >>> 3) {
case 1:
if (tag !== 8) {
break;
}

message.id = reader.int32();
continue;
case 2:
if (tag !== 18) {
break;
}

message.username = reader.string();
continue;
case 3:
if (tag !== 26) {
break;
}

message.profile = ProfileInfo.decode(reader, reader.uint32());
continue;
}
if ((tag & 7) === 4 || tag === 0) {
break;
}
reader.skipType(tag & 7);
}
return message;
},

fromJSON(object: any): User {
return {
id: isSet(object.id) ? globalThis.Number(object.id) : 0,
username: isSet(object.username) ? globalThis.String(object.username) : "",
profile: isSet(object.profile) ? ProfileInfo.fromJSON(object.profile) : null,
};
},

toJSON(message: User): unknown {
const obj: any = {};
if (message.id !== 0) {
obj.id = Math.round(message.id);
}
if (message.username !== "") {
obj.username = message.username;
}
if (message.profile !== undefined && message.profile !== null) {
obj.profile = ProfileInfo.toJSON(message.profile);
}
return obj;
},

create<I extends Exact<DeepPartial<User>, I>>(base?: I): User {
return User.fromPartial(base ?? ({} as any));
},
fromPartial<I extends Exact<DeepPartial<User>, I>>(object: I): User {
const message = createBaseUser();
message.id = object.id ?? 0;
message.username = object.username ?? "";
message.profile = (object.profile !== undefined && object.profile !== null)
? ProfileInfo.fromPartial(object.profile)
: undefined;
return message;
},
};

function createBaseUserById(): UserById {
return { id: 0 };
}

export const UserById = {
encode(message: UserById, writer: _m0.Writer = _m0.Writer.create()): _m0.Writer {
if (message.id !== 0) {
writer.uint32(8).int32(message.id);
}
return writer;
},

decode(input: _m0.Reader | Uint8Array, length?: number): UserById {
const reader = input instanceof _m0.Reader ? input : _m0.Reader.create(input);
let end = length === undefined ? reader.len : reader.pos + length;
const message = createBaseUserById();
while (reader.pos < end) {
const tag = reader.uint32();
switch (tag >>> 3) {
case 1:
if (tag !== 8) {
break;
}

message.id = reader.int32();
continue;
}
if ((tag & 7) === 4 || tag === 0) {
break;
}
reader.skipType(tag & 7);
}
return message;
},

fromJSON(object: any): UserById {
return { id: isSet(object.id) ? globalThis.Number(object.id) : 0 };
},

toJSON(message: UserById): unknown {
const obj: any = {};
if (message.id !== 0) {
obj.id = Math.round(message.id);
}
return obj;
},

create<I extends Exact<DeepPartial<UserById>, I>>(base?: I): UserById {
return UserById.fromPartial(base ?? ({} as any));
},
fromPartial<I extends Exact<DeepPartial<UserById>, I>>(object: I): UserById {
const message = createBaseUserById();
message.id = object.id ?? 0;
return message;
},
};

export interface HeroService {
FindOneHero(request: UserById): Promise<User>;
}

export const HeroServiceServiceName = "useNullAsOptional.HeroService";
export class HeroServiceClientImpl implements HeroService {
private readonly rpc: Rpc;
private readonly service: string;
constructor(rpc: Rpc, opts?: { service?: string }) {
this.service = opts?.service || HeroServiceServiceName;
this.rpc = rpc;
this.FindOneHero = this.FindOneHero.bind(this);
}
FindOneHero(request: UserById): Promise<User> {
const data = UserById.encode(request).finish();
const promise = this.rpc.request(this.service, "FindOneHero", data);
return promise.then((data) => User.decode(_m0.Reader.create(data)));
}
}

interface Rpc {
request(service: string, method: string, data: Uint8Array): Promise<Uint8Array>;
}

type Builtin = Date | Function | Uint8Array | string | number | boolean | undefined;

export type DeepPartial<T> = T extends Builtin ? T
: T extends globalThis.Array<infer U> ? globalThis.Array<DeepPartial<U>>
: T extends ReadonlyArray<infer U> ? ReadonlyArray<DeepPartial<U>>
: T extends {} ? { [K in keyof T]?: DeepPartial<T[K]> }
: Partial<T>;

type KeysOfUnion<T> = T extends T ? keyof T : never;
export type Exact<P, I extends P> = P extends Builtin ? P
: P & { [K in keyof P]: Exact<P[K], I[K]> } & { [K in Exclude<keyof I, KeysOfUnion<P>>]: never };

function isSet(value: any): boolean {
return value !== null && value !== undefined;
}
Loading