feat(experimental-utils): extract isNodeOfTypeWithConditions out of ast-utils' predicates

[MichaelDeBoey]

Like #3677, but with extra conditions

[typescript-eslint]

Thanks for the PR, @MichaelDeBoey!

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.

[codecov]

Codecov Report

Merging #3837 (ebf6f45) into master (dda9cee) will decrease coverage by 0.86%.
The diff coverage is 92.30%.

@@            Coverage Diff             @@
##           master    #3837      +/-   ##
==========================================
- Coverage   93.51%   92.65%   -0.87%     
==========================================
  Files         151      192      +41     
  Lines        8144     8767     +623     
  Branches     2584     2694     +110     
==========================================
+ Hits         7616     8123     +507     
- Misses        167      260      +93     
- Partials      361      384      +23     

Flag	Coverage Δ
unittest	92.65% <92.30%> (-0.87%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...gin-internal/src/rules/no-poorly-typed-ts-props.ts	88.88% <ø> (ø)
...ges/eslint-plugin/src/rules/no-inferrable-types.ts	93.33% <ø> (ø)
...nt-plugin/src/rules/no-unused-vars-experimental.ts	91.39% <ø> (ø)
...ges/experimental-utils/src/ast-utils/predicates.ts	54.83% <75.00%> (ø)
...plugin-internal/src/rules/prefer-ast-types-enum.ts	91.30% <100.00%> (ø)
...plugin/src/rules/explicit-module-boundary-types.ts	89.47% <100.00%> (ø)
packages/eslint-plugin/src/rules/no-shadow.ts	77.34% <100.00%> (+0.17%)	⬆️
packages/eslint-plugin/src/rules/no-type-alias.ts	100.00% <100.00%> (ø)
...es/eslint-plugin/src/rules/no-use-before-define.ts	91.20% <100.00%> (+0.09%)	⬆️
.../eslint-plugin/src/util/explicitReturnTypeUtils.ts	100.00% <100.00%> (ø)
... and 41 more

[nx-cloud]

Nx Cloud Report

CI ran the following commands for commit ebf6f45. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status	Command
	nx test @typescript-eslint/eslint-plugin-internal
	nx test @typescript-eslint/eslint-plugin-tslint
	nx test @typescript-eslint/eslint-plugin
	nx test @typescript-eslint/eslint-plugin-tslint
	nx test @typescript-eslint/eslint-plugin
	nx test @typescript-eslint/eslint-plugin-tslint
	nx test @typescript-eslint/eslint-plugin
	nx run-many --target=typecheck --all --parallel
	nx run-many --target=build --all --parallel
	nx test @typescript-eslint/parser
	nx test @typescript-eslint/parser
	nx test @typescript-eslint/experimental-utils
	nx test @typescript-eslint/experimental-utils
	nx test @typescript-eslint/scope-manager
	nx test @typescript-eslint/visitor-keys
	nx test @typescript-eslint/typescript-estree
	nx run-many --target=build --all --parallel
	nx run-many --target=build --all --parallel
	nx test @typescript-eslint/scope-manager
	nx test @typescript-eslint/visitor-keys

---

Sent with 💌 from NxCloud.

[bradzacher]

I don't think that this a "good" change.
#3677 was great because it deduplicated a bunch of types AND it also kept the same perf with the constant logical expressions.

However this change adds in several function calls (Object.entries and Array.every) where before there was just one logical expression - which results in code that is 98% slower - https://jsbench.me/acktt3qk8u/1.

[MichaelDeBoey]

@bradzacher We're really talking about fractions over here.

The big benefit is that you don't have to declare the same thing in both return type and check, which for me is a big win in DX.

The Object.entries & Array.every are because the passed object could have multiple properties to be checked at (although that's not used currently).
I'm happy to implement it in another way if you see fit.

[bradzacher]

We're really talking about fractions over here.

In isolation - yes, but these functions can be called thousands of times each over a codebase - which can add up very quickly.
I guess though it doesn't matter toooo much as it's so small of a function.
These functions also aren't on any hot paths unless the code needs to be fixed - so they aren't called much.

Let's move the Object.entries call outside the inner function and that'll do.

[MichaelDeBoey]

Let's move the Object.entries call outside the inner function and that'll do.

@bradzacher What do mean exactly by this?

[MichaelDeBoey]

Due to the extraction of entries & @typescript-eslint/explicit-function-return-type, we have to duplicate this return type

If you want, I can extract this into a NodeIsOfTypeWithConditionsFunction type if you want, but this won't be much clearer I'm afraid 🤷‍♂️

[bradzacher]

I'm fine with the duplication on the util.
I'm also happy to just disable the linter on this check too.
We can merge this as is.

[bradzacher]

thanks for this!