[TS] Support vectors, numerics, and strings with defaults#8896
Conversation
788c313 to
0ebb35d
Compare
|
Will likely have a merge conflict with #8861 |
|
@abhay-agarwal this also looks nice, so if we can get #8861 in then feel free to also update this and I'll try to get it in too. |
0ebb35d to
ec8f6a4
Compare
|
Not sure if I understand the codebase well enough, but running the test suite does generate a lot of changed test files. Nevertheless, the test suite does seem to pass locally. @bjornharrtell can you approve the test suite pass? |
|
@abhay-agarwal looks like you have a merge conflict - do you mind updating this? |
|
Yup let me take a look tonight! |
ec8f6a4 to
2641c0c
Compare
|
@jtdavis777 done! |
|
@abhay-agarwal unfortunately still some test failures to figure out. |
9c62cc0 to
649d26b
Compare
1e6624f to
cfee84e
Compare
|
@bjornharrtell please approve the tests again, I had some rebase issues. |
cfee84e to
ef333f8
Compare
|
hmm, okay I think it should work now? |
|
@abhay-agarwal still a few generated code test failures. |
…ts are supported Fixing issues with generated ts/js
ef333f8 to
76c34b2
Compare
|
Okay, try now. Sorry, I'm still a bit confused by how the test suite works! |
bjornharrtell
left a comment
There was a problem hiding this comment.
LGTM and a great contribution, thanks @abhay-agarwal for your efforts and patience.
This PR enables vector, numeric, and string types in typescript to have default values. The method signature for accessing a string now depends on whether the property is either required, or has a default value. In either case, the signature becomes:
instead of
For string types, typescript allows you to inline a global/constant, so we just return that if there is no value in the buffer. For vectors, we return a new vector, (I.e.
new Float32Array(), etc.), since there is no concept of an "inner" default value for vector types.One significant finding was that the parser parses optional string type properties with a value of
"0", meaning that if a user were to create a string with a default value of "0", then it would parse as nullable. I didn't want to disturb the codebase too much, but changing this would seemingly require some new logic in the parser to handle zero vs null values.Closes #8888