search

creditkarma / thrift-typescript

Generate TypeScript from Thrift IDL files
Apache License 2.0
155 stars 32 forks source link

Replace import equals declarations with const and type alias statements #155

Closed hayes closed 5 years ago

hayes commented 5 years ago

Currently the typedefs file uses a lot of import-equals statements to re-export aliased types and variables.

Unfortunately, babel does not support import-equals syntax. Import equals is part of typescript modules which are being replaced with es6 modules.

This PR replaces importEquals statements in the thrift-server output with consts and type aliases. I did not update the apache output because it does not have an existing clean way to separate from classes an enums. Without that separation there is no way to determine if the statement needs to be a type alias or a const declaration.

kevin-greene-ck commented 5 years ago

@hayes Thanks for taking this on. One regression I'm getting is that for a typedef where the definitionType is an enum you reexport as a const which works fine when using the enum as a value, but you can also use an enum as a type which doesn't work now. I think you can reexport enums twice, once as a const and once as a type and that seems to work in my brief testing.

kevin-greene-ck commented 5 years ago

It would be easier to properly test this library if it was in the thrift-server mono repo. Maybe...

hayes commented 5 years ago

@kevin-greene-ck good catch! added some tests to cover that case, and updated enum export to export both type and const. Looks like that works for using as both a type and value when imported!

kevin-greene-ck commented 5 years ago

@hayes Awesome, quick turn around. I'll spend a little time testing but the code looks great and I'll hopefully merge and release today.

hayes commented 5 years ago

thanks!

hayes commented 5 years ago

Going to push another change. I discovered that the import order matters with the way that babel compiles the the const exports for some use cases.

hayes commented 5 years ago

@kevin-greene-ck I just pushed a small change to move the typedef imports to the bottom of the index.ts imports.

This was because the way that babel compiles imports causes some exports from typedefs to be undefined. This is because of the circular dependency created by every non-index file importing index. index initially has an empty export object, and this is used to set the const values in the typedef file. By importing typedefs last, we avoid these issues.