New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid transpile the TypeScript code twice. #410
Conversation
@lijunle Thanks for the PR :) I'll try to take a look at this after work today. btw, could you also:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
src/utils.ts
Outdated
// See | ||
// https://github.com/Microsoft/TypeScript/blob/a757e8428410c2196886776785c16f8f0c2a62d9/src/compiler/sys.ts#L203 : | ||
// tslint:disable-next-line:max-line-length | ||
// See https://github.com/Microsoft/TypeScript/blob/a757e8428410c2196886776785c16f8f0c2a62d9/src/compiler/sys.ts#L203 : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, this change can be reverted.
src/utils.ts
Outdated
const start = src.length > 12 ? src.substr(1, 10) : ''; | ||
|
||
const sourceMapHook = `require('ts-jest').install()`; | ||
const sourceMapHook = `require('ts-jest').install(${JSON.stringify( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code style is strange.
export function injectSourcemapHook( | ||
filePath: string, | ||
typeScriptCode: string, | ||
src: string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The typeScriptCode
and src
are related but different. I do need to understand what/why we need babel after TypeScript (later).
@kulshekhar @GeeWee I suppose the code is ready to merge. Howover, I could like to discuss why anybody need babel be after TypeScript transform. The |
The main reason for using babel after TypeScript is to support hoisting. Without hoisting support, users would be forced to place |
With the other changes in this PR, there's really no reason to leave this in even behind a flag
@lijunle the PR has been merged and the latest version published on npm. Thanks again 馃槂 |
@kulshekhar It works for me...
BTW, I am not sure why my pre-commit style check convert the space to 4. 馃