feat(invites): target users routes#1493
feat(invites): target users routes#1493Inbestigator wants to merge 10 commits intodiscordjs:mainfrom
Conversation
Inbestigator
requested review from
Jiralite,
almeidx,
iCrawl,
sdanialraza and
vladfrangu
|
@Inbestigator is attempting to deploy a commit to the discordjs Team on Vercel. A member of the Team first needs to authorize it. |
Inbestigator
changed the title
|
Can you add this for API v9 too please? |
Co-authored-by: Naiyar <137700126+imnaiyar@users.noreply.github.com>
|
Also missed |
Oops, didn't realize invites changed, sorry |
|
discord/discord-api-docs#8117 acc to this, not all of the properties are returned for role objects in invites, might wanna partialize it or since the properties returned through this endpoint is known, we can maybe make an APIInviteRole interface or something. Not sure what would be the best here |
|
Checking the API specification, if it's updated, would be a good idea in that case. |
export type APIInviteRole = Pick<APIRole, 'color' | 'colors' | 'icon' | 'id' | 'name' | 'position'>; |
| export type RESTPostAPIChannelInviteFormDataBody = { | ||
| /** | ||
| * A csv file with a single column of user IDs for all the users able to accept this invite | ||
| */ | ||
| target_users_file?: unknown; | ||
| } & ( | ||
| | RESTPostAPIChannelInviteJSONBody | ||
| | { | ||
| /** | ||
| * JSON stringified invite data | ||
| */ | ||
| payload_json?: string | undefined; | ||
| } | ||
| ); |
There was a problem hiding this comment.
No, intersections never are. If possible interfaces should always be preferred from a docs perspective.
There was a problem hiding this comment.
This follows how the other instances of payload_json are done
There was a problem hiding this comment.
And they are horrible from docs perspective too. I just answered Jiralite's question
https://discord.js.org/docs/packages/discord-api-types/0.38.38/v10/RESTPostAPIChannelMessageFormDataBody:TypeAlias
There was a problem hiding this comment.
Doesn't mean it should block this PR, you can PR a proper version
There was a problem hiding this comment.
what would be considered to be a "proper" fix for such a type?
There was a problem hiding this comment.
If i understood the issue correctly, something like this should work. do not mind the type names lol
interface NameWhtvX {
target_users_file?: unknown;
}
interface NameWhtvXWithJSONBody extends NameWhtvX, RESTPostAPIChannelInviteJSONBody {}
interface NameWhtvXWithPayload extends NameWhtvX {
payload_json?: string
}
type MainWhtv = NameWhtvXWithJSONBody | NameWhtvXWithPayload
| | 'verification_level' | ||
| >; | ||
|
|
||
| export type APIInviteRole = Pick<APIRole, 'color' | 'colors' | 'icon' | 'id' | 'name' | 'position'>; |
There was a problem hiding this comment.
From a docs perspective Pick is not a good idea. Would be better to introduce this APIInviteRole as new interface and do
export interface APIRole extends APIInviteRole { ... }moving the common properties to APIInviteRole.
There was a problem hiding this comment.
but APIInviteRole is technically derivative of APIRole, so it feels kinda confusing to make the actual APIRole type an extension of it.
There was a problem hiding this comment.
Confusing for whom? One is partial the other has more properties.
There was a problem hiding this comment.
I feel like it would seem backwards when reading the source. If you guys want this I'm fine to do it, but the Pick is also a precedented pattern.
5 participants
Please describe the changes this PR makes and why it should be merged:
This PR adds the new routes and their responses for target users. It also adds the
target_users_filekey in a formdata version ofRESTPostAPIChannelInviteJSONBody.If applicable, please reference Discord API Docs PRs or commits that influenced this PR:
https://discord.com/developers/docs/change-log#new-invite-endpoints