FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: add missing parameter types by VoltrexKeyva · Pull Request #39013 · nodejs/node · GitHub

/ node Public

doc: add missing parameter types - #39013

Closed
VoltrexKeyva wants to merge 2 commits into
nodejs:masterfrom
VoltrexKeyva:patch-8
Closed

doc: add missing parameter types#39013
VoltrexKeyva wants to merge 2 commits into
nodejs:masterfrom
VoltrexKeyva:patch-8

Conversation

VoltrexKeyva commented Jun 12, 2021
edited
Loading

Copy link
Copy Markdown
Contributor

Both of the clearTimeout() and clearInterval() functions in the timers lib accepts the primitive of the Timeout object returned by the functions in a number or string type, e.g.

const t = setTimeout(console.log, 5000, 'test');

clearTimeout(t[Symbol.toPrimitive]());

Both of the `clearTimeout()` and `clearInterval()` functions in the `timers` lib accepts the ID of the `Timeout` object returned by the functions in a number or string type, e.g.
```js
const t = setTimeout(console.log, 5000, 'test');

clearTimeout(t[Symbol.toPrimitive]());
```
github-actions Bot added doc Issues and PRs related to the documentations. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. labels Jun 12, 2021

Trott left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Generally looks good to me. OK with it as is if people want to land it, but left a few comments suggesting some modifications I'd prefer to see.

Comment thread doc/api/timers.md Outdated
Comment thread doc/api/timers.md Outdated

Trott commented Jun 12, 2021

Copy link
Copy Markdown
Member

@nodejs/timers

lpinca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM with @Trott's comments addressed.

Change "ID" to "primitive" and reference `timeout[Symbol.toPrimitive]()` with a hyperlink.

benjamingr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I don't understand this change - when is the return value not a timeout?

Copy link
Copy Markdown
Contributor Author

I don't understand this change - when is the return value not a timeout?

This change is not related to the return value, it is related to the parameter type of clearTimeout() and clearInterval() which also accepts the primitive of a Timeout object rather than just the Timeout object itself.

benjamingr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

RaisinTen added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jun 13, 2021

jasnell commented Jun 14, 2021

Copy link
Copy Markdown
Member

Landed in 68229a9

jasnell closed this Jun 14, 2021
jasnell pushed a commit that referenced this pull request Jun 14, 2021
Both of the `clearTimeout()` and `clearInterval()` functions in the
`timers` lib accepts the ID of the `Timeout` object returned by the
functions in a number or string type, e.g.

```js
const t = setTimeout(console.log, 5000, 'test');

clearTimeout(t[Symbol.toPrimitive]());
```

PR-URL: #39013
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
VoltrexKeyva deleted the patch-8 branch June 14, 2021 15:49
danielleadams pushed a commit that referenced this pull request Jun 15, 2021
Both of the `clearTimeout()` and `clearInterval()` functions in the
`timers` lib accepts the ID of the `Timeout` object returned by the
functions in a number or string type, e.g.

```js
const t = setTimeout(console.log, 5000, 'test');

clearTimeout(t[Symbol.toPrimitive]());
```

PR-URL: #39013
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams mentioned this pull request Jun 15, 2021
danielleadams pushed a commit that referenced this pull request Jun 17, 2021
Both of the `clearTimeout()` and `clearInterval()` functions in the
`timers` lib accepts the ID of the `Timeout` object returned by the
functions in a number or string type, e.g.

```js
const t = setTimeout(console.log, 5000, 'test');

clearTimeout(t[Symbol.toPrimitive]());
```

PR-URL: #39013
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
richardlau pushed a commit that referenced this pull request Jul 19, 2021
Both of the `clearTimeout()` and `clearInterval()` functions in the
`timers` lib accepts the ID of the `Timeout` object returned by the
functions in a number or string type, e.g.

```js
const t = setTimeout(console.log, 5000, 'test');

clearTimeout(t[Symbol.toPrimitive]());
```

PR-URL: #39013
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
richardlau pushed a commit that referenced this pull request Jul 20, 2021
Both of the `clearTimeout()` and `clearInterval()` functions in the
`timers` lib accepts the ID of the `Timeout` object returned by the
functions in a number or string type, e.g.

```js
const t = setTimeout(console.log, 5000, 'test');

clearTimeout(t[Symbol.toPrimitive]());
```

PR-URL: #39013
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
richardlau mentioned this pull request Jul 20, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. doc Issues and PRs related to the documentations. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL