Added typescript support and hello world test #45
Conversation
jayconrod
left a comment
jayconrod
left a comment
There was a problem hiding this comment.
LGTM, but I don't know much about TypeScript. Sara, Will, or Louis would be able to give a better review.
| @@ -0,0 +1,7 @@ | |||
| { | |||
There was a problem hiding this comment.
I know nothing about typescript: do we need to track external dependencies like this though? For other languages, you can do most things through BUILD or WORKSPACE files, so I'm surprised we need these.
There was a problem hiding this comment.
According to the documentation, we need those files as bazel builds entire projects. I have checked it works as it translated the code into a .js file in bazel-bin and when you execute it show hello world. I think we will improve this an all other examples for a most robust testing. This json file and more will increase in its dependency tree. Anyways, I will wait today, maybe we will have more comments from Will and Sara.
Just for your information, these are the rules I am using: https://github.com/aspect-build/rules_ts
| @@ -0,0 +1,4 @@ | |||
| export const hello: String = "Hello "; | |||
| export const world: String = "World " | |||
There was a problem hiding this comment.
Nit: semicolons after each line (ideally this would pass our linter tests which are not configured for this package)
| "compilerOptions": { | ||
| "declaration": true, | ||
| "baseUrl": ".", | ||
| "types": ["node"] |
There was a problem hiding this comment.
Do you need this type? We generally aren't transpiling for Node.js (for the browser instead)
There was a problem hiding this comment.
We don't really have a requirement here. I just added typescript support with rules_ts and to execute a simple example. I think, however, that line 5 is not necessary. I will remove it.
About
Adding Typescript support and hello world test.
Run with
bazel test //typescript:typesctript_testProgress #36