timers: use reflect.apply instead of spread

[gurgunday]

Small PR that uses ReflectApply for immediates like timeouts

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.47%. Comparing base (c6316f9) to head (6ae5953).
⚠️ Report is 210 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #60063      +/-   ##
==========================================
+ Coverage   88.45%   88.47%   +0.02%     
==========================================
  Files         703      703              
  Lines      207546   207547       +1     
  Branches    40011    39995      -16     
==========================================
+ Hits       183591   183635      +44     
+ Misses      15949    15898      -51     
- Partials     8006     8014       +8     

Files with missing lines	Coverage Δ
lib/internal/timers.js	99.58% <100.00%> (+<0.01%)	⬆️

... and 31 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[BridgeAR]

Benchmark https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1736/

[gurgunday]

14:04:14                                                confidence improvement accuracy (*)   (**)  (***)
14:04:14 timers/immediate.js type='breadth' n=5000000                  -0.18 %       ±0.86% ±1.15% ±1.49%
14:04:14 timers/immediate.js type='breadth1' n=5000000                 -0.59 %       ±0.76% ±1.02% ±1.34%
14:04:14 timers/immediate.js type='breadth4' n=5000000                  0.37 %       ±3.22% ±4.28% ±5.58%
14:04:14 timers/immediate.js type='clear' n=5000000              *     -0.66 %       ±0.58% ±0.78% ±1.01%
14:04:14 timers/immediate.js type='depth' n=5000000             **     -1.00 %       ±0.70% ±0.94% ±1.24%
14:04:14 timers/immediate.js type='depth1' n=5000000                   -1.04 %       ±1.21% ±1.61% ±2.09%
14:04:14 timers/set-immediate-breadth-args.js n=5000000                 0.37 %       ±1.26% ±1.67% ±2.18%
14:04:14 timers/set-immediate-breadth.js n=10000000                    -1.11 %       ±5.33% ±7.09% ±9.23%
14:04:14 timers/set-immediate-depth-args.js n=5000000                  -0.98 %       ±1.24% ±1.65% ±2.14%
14:04:14 
14:04:14 Be aware that when doing many comparisons the risk of a false-positive
14:04:14 result increases. In this case, there are 9 comparisons, you can thus
14:04:14 expect the following amount of false-positive results:
14:04:14   0.45 false positives, when considering a   5% risk acceptance (*, **, ***),
14:04:14   0.09 false positives, when considering a   1% risk acceptance (**, ***),
14:04:15   0.01 false positives, when considering a 0.1% risk acceptance (***)

[gurgunday]

I think we should still merge this since it prevents Array.prototype[Symbol.iterator] tampering

setTimeout(() => {
  console.log('Timeout executed');
  Array.prototype[Symbol.iterator] = function* () {
    console.log('Symbol.iterator called');
    yield* this.values();
  };

  setImmediate(
    (a, b, c) => {
      console.log('immediate executed');
      console.log(a, b, c);
    },
    1,
    2,
    3
  );
}, 1000);

[gurgunday]

@ljharb wdyt? we still use primordials where feasible right?

[ljharb]

Opinions vary on that - personally I'd like to see them used everywhere, but the best way to get a bunch of collabs blocking your change is to use a primordial in a way that causes a performance hit :-/

If folks think this perf hit is insignificant, then I'm sure it will land; if not, then it likely won't.

[gurgunday]

I see, I myself being someone obsessed with performance, I'd say there is a discrepancy between setTimeout and setImmediate right now

I think both should either use ReflectApply or neither should for predictably