Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: always do type check for all transformed files #1450

Merged
merged 5 commits into from
Mar 26, 2020
Merged

fix: always do type check for all transformed files #1450

merged 5 commits into from
Mar 26, 2020

Conversation

ahnpnl
Copy link
Collaborator

@ahnpnl ahnpnl commented Mar 25, 2020

closes #1390

Previously we always do type checking while compiling codes. However, when a file is cached and the next time when running tests, our compiling logic won't be invoked when the file content (which is provided by jest as input for getCacheKey) is the same as cached content therefore the unchanged file won't be type checking.

So the change is moving the part of type checking to the entry point getCacheKey which jest uses to determine whether a file should be recompiled or simply get cached content.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 4189

  • 28 of 28 (100.0%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.01%) to 92.631%

Totals Coverage Status
Change from base Build 4188: -0.01%
Covered Lines: 1123
Relevant Lines: 1160

💛 - Coveralls

@ahnpnl ahnpnl marked this pull request as ready for review March 26, 2020 14:59
@ahnpnl ahnpnl requested a review from kulshekhar March 26, 2020 14:59
@ahnpnl ahnpnl merged commit 107e062 into kulshekhar:master Mar 26, 2020
@ahnpnl ahnpnl deleted the fix-type-check branch March 26, 2020 15:49
@kirillgroshkov
Copy link
Contributor

Will it still respect the diagnostics: false config option?

For me, after updating to this version, I see the increased memory heap consumption (as if it was not using jest cache). Basically, with [email protected] with warm cache my first tests reported ~100Mb heap (or ~500Mb with cold cache). [email protected] report ~500Mb on first test files regardless of warm or cold cache.

@ahnpnl
Copy link
Collaborator Author

ahnpnl commented Mar 30, 2020

yes, this PR still respect diagnostics: false.

@kirillgroshkov
Copy link
Contributor

yes, this PR still respect diagnostics: false.

Got it. Then my observations are probably caused by something else. Will keep an eye on the new version and report if I find anything else suspicious.

Thanks for the great work BTW!

@ahnpnl
Copy link
Collaborator Author

ahnpnl commented Mar 30, 2020

watch mode fix will have to wait :) this PR only fixed the issue for non watch mode

@ahnpnl
Copy link
Collaborator Author

ahnpnl commented Mar 30, 2020

@kirillgroshkov besides the memory part, how do you see the speed of no cache in 25.3.0 comparing to 25.2.1 ?

@kirillgroshkov
Copy link
Contributor

The speed, in terms of how many seconds it takes to complete all tests is more or less identical, so, no change between 25.2.1 and 25.3.0

@ahnpnl
Copy link
Collaborator Author

ahnpnl commented Mar 30, 2020

thanks :) i'm glad to hear that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

With cached results, changes to type definitions are not detected
4 participants