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

timers: support `Symbol.dispose` by MoLow · Pull Request #48633 · nodejs/node · GitHub

/ node Public

timers: support Symbol.dispose - #48633

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
MoLow:timers-destroy
Jul 5, 2023
ConversationCommits (1)ChecksFiles changed
Merged

timers: support Symbol.dispose#48633
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
MoLow:timers-destroy

Conversation

MoLow commented Jul 2, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

this does not change the behavior of node:timers/promises.

nodejs-github-bot added needs-ci PRs that need a full CI run. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. labels Jul 2, 2023
MoLow requested review from benjamingr and ronag July 2, 2023 08:02

aduh95 commented Jul 2, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Is the web going to follow this? I assume they will, but if it hasn't been spec'd yet, it seems more prudent to hold off until than happens.
EDIT: well obviously the web is returning a number, not an object, so it seems highly unlikely that they will ever do that. So I guess the question is wether we want to introduce this knowing that it's going to make code diverge between Node.js and other runtimes.

MoLow commented Jul 2, 2023

Copy link
Copy Markdown
Member Author

So I guess the question is whether we want to introduce this knowing that it's going to make code diverge between Node.js and other runtimes.

I am ok with adding it since it is experimental and using keyword is opt-in, but this is a very good point.
also we already expose unref/ref etc. so it wont be the first difference.

MoLow added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 5, 2023
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 5, 2023

nodejs-github-bot commented Jul 5, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52615/

nodejs-github-bot commented Jul 5, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52619/

MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 5, 2023
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 5, 2023
nodejs-github-bot merged commit 56b8de1 into nodejs:main Jul 5, 2023

nodejs-github-bot commented Jul 5, 2023

Copy link
Copy Markdown
Collaborator

Landed in 56b8de1

MoLow deleted the timers-destroy branch July 5, 2023 13:49
This was referenced Jul 6, 2023
Open
Open
Open
Open
Open
juanarbol pushed a commit that referenced this pull request Jul 13, 2023
timers: support Symbol.dispose
e2442bb 
PR-URL: #48633
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
juanarbol mentioned this pull request Jul 13, 2023
Merged
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
timers: support Symbol.dispose
7b2121d 
PR-URL: nodejs#48633
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
timers: support Symbol.dispose
3d76ff8 
PR-URL: nodejs#48633
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>

ruyadorno commented Sep 11, 2023

Copy link
Copy Markdown
Member

This commit does not land cleanly on v18.x-staging and will need manual backport in case we want it in v18.

MoLow commented Sep 11, 2023

Copy link
Copy Markdown
Member Author

@ruyadorno now that #49598 landed on v18.x-staging this seems to land cleanly

ruyadorno pushed a commit that referenced this pull request Sep 12, 2023
timers: support Symbol.dispose
0a12055 
PR-URL: #48633
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
ruyadorno pushed a commit that referenced this pull request Sep 13, 2023
timers: support Symbol.dispose
959b0c2 
PR-URL: #48633
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
ruyadorno mentioned this pull request Sep 13, 2023
Merged
ruyadorno pushed a commit that referenced this pull request Sep 17, 2023
timers: support Symbol.dispose
7822a54 
PR-URL: #48633
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
skyclouds2001 mentioned this pull request Apr 5, 2025
Closed
lucacasonato mentioned this pull request Jun 12, 2025
Open
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

benjamingr benjamingr approved these changes
ronag ronag approved these changes
himself65 himself65 approved these changes

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL