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

perf: only register ts-node once when loading TS config files #12160

Merged
merged 3 commits into from Dec 20, 2021

Conversation

grxy
Copy link
Contributor

@grxy grxy commented Dec 17, 2021

Summary

Memory usage increased linearly when using multiple jest.config.ts files. This seems to have been caused by the fact that we require and register a new instance of ts-node whenever a new jest.config.ts file is encountered. By only registering one instance of ts-node, we are able to reduce memory usage significantly in multi-config projects.

Test plan

  1. Made the change in this repo.
  2. Ran yarn jest-ts in the issue repo
  3. Added additional jest.config.ts files to the issue repo
  4. Noticed that heap usage remained about the same regardless of number of jest.config.ts files

Closes #12159

@grxy grxy changed the title fix: only register ts-node once when loading TS config files perf: only register ts-node once when loading TS config files Dec 17, 2021
@codecov-commenter
Copy link

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

Codecov Report

Merging #12160 (431670a) into main (2b78dd2) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #12160   +/-   ##
=======================================
  Coverage   67.71%   67.71%           
=======================================
  Files         328      328           
  Lines       16991    16991           
  Branches     4817     4817           
=======================================
  Hits        11506    11506           
  Misses       5452     5452           
  Partials       33       33           
Impacted Files Coverage Δ
...ges/jest-config/src/readConfigFileAndSetRootDir.ts 2.94% <0.00%> (ø)

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 2b78dd2...431670a. Read the comment docs.

Copy link
Contributor

@G-Rath G-Rath left a comment

Damn this didn't occur to me when I reviewed the original PR, but it makes sense - while the ts-node requiring will be cached, the register won't; sorry about that!

This looks good to me 👍

(I don't have explicit permission to make this an approve, so this'll have to do)

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

@SimenB SimenB left a comment

makes a lot of sense, thanks!

@SimenB SimenB merged commit 4113c1d into facebook:main Dec 20, 2021
33 of 34 checks passed
@github-actions
Copy link

@github-actions github-actions bot commented Jan 20, 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 20, 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.

5 participants