Skip to content
This repository was archived by the owner on Jan 26, 2019. It is now read-only.

Read tsconfig content for jest ts-transform correctly #99

Closed
wants to merge 1 commit into from
Closed

Read tsconfig content for jest ts-transform correctly #99

wants to merge 1 commit into from

Conversation

andischerer
Copy link

Hi William,

thanks for your work on create-react-app on getting ts in there 👍.

I've ran into a bug in by using experimentaldecorators in tsconfig by running test and coverage scripts. Build and start tasks run fine while test runs fail with an ts-error related to decorators.
The jest ts-transform does not respect the tsconfig in my project root.

Looks like the method signature of readConfigFile had changed. Now there are 2 parameters required. I've added the missing fs-read parameter. Without this parameter, the tsconfig const is always empty and the default compilerOptions are used.
The content of the tsconfig file is now correctly read and the compilerOptions are getting respected in the ts-transform task.

@@ -15,7 +15,8 @@ let compilerConfig = {

if (fs.existsSync(tsconfigPath)) {
try {
const tsconfig = tsc.readConfigFile(tsconfigPath).config;
const tsconfig = tsc.readConfigFile(tsconfigPath, name =>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DorianGrey
Copy link
Collaborator

DorianGrey commented Jul 5, 2017

Oh, yeah, that's a result of an earlier PR of mine. Did not take the recent versions into consideration.

However, I'm afraid it takes a little more effort to get this working properly considering all potential circumstances.
A PR with the correct way to read the tsconfig.json already landed in ts-jest.
The relevant block starts here: https://github.com/kulshekhar/ts-jest/blob/master/src/utils.ts#L73

Just forgot to add a proper PR here as well - sorry for this inconvenience.

I'm wondering ... would there be any problem in using ts-jest instead of a custom transform solution (which aims at doing exactly the same)?

@wmonk
Copy link
Owner

wmonk commented Jul 5, 2017

I'm happy to move to ts-jest. Will it fix the issues found before? When I started this project i'm not sure ts-jest was working the way I wanted. But happy to move now!

@DorianGrey
Copy link
Collaborator

I've added a PR for further discussing the ts-jest topic (#100).

@wmonk
Copy link
Owner

wmonk commented Jul 11, 2017

Thanks for your work on this @andischerer, I believe using ts-jest is the way to go, so closing in favour of #100.

@wmonk wmonk closed this Jul 11, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants