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

[TS] Feature Request: Make optional field getters return undefined (instead of null) if the value is not set #7656

Open
jmillan opened this issue Nov 21, 2022 · 8 comments

Comments

@jmillan
Copy link
Contributor

jmillan commented Nov 21, 2022

Flatbuffers version:master
OS: Mac OS BigSur

If I wanted to create a flatbuffers table that represents the following TS type:

type Foo = {
 name?: string; 
}

which is the same as:

type Foo = {
 name: string | undefined; 
}

..., I would define the following table, which represents a type with an optional field name of type string.

table Foo {
  name:string;
}

When generating the TS code of Foo table, the obtained code is as follows:

export class Foo {
..
..
..
name():string|null
name(optionalEncoding:flatbuffers.Encoding):string|Uint8Array|null
name(optionalEncoding?:any):string|Uint8Array|null {
  const offset = this.bb!.__offset(this.bb_pos, 4);
  return offset ? this.bb!.__string(this.bb_pos + offset, optionalEncoding) : null;
}

The return type of name() method is string|null, whereas string|undefined would perfectly match the optional fields defined in TS as exposed above.

I'm finding myself manually converting types due to this. It would be great to have a flatc option to indicate that optional field getters should return undefined instead of null if a value is not set.

Is there any reason for not having this?
Would you be willing to accept a contribution for this feature request?

@jmillan
Copy link
Contributor Author

jmillan commented Nov 21, 2022

At the same time, if a table field is required I would expect it NOT to return null nor undefined, at least when generating the object API, in the unpack() method, but the field type in question.

@ibc
Copy link

ibc commented Apr 20, 2023

Hi, developers. Any update or thoughts about this? IMHO it makes lot of sense.

@github-actions
Copy link

This issue is stale because it has been open 6 months with no activity. Please comment or label not-stale, or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Oct 19, 2023
@ibc
Copy link

ibc commented Oct 19, 2023

Just keeping this alive.

@github-actions github-actions bot removed the stale label Oct 20, 2023
@CasperN
Copy link
Collaborator

CasperN commented Oct 29, 2023

Hi, not a TS person, but are you envisioning a flag that changes every accessor or adding a key to the schema? The global flag seems too coarse grained for sufficiently large projects. I think its plausible that depending on the type null may be preferred to undefined and vice versa.

Adding field or table level options seems like more work but ultimately a better solution, e.g.

table Foo {
  // field level option?
  foo: string (ts_getter_null_to_undefined);
}
// and also a table level option to save typing?
table Bar (ts_getter_null_to_undefined) {
}

@jmillan
Copy link
Contributor Author

jmillan commented Nov 1, 2023

I would say this should come as a flatc option, being a language specific matter.

If you use 'undefined' rather than 'null' in your project you would need to add such flag everywhere in the schema otherwise. Usually you use one of them and not mix them.

Copy link

github-actions bot commented May 1, 2024

This issue is stale because it has been open 6 months with no activity. Please comment or label not-stale, or this will be closed in 14 days.

@github-actions github-actions bot added the stale label May 1, 2024
@ibc
Copy link

ibc commented May 1, 2024

Ping

@github-actions github-actions bot removed the stale label Jul 25, 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

No branches or pull requests

3 participants