Codecov Report

All modified and coverable lines are covered by tests ✅

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

@@            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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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'))

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

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?

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?

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.

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

Ah, that makes sense!