types(ClientEventTypes): use Events as keys#11121
types(ClientEventTypes): use Events as keys#11121Pavel-Boyazov wants to merge 6 commits intodiscordjs:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Jiralite
left a comment
There was a problem hiding this comment.
index.test-d.ts is full of errors due to this change.
It seems this stops strings from being used. Unsure if this is desired. @vladfrangu, any ideas?
|
I agree that removing the ability to use strings directly is a bad idea. But I would also like to add compatibility of the type with the Events enum, since in fact they are completely identical. |
|
As I see it, it comes down to the async_event_emitter's typing. I would like to understand more precisely what can and should be changed there in order to keep support for raw strings |
|
Another problem. |
I wouldn't want this to break normal strings, so if it does... I am -1 on this |
📝 WalkthroughWalkthroughA TypeScript type definition update to the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 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)
📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✏️ Tip: You can disable this entire section by setting Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/discord.js/typings/index.d.ts (1)
5461-5577:⚠️ Potential issue | 🟠 MajorAdd missing
voiceServerUpdatetoClientEventTypesinterface.
VoiceServerUpdateis defined in theEventsenum but has no corresponding entry inClientEventTypes. Withextends Record<Events, any[]>, any missing event key defaults toany[], breaking type safety. Add the typed entry to preserve strict event handling.Proposed fix
voiceChannelEffectSend: [voiceChannelEffect: VoiceChannelEffect]; + voiceServerUpdate: [data: GatewayVoiceServerUpdateDispatchData]; voiceStateUpdate: [oldState: VoiceState, newState: VoiceState];
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
packages/discord.js/typings/index.d.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Tests
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Please describe the changes this PR makes and why it should be merged:
Allow to use native Events as key for ClientEventTypes (for examle
ClientEventTypes[Events]instead ofClientEventTypes[keyof ClientEventTypes])Status and versioning classification: