Basic TypeScript Support #84
Conversation
| @@ -10,6 +10,7 @@ class Parser { | |||
| this.registerExtension('js', './assets/JSAsset'); | |||
| this.registerExtension('jsx', './assets/JSAsset'); | |||
| this.registerExtension('es6', './assets/JSAsset'); | |||
| this.registerExtension('ts', './assets/TypeScriptAsset'); | |||
src/assets/TypeScriptAsset.js
Outdated
| let typescript = localRequire('typescript', this.name); | ||
|
|
||
| let parserOptions = { | ||
| module: typescript.ModuleKind.CommonJS |
There was a problem hiding this comment.
you might need to add jsx: true to the options
src/assets/TypeScriptAsset.js
Outdated
| let typescript = localRequire('typescript', this.name); | ||
|
|
||
| let transpilerOptions = { | ||
| compilerOptions: { |
There was a problem hiding this comment.
Might make sense to use this already to load a project's tsconfig.json file: https://www.npmjs.com/package/tsconfig
If you do, be sure to overwrite noEmit to be false, so that people can use tsc as a kind of type-linter by setting noEmit to true in their tsconfig.json.
There was a problem hiding this comment.
will def look into this
There was a problem hiding this comment.
Looks awesome thanks so much!! Just a couple things!
Can you also add a few tests? You can check the javascript ones to see how they are written. https://github.com/parcel-bundler/parcel/blob/master/test/javascript.js
src/assets/TypeScriptAsset.js
Outdated
| async transform() { | ||
| this.ast = await this.parse(this.contents); | ||
| this.isAstDirty = true; | ||
| } |
There was a problem hiding this comment.
This method shouldn't be necessary, and overriding it here disables other transforms like babel (post typescript) if needed, and uglify. parse should already be called by the super Asset class.
src/assets/TypeScriptAsset.js
Outdated
|
|
||
| let transpilerOptions = { | ||
| compilerOptions: { | ||
| module: typescript.ModuleKind.CommonJS, |
There was a problem hiding this comment.
Does this still support ES2015 modules as well?
There was a problem hiding this comment.
It's configurable by adding a tsconfig.json file, I changed it to default to ES2015 now.
There was a problem hiding this comment.
Sorry, I guess I should ask what this option is for. Does it control parsing behavior or code generation? If codegen, we want commonjs for sure.
src/assets/TypeScriptAsset.js
Outdated
| module: typescript.ModuleKind.CommonJS, | ||
| jsx: true | ||
| }, | ||
| fileName: this.basename |
There was a problem hiding this comment.
Maybe I'm missing something, but why do we need fileName here?
There was a problem hiding this comment.
If i'm reading the TypeScript transpile API docs correctly it should autodetect wether the code is ts or tsx based on filename, this is why i included it.
Usually TypeScript compiler uses file extension to determine if file should be parsed as '.tsx' or '.ts'. The same rule is applied during single file transpilation if the file name is provided.
There was a problem hiding this comment.
According to the docs, fileName is an optional parameter of the ts#transpile (which by the way has been deprecated in favour of transpileModule, that is been used here) and not a property of the tsconfig. If you take a look at the Json schema you won't find a fileName property there: http://json.schemastore.org/tsconfig
There was a problem hiding this comment.
The interface TranspileOptions which this is, does contain a FileName as far as i can see in their source code
export interface TranspileOptions {
compilerOptions?: CompilerOptions;
fileName?: string;
reportDiagnostics?: boolean;
moduleName?: string;
renamedDependencies?: Map<string>;
}
export function transpileModule(input: string, transpileOptions: TranspileOptions): TranspileOutput
|
Awesome thanks for your work on this!! I cleaned up a couple things here and added a test for |
|
Really good. I went through the PR I would also like to know understand how this was done. The code change was minimal but functionality wise I couldn't understand. Is there any doc that says explains how to extend support? |
|
@imvetri It extends the JSAsset class, which just takes over all functionality. |
* Tiny gitignore cleanup * firefox fix, #36 * ts support * tsx added * transpileoptions improvements and bugfix * tsconfig.json support * add basic typescript test * Default to ES2015 to stay consistent with the js default * remove custom transform, implement check in js transform for ts and put codegen back to commonjs * keep ts transform addition within ts module * ts testing and bugfix * improve jsx parameter and default to preserve and let babel handle it * improve jsx parameter and default to preserve and let babel handle it
* Tiny gitignore cleanup * firefox fix, #36 * ts support * tsx added * transpileoptions improvements and bugfix * tsconfig.json support * add basic typescript test * Default to ES2015 to stay consistent with the js default * remove custom transform, implement check in js transform for ts and put codegen back to commonjs * keep ts transform addition within ts module * ts testing and bugfix * improve jsx parameter and default to preserve and let babel handle it * improve jsx parameter and default to preserve and let babel handle it
Potential implementation of basic typescript support #3