Skip to content

refactor: remove timer utilities from Client #6113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 16, 2021

Conversation

kyranet
Copy link
Member

@kyranet kyranet commented Jul 13, 2021

Please describe the changes this PR makes and why it should be merged:

⚠️ This PR removes the following methods:

  • BaseClient#setTimeout
  • BaseClient#setInterval
  • BaseClient#setImmediate
  • BaseClient#clearTimeout
  • BaseClient#clearInterval
  • BaseClient#clearImmediate

Why?

The aforementioned methods existed to make timers not hang the process when it's destroyed and nothing else kept the process on (from the user's side). Since all timers have an unref method (Immediate#unref, Immediate#unref), there is not longer a need to keep and track those timers, which can potentially lead to memory leaks.

Status and versioning classification:

  • Code changes have been tested against the Discord API, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating
  • This PR changes the library's interface (methods or parameters added)
  • This PR includes breaking changes (methods removed or renamed, parameters moved or removed)

@kyranet kyranet added this to the Version 13 milestone Jul 13, 2021
Copy link
Member

@SpaceEEC SpaceEEC left a comment

Choose a reason for hiding this comment

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

Is it really okay to not clear at least the intervals anymore?
A destroyed client will now continue sending typing start requests.
(As well as sweep messages and handlers, but those don't hit up Discord's api)

@kyranet
Copy link
Member Author

kyranet commented Jul 14, 2021

A destroyed client will now continue sending typing start requests.

Fixed with #6114 😄

(As well as sweep messages and handlers, but those don't hit up Discord's api)

Assigned the relevant intervals and calling clear on them since the last commit, good catch 👍🏼

@iCrawl iCrawl requested a review from SpaceEEC July 14, 2021 15:14
@iCrawl
Copy link
Member

iCrawl commented Jul 16, 2021

This needs a rebase.

@kyranet kyranet force-pushed the refactor/remove-timer-utilities branch from 77de634 to 46d8bfd Compare July 16, 2021 12:27
@kyranet
Copy link
Member Author

kyranet commented Jul 16, 2021

Done, @iCrawl

@iCrawl iCrawl merged commit 5ca97c9 into discordjs:master Jul 16, 2021
@kyranet kyranet deleted the refactor/remove-timer-utilities branch July 16, 2021 13:06
KagChi added a commit to KagChi/discord-akairo that referenced this pull request Jul 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants