Cache tsconfig when generating per-file cache key.

[tvald]

No description provided.

[tvald]

Fixes #280.

[kulshekhar]

@tvald Thanks for the PR!

This looks good to me. Would it help to do this for calls to getTSConfig, getTSJestConfig and getPostProcessHook in the process function as well?

[GeeWee]

We'll need to keep in mind that caching might make #238 more difficult - however I definitely think it's worth it. Would like a comment explaining the reason for caching it though.

[kulshekhar]

@GeeWee yeah. That was the reason I was okay (tsconfig was being cached in the process function in earlier versions). But I now think this would be a good default which, if required in future, can be controlled by a configuration setting.

I also tried this setup when investigating #280. While there was some improvement, it wasn't enough that I'd consider #280 fixed by this. It would still be slower than mocha due to what I think is some kinda jest overhead

[tvald]

@GeeWee, quoting from #280:

We've identified the issue with ts-jest's caching implementation: in preprocessor.ts:getCacheKey(), tsconfig.json is loaded and wildcard paths (eg, include: ['src/**/*']) are resolved.

This results in an expensive walk of the entire source directory for each typescript file in the source directory, causing exponential growth in running time as a codebase expands. With our large codebase, this caused a no-op test to take more than 7 seconds to complete.

[tvald]

But also, I do agree that this is merely a quick patch to make ts-jest usable. I didn't dig into your architecture enough to determine the appropriate way to cache tsconfig.json exactly once per (project?) configuration.

[kulshekhar]

But also, I do agree that this is merely a quick patch to make ts-jest usable. I didn't dig into your architecture enough to determine the appropriate way to cache tsconfig.json exactly once per (project?) configuration.

I think this is the right way. The only concern is that there might be projects with multiple tsconfigs in different directories. But since we're not handling that use case currently and because that doesn't seem to be a very common use case, this change makes sense.

[kulshekhar]

@tvald Could you also

• do this for calls to getTSConfig, getTSJestConfig and getPostProcessHook in the process function as well
• add your name to the authors file
• bump the version patch in package.json

[tvald]

I moved caching inside getTSConfig(), keyed by the relevant parameters (skipBabel, collectCoverage, and config). That should alleviate @GeeWee's concern over #238, while also caching for every call into getTSConfig().

do this for calls to... getTSJestConfig and getPostProcessHook in the process function as well

Neither getTSJestConfig() nor getPostProcessHook appear to hit disk. getJestConfig() does hit disk, but isn't called anywhere within ts-jest. Is there a need to cache these?

[DorianGrey]

getJestConfig is only called by some unit tests - regularly, jest parses its config and forwards it (in a slightly transformed version) to the hooks of the preprocessors (esp. to process). So I'd say it's not required to cache its result.

getTSJestConfig only accesses a hash, so it should be negligible.

getPostProcessHook will automatically benefit from the caching in getTSConfig, and only deals with configs already parsed. Caching wouldn't be beneficial here.

getTSConfig is the only really expensive one since it reads, parses and transforms the config file. Even the typescript cli does several steps to ensure it is parsed only once per project.

[kulshekhar]

I've gone through the PR and this looks good to me

This is good to merge after one more collaborator okays it.

/cc @GeeWee @bcruddy @morajabi

[GeeWee]

Are we using a stringified JSON object as an object key here? That seems rather.. odd. Is there a reason we don't simply hash it?
At least I'd like a comment here explaining why JSON.stringify was chosen, as it's reasonably unintuitive, at least to me.

[GeeWee]

Unless I'm misunderstanding, then please clarify.

[tvald]

JSON.stringify is available without imports and will produce a consistent, unique key from the parameters. It didn't seem worth the extra effort to generate a uniform key via crypto. Can do that if it's desired, though.

[GeeWee]

It's a fair point, it might be faster as well. I'd like a comment pointing it out though, generally the 'what' factor of it is pretty high - it's definitely not what you expect a json serialized object to be used for.

[tvald]

Ah, I thought you meant that you wanted a comment here on GitHub. I've just added the explanation in the code.

[GeeWee]

Wonderful

[GeeWee]

I just have some minor nitpickery and we're good to go!

[GeeWee]

Merging - thanks for the contribution!