Add --ts-undefined-for-optionals command line option #8861
Conversation
# Details - Fixes google#7656 - Added a new `--ts-undefined-for-optionals` command line option for `flatc`. - If enabled, generated TypeScript code uses `undefined` for optional fields rather than `null`.
github-actions
bot
added
python
c++
javascript
typescript
codegen
|
Note for myself: run |
|
This is ready. |
bjornharrtell
left a comment
bjornharrtell
left a comment
There was a problem hiding this comment.
Some perhaps unnecessary source format changes but otherwise looks like solid work
Could you detail which unnecessary source format changes you mean? I've run |
|
@aardappel I think it's is good to go but can't merge due to the test failure for swift which I would think is unrelated to this PR. |
The error is due to a HTTP download timeout. Probably fixable by re-running the GH action. |
It seems a network issue. Maybe re-running the job could work? Note: I cannot rerun it. |
|
@aardappel I was able to trigger re-run and it seems to work now.. but can't because of this: 
No idea what that means. |
|
afaik @dbaileychess has locked the repo for some to changes how things are set up.. we'll have to wait till he is done before further PRs can get merged. |
|
I'm trying to minimize changes while I am working on merging the last 6 months of changes back into Google. Hopefully I can finish tonight or tomorrow, can this wait? |
src/idl_gen_ts.cpp
Outdated
| "throw", "true", "try", "typeof", "var", | ||
| "void", "while", "with", "as", "implements", | ||
| "interface", "let", "package", "private", "protected", | ||
| "public", "static", "yield", "undefined"}; |
There was a problem hiding this comment.
can you add a trailing comment to it formats this nicely?
src/idl_gen_ts.cpp
Outdated
|
|
||
| // Value for non present optional fields. It's "null" by default and | ||
| // "undefined" if parser.ts_undefined_for_optionals flag is set. | ||
| std::string null_ = "null"; |
There was a problem hiding this comment.
can you name this better?
src/idl_gen_ts.cpp
Outdated
| std::string GenDefaultValue(const FieldDef& field, import_set& imports) { | ||
| if (field.IsScalarOptional()) { | ||
| return "null"; | ||
| return null_; |
There was a problem hiding this comment.
are all "null" supposed to be replaced? Or just for optional stuff.
There was a problem hiding this comment.
The purpose of the new flag is to produce TS code with value undefined rather than null when the key has no value. This includes the default value.
For example, if the FBS schema is like this:
table ScalarStuff {
just_i8: int8;
maybe_i8: int8 = null;
}
Then, if --ts-undefined-for-optionals is set and no explicit value is given to maybe_i8, its default value in TS will be undefined rather than null. This behavior is consistent with the intended behavior.
Sure, no hurries. BTW I've addressed feedback already. |
jtdavis777
added
the
ready-for-merge
6 participants
Details
undefined(instead ofnull) if the value is not set #7656--ts-undefined-for-optionalscommand line option forflatc.undefinedfor optional fields rather thannull.