Disallow clearing timers without no argument at all #1330
Conversation
|
Thanks for the PR! This section of the codebase is owned by @saschanaz - if they write a comment saying "LGTM" then it will be merged. |
inputfiles/overridingTypes.jsonc
Outdated
| "method": { | ||
| "clearInterval": { | ||
| "overrideSignatures": [ | ||
| "clearInterval(id: number | undefined): void" |
There was a problem hiding this comment.
overrideSignatures should be used as a last resort. Could you do the signature way as it's done below in OnErrorEventHandlerNonNull etc.?
Andarist
force-pushed
the
clear-timers-disallow-lack-of-arg
branch
2 times, most recently
from
9508d85
to
c6fcd7b
Compare
inputfiles/overridingTypes.jsonc
Outdated
| { | ||
| "name": "id", | ||
| "optional": "false", | ||
| "overrideType": "number | undefined" |
There was a problem hiding this comment.
overrideType should be also a last resort, perhaps additionalTypes: ["undefined"]?
Andarist
force-pushed
the
clear-timers-disallow-lack-of-arg
branch
from
c6fcd7b
to
40c0e96
Compare
Andarist
force-pushed
the
clear-timers-disallow-lack-of-arg
branch
from
40c0e96
to
418b048
Compare
|
@saschanaz thanks for the suggestions. Sorry for a few rapid pushes - I forgot that I had to accept baselines on top of rerunning the build script and didn't know at first how I should verify my changes locally 😅 I think this build will finally pass and it comes with your suggestions applied. |
|
Thanks! lgtm |
|
Merging because @saschanaz is a code-owner of all the changes - thanks! |
This aligns the signatures with the recent change in
@types/node(here).My rationale behind this change is outlined here, but in a short, I don't think those calls make any sense: