fix: clear existing idle timeout in Collector.resetTimer#11460
fix: clear existing idle timeout in Collector.resetTimer#11460Sim-hu wants to merge 2 commits intodiscordjs:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can generate a title for your PR based on the changes.Add |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/discord.js/src/structures/interfaces/Collector.js`:
- Around line 280-286: In the method that clears/restarts the idle timer (the
block using this._idletimeout and setTimeout — e.g., resetTimer), after
clearTimeout(this._idletimeout) explicitly set this._idletimeout = null to avoid
keeping a stale truthy reference; when you later create a new timer assign the
return of setTimeout to this._idletimeout (and call .unref() as before). This
mirrors the stop() behavior and prevents handleCollect from mis-detecting an
active timer and re-enabling this.options.idle when idle was set to 0.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 31743e62-1a34-459c-9704-51b7f3beffa1
📒 Files selected for processing (1)
packages/discord.js/src/structures/interfaces/Collector.js
Summary
Collector.resetTimer({ idle: newValue })not properly resetting the idle timerthis._idletimeoutalready existed, and kept the clear and set coupled inside the same conditionalTest plan
resetTimer({ idle: 5000 })on a collector that was created with a different idle value and verify the new idle duration takes effectresetTimer({ idle: 5000 })on a collector that was created without an idle option and verify an idle timeout is now activeresetTimer()with no arguments on a collector with an existing idle timeout and verify it resets to the original idle value