Thanks for the PR, @sheetalkamat!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

Thanks for looking into the performance for us! I really appreciate the help 😄

It looks like this change breaks the persistent parse tests, which ensure that the IDE use case is working as expected.

you can run them locally from within the typescript-estree package:

cd packages/typescript-estree
yarn test persistentParse

cd packages/typescript-estree
yarn test persistentParse

Will do

Codecov Report

Merging #1693 into master will decrease coverage by 0.01%.
The diff coverage is 81.81%.

@@            Coverage Diff             @@
##           master    #1693      +/-   ##
==========================================
- Coverage   95.21%   95.19%   -0.02%     
==========================================
  Files         148      148              
  Lines        6953     6967      +14     
  Branches     2006     2007       +1     
==========================================
+ Hits         6620     6632      +12     
- Misses        123      124       +1     
- Partials      210      211       +1

@uniqueiniquity This PR is ready to review and merge with the changes we talked about

And I am not sure failing codecov/patch is about

@bradzacher any thoughts on the code coverage issue? it's a preemptive change for 3.9 so it makes sense that the path isn't taken.

If you want to satisfy the branch coverage, you should be able to use istanbul ignore comments to stop the coverage reporting the missed branch.

/* istanbul ignore if */ if (isRunningNoTimeoutFix) {
// ....
}

/* istanbul ignore else */ if (!isRunningNoTimeoutFix) {
// ....
}

I keep it around to try and prompt contributors to cover their code appropriately, though I don't usually treat code coverage as a hard merge blocker because there's always some edge cases and checks to keep the types "safe" (unless of course the coverage is terrible, then I RC 😅).

When it fails, I usually review the coverage report to see what code was missed, and if it's something that should be tested.
(yellow = if has a missed branch, red = code not executed)

If you're happy with this change, approve it and I'll gladly override the failed check to merge it.

(again, thanks to you both for looking into all of this for us)

I think we should ignore the coverage result instead of adding ignore comment since as soon as 3.9 releases things should start working... I will update this with master for merge