Adding Typescript support #188
Conversation
|
|
||
| /// Represents typescript template constants | ||
| pub(crate) mod typescript { | ||
| pub(crate) const INTERFACE: &str = include_template!("typescript/interface.templ"); | ||
| pub(crate) const SCALAR: &str = include_template!("typescript/scalar.templ"); | ||
| pub(crate) const SEQUENCE: &str = include_template!("typescript/sequence.templ"); | ||
| pub(crate) const ENUM: &str = include_template!("typescript/enum.templ"); | ||
| pub(crate) const UTIL_MACROS: &str = include_template!("typescript/util_macros.templ"); | ||
| pub(crate) const NESTED_TYPE: &str = include_template!("typescript/nested_type.templ"); | ||
| pub(crate) const IMPORT: &str = include_template!("typescript/import.templ"); | ||
| } |
There was a problem hiding this comment.
Accidental add; will remove it
There was a problem hiding this comment.
Updated it with the rest of the code, please take a look
Prashant18
force-pushed
the
feature/typescript
branch
7 times, most recently
from
7aef30c to
764cda2
Compare
Prashant18
force-pushed
the
feature/typescript
branch
from
764cda2 to
6226224
Compare
There was a problem hiding this comment.
Thank you Prashant for this contribution! I would appreciate having some description in this PR for the intention for this pull request.
I have done one pass to look at this PR but a high-level feedback is to just follow how other languages have used Language trait and make a similar change for typescript.
Also, a suggestion for this PR would be to make just add the basic support provided similar to other languages(i.e. Java and Rust).
For example, we do not have annotations supported with code generation at this point, so adding that implementation with a separate PR would be better. Same goes for the symbol, timestamp, decimal support. While having these features would definitely be nice, it would be better to review those new feature if they are in separate PR. And also makes the current PR shorter to review.
| @@ -179,6 +190,32 @@ impl TryFrom<&Value> for FullyQualifiedTypeReference { | |||
| } | |||
| } | |||
|
|
|||
| impl Display for FullyQualifiedTypeReference { | |||
There was a problem hiding this comment.
Using Display here might not be a good idea, since this display implementation is particularly related to how TypeScript prints namespaces instead we already have a method called string_representation which uses L::namespace_separator() to print namespace and is also then used by tera to get the string representation of fully qualified namespace.
You can just add namespace separator for typescript in Language implementation of TypescriptLanguage. This way its consistent with other languages and easy to expand more languages in future.
| @@ -332,6 +369,12 @@ pub struct Scalar { | |||
| source: IslType, | |||
| } | |||
|
|
|||
| impl Scalar { | |||
| pub fn base_type(&self) -> &FullyQualifiedTypeReference { | |||
There was a problem hiding this comment.
All the properties of these AbstractDataTypes are available at the template level using the serialized form of these types. i.e. on the template side you can just use scalar_model.base_type. That way you don't really need to include these methods unless you are using it in the generator in which case it actually makes it difficult to have the separation between the generator and templates.
| * {{ model.code_gen_type.doc_comment }} | ||
| */ | ||
| {% endif %} | ||
| {% if model.annotations %} |
There was a problem hiding this comment.
The preview version of code generation does not support annotations at this point. I think it would be best if we can add this support separately so that its consistent with other languages(i.e. Java and Rust).
|
|
||
| {{ macros::type_guard(model=model) }} | ||
|
|
||
| export class {{ model.name }}Impl implements {{ model.name }} { |
There was a problem hiding this comment.
Similar to other templates maybe you can just create a macro for the class in interface.templ and use that macro in this template so that you dont have to write the same template again.
|
|
||
|
|
||
|
|
||
| export enum EnumType {FOO_BAR_BAZ = "FooBarBaz", BAR = "bar", BAZ = "baz", FOO = "foo" |
There was a problem hiding this comment.
I understand you submitting these generated files for making it easy for the reviewer to look at it but we generally don't merge the generated code int the repository. You can probably just create some gists and add it to the PR description.
| @@ -0,0 +1,25 @@ | |||
| import * as ion from 'ion-js'; | |||
There was a problem hiding this comment.
File naming here is in snake case and probably utils might be a better name to use here.
| @@ -0,0 +1 @@ | |||
|
|
|||
There was a problem hiding this comment.
Not sure if this was intended but all the complex_types.* files are empty.
| "prebuild": "npm run generate", | ||
| "build": "tsc", | ||
| "test": "jest", | ||
| "generate": "ion-cli generate -l typescript -d ../../schema -o ./src/models", |
There was a problem hiding this comment.
This should be ion instead of ion-cli since thats the executable name used in the CLI tool.
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.