timers: (clearImmediate) only accept immediate #57069
Conversation
nodejs-github-bot
added
needs-ci
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #57069 +/- ##
==========================================
- Coverage 89.09% 89.08% -0.01%
==========================================
Files 665 665
Lines 193249 193249
Branches 37231 37232 +1
==========================================
- Hits 172175 172159 -16
- Misses 13802 13822 +20
+ Partials 7272 7268 -4
|
gurgunday
force-pushed
the
timers-fix
branch
from
2a34ac5 to
69cec8a
Compare
|
Rebased |
jasnell
added
the
semver-major
| @@ -301,7 +301,7 @@ ObjectDefineProperty(setImmediate, customPromisify, { | |||
| * @returns {void} | |||
| */ | |||
| function clearImmediate(immediate) { | |||
| if (!immediate || immediate._destroyed) | |||
| if (!immediate?._onImmediate || immediate._destroyed) | |||
There was a problem hiding this comment.
Unfortunately, even tho we're not adding a throw, I think this is going to need to be semver-major
There was a problem hiding this comment.
No strong opinions, but isn't it a bug fix? Bug fixes are "breaking" by nature
There was a problem hiding this comment.
I think opinions on this can vary. Unfortunately since it has worked this way for such a long time we have to treat it as a semver-major
There was a problem hiding this comment.
Yeah I see, just wanted to double check, thanks for the response!
There was a problem hiding this comment.
I am +1 on this being semver-major
Fixes #57063