feat(`no-callback-in-promise`): add `timeoutsErr` option

[brettz9]

What is the purpose of this pull request?

• [x] Bug fix
• [x] Changes an existing rule

What changes did you make? (Give an overview)

• feat(no-callback-in-promise): add timeoutsErr option; fixes #167
• fix(no-callback-in-promise): ensure timeouts do not err (by default); fixes #220

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (bbcfcbf) to head (cee71eb). Report is 65 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #514 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 25 26 +1 Lines 649 711 +62 Branches 250 277 +27 ========================================= + Hits 649 711 +62 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[scagood]

I think the approach here may not be right, as we're looking for the timer functions with callbacks, not just timer functions.

eg if we use the node:timers/promises api, then timers are fine:

import { setImmediate } from 'timers/promises';

whatever.then((err) => setImmediate('value'))

[brettz9]

Ah, good catch. Do you have a suggested way of catching those? Just introspecting on the imports for known promise APIs and failing otherwise?

[scagood]

The first way that comes to mind is using the ReferenceTracker from our eslint-utils.

But I think that it is going about things the wrong way round

---

Maybe the check could be as simple as checking the scope of the variable and if it's got declarations?

[brettz9]

The first way that comes to mind is using the ReferenceTracker from our eslint-utils.

But I think that it is going about things the wrong way round

Maybe the check could be as simple as checking the scope of the variable and if it's got declarations?

You mean because a declaration would itself imply a potentially Promise-based library import as opposed to the built-ins?

[brettz9]

I think the approach here may not be right, as we're looking for the timer functions with callbacks, not just timer functions.

eg if we use the node:timers/promises api, then timers are fine:

import { setImmediate } from 'timers/promises';

whatever.then((err) => setImmediate('value'))

Actually, the code is only checking for timers with callbacks. Your example currently passes with the PR.

The main idea with the timeouts in this PR is to permit them unless the option timeoutsErr: true is set, and then, to only fail with them if the contain a callback (not to fail with their presence).

I was thinking you were suggesting that the following could be more acceptable with the option set:

import { setImmediate } from 'timers/promises';
whatever.then((err) => setImmediate(cb))

...but I'm not sure that it is, as it is still passing around a callback inside of a promise.

[scagood]

Actually, the code is only checking for timers with callbacks. Your example currently passes with the PR.

Ah, that makes sense!