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(babel-jest): add process.version chunk to the cache key #12122

Merged
merged 4 commits into from Dec 5, 2021

Conversation

mrazauskas
Copy link
Contributor

@mrazauskas mrazauskas commented Dec 5, 2021

Fixes #12117

Summary

As it is pointed out in the issue, currently babel-jest does not invalided Jest’s cache in case if users downgrade (or upgrade) Node’s version. (jest --clear-cache should be used.)

Seems like it may be a good idea to add process.version as one of cache key chunks. Should the string include only major / minor or full version? Looking at other chunks of the cache key, I went for full version. Not sure if this can be an issue for performance?

Test plan

I added a simple unit for getCacheKey function.

@codecov-commenter
Copy link

@codecov-commenter codecov-commenter commented Dec 5, 2021

Codecov Report

Merging #12122 (d3b85e3) into main (5640f88) will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #12122      +/-   ##
==========================================
+ Coverage   67.65%   67.71%   +0.05%     
==========================================
  Files         328      328              
  Lines       16990    16990              
  Branches     4817     4817              
==========================================
+ Hits        11495    11505      +10     
+ Misses       5462     5452      -10     
  Partials       33       33              
Impacted Files Coverage Δ
packages/babel-jest/src/index.ts 86.48% <ø> (+13.51%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5640f88...d3b85e3. Read the comment docs.

SimenB
SimenB approved these changes Dec 5, 2021
Copy link
Collaborator

@SimenB SimenB left a comment

thanks!

New syntax is added in minor versions, so we should at least have that. I think full makes sense as well swapping node versions often doesn't seem likely to me, so I don't think there's a performance issue here

@SimenB SimenB merged commit 3093c18 into facebook:main Dec 5, 2021
32 of 34 checks passed
@mrazauskas mrazauskas deleted the fix-add-process-version branch Dec 5, 2021
@github-actions
Copy link

@github-actions github-actions bot commented Jan 5, 2022

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants