Typescript support (alternative) #4244
Conversation
fixed issues with 'this' not having intellisense, causing issue with generic in test_generated.ts line 220, tsc is dropping the namespace 'flatbuffers' off of 'flatbuffers.Table' and recognizing it as 2 different types
Caused issues all all around including with webpack/browserify
had initialized local variable with same name c\p bug
Make sure node doesn't grap the ts files
Had to make the __union method generic so that calling method would know return value
fixed cast when type is boolean
Needs implementation check, just repeated writeIntXX methods
…into typescript-gen
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
|
(Turns out I had already signed the Google CLA - I guess all that remains is asking @sampaioletti if he's okay with my PR being based on his?) |
|
Of course, I'm all for making this work for the project. My original work was a late night hack that I just put together so I could use it quickly in a project I needed it for, sort of a proof of concept. I have been meaning to get back to it but have not had the time. I'll try and contribute where I can by reviewing and testing the updated code. After using TS with FB it really works quite well and I think is a perfect match for the project. |
|
@fasterthanlime : Thanks for your help in getting this functionality in, regardless of whether this version or the one from @krojew. Besides correctness, efficiency, and an appropriate API for the language, I share your concern for keeping the code simple, i.e. reducing the amount of if-thens needed to accomplish this as much as possible, by factoring them into functions, or even better, constant lookups. |
|
The diff for this one seems a bit harder to read since there's a lot of spacing (indentation?) changes. Maybe that can be kept seperate. |
|
Also, like the other PR, generated code should be included in the PR. |
|
Closing this PR since it was absorbed into #4232 (the fork+branch remains for any future reference) |
4 participants
I've started cleaning up #4065 as TypeScript being supported by upstream flatbuffers is something I'd really like, but then I noticed someone was also doing that, in #4232.
I believe #4232 should eventually be the one that gets merged, mostly because it has some import / namespacing logic that neither mine nor the original have, but some of the changes I made are, I believe, good examples of how to fit better into the rest of the flatbuffers codebase / are more in line with the review comments made by @gwvo in #4065.
In particular:
new Generator(true)andnew Generator(false)isn't obvious to the reader - enums are a better choice, even when there's only two options. TheIDL::Options::k{language}enum does just fine, and is in fact already passed to JsGenerator's constructoridl_gen_general's convention and started making an array of structs containing characteristics and code snippets to be used when generating either JavaScript or TypeScript code.JsGenerateandJsMakeRulefunctions, and removesGetNameSpace, whose job was already adequately done byFullNamespaceI believe this PR looks more like what the flatbuffers maintainers would've written if they had written TypeScript support themselves, that it fits better in the codebase, however, I haven't finished cleaning it, since it lacks other things in #4232, such as:
--no-fb-importoption to disable that behaviorI'm submitting the PR so that I can sign the CLA and release my initial work to the flatbuffers project, in the hopes that @krojew can leverage it to polish #4232 to the point where it gets merged and everybody wins!
TL;DR: want typescript, found old PR, researched conventions, started making changes, noticed more feature-rich PR, this PR is destined to serve as a reference (possibly of what not to do, it hasn't been reviewed yet) and to be closed when #4232 is merged.