Skip to content
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

Make ClientEvents, CollectorFilter and Message types generic. #5447

Closed
wants to merge 5 commits into from
Closed

Make ClientEvents, CollectorFilter and Message types generic. #5447

wants to merge 5 commits into from

Conversation

VimHax
Copy link

@VimHax VimHax commented Mar 29, 2021

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

I've added some generic type parameters for the following types for the respective reasons stated below.

-> CollectorFilter - I feel that this type would benefit from being able to define exactly what parameters will be passed in to the function. Not only does this leave less room for mistakes but it also makes it so users don't have to work with the type any as much when they're using TypeScript.
-> ClientEvents - This type has been given a generic parameter which dictates which events will emit partial data. This is useful as redundant checking for partial data won't be required if the user has not enabled the respective partial in the ClientOptions.
-> Message - The generic parameter for this type describes whether or not the message was sent in a guild. When you're working with messages you often have to check whether or not Message#guild is null even when you're pretty certain it isn't, inserting Message#guild as Guild everywhere is also much less elegant. This parameter can also be used to add more type information to the return types of some functions, such as Message#reply which will always return a Message which was sent in a guild if the original message was sent in a guild.

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

@iCrawl
Copy link
Member

iCrawl commented Apr 3, 2021

This needs a rebase.

@VimHax
Copy link
Author

VimHax commented Apr 7, 2021

I merged the changes into master, will that do?

typings/index.d.ts Outdated Show resolved Hide resolved
@VimHax
Copy link
Author

VimHax commented Apr 16, 2021

Should've made separate PRs for each of the features I implemented.

@VimHax VimHax closed this Apr 16, 2021
@iCrawl iCrawl added this to the Version 13 milestone Jun 27, 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.

4 participants