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.

Sign up for GitHub

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

Jump to bottom

types: Allow only ephemeral for defer reply #10696

Merged
merged 8 commits into from
Jan 14, 2025
Merged

types: Allow only ephemeral for defer reply #10696

merged 8 commits into from
Jan 14, 2025

Conversation

imnaiyar
Copy link
Contributor

Please describe the changes this PR makes and why it should be merged:
Only ephemeral flag can be set for defer replies. The rest are ignored. But our typing indicates the other two can also be set.

Also typings for WebhookMessageEditOptions omits flags (for some reason) when SupressEmbeds can clearly be set

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

@imnaiyar imnaiyar requested a review from a team as a code owner January 10, 2025 14:36
@vercel Vercel
Copy link

vercel bot commented Jan 10, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
discord-js ⬜️ Ignored (Inspect) Visit Preview Jan 14, 2025 2:12pm
discord-js-guide ⬜️ Ignored (Inspect) Visit Preview Jan 14, 2025 2:12pm

@imnaiyar imnaiyar changed the title fix(types): remove unusable flags from defer options and include flags for webhook edit options fix(types): incorrect flags typings Jan 10, 2025
@Jiralite Jiralite changed the title fix(types): incorrect flags typings types: Allow only ephemeral for defer reply Jan 10, 2025
@Jiralite Jiralite added this to the discord.js 15.0.0 milestone Jan 10, 2025
@Jiralite
Copy link
Member

Jiralite commented Jan 10, 2025 •
edited
Loading

Also typings for WebhookMessageEditOptions omits flags (for some reason) when SupressEmbeds can clearly be set

The reason for this is because it is not documented. The API specification was not around then and it seems to be documented there:

https://github.com/discord/discord-api-spec/blob/997fa1595a6de6065f6aad0ebca47a2307df5bed/specs/openapi.json#L5137C47-L5137C82

https://github.com/discord/discord-api-spec/blob/997fa1595a6de6065f6aad0ebca47a2307df5bed/specs/openapi.json#L21224-L21291

It should be fine to allow this now.

Jiralite
Jiralite previously requested changes Jan 10, 2025
View reviewed changes
packages/discord.js/typings/index.d.ts Show resolved Hide resolved
@imnaiyar imnaiyar requested a review from Jiralite January 10, 2025 15:11
Jiralite
Jiralite reviewed Jan 13, 2025
View reviewed changes
Copy link
Member

@Jiralite Jiralite left a comment

Choose a reason for hiding this comment

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

Why not make WebhookMessageEditOptions extend MessageEditOptions in the JSDoc now?

@didinele didinele requested a review from Jiralite January 14, 2025 09:39
@kodiakhq kodiakhq bot merged commit 1fd587c into discordjs:main Jan 14, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jiralite Jiralite Jiralite approved these changes

@SpaceEEC SpaceEEC SpaceEEC approved these changes

@vladfrangu vladfrangu vladfrangu approved these changes

@iCrawl iCrawl Awaiting requested review from iCrawl iCrawl is a code owner automatically assigned from discordjs/core

@kyranet kyranet Awaiting requested review from kyranet kyranet is a code owner automatically assigned from discordjs/core

Assignees
No one assigned
Labels
packages:discord.js semver:patch typings
Projects
discord.js
Status: Done
Milestone
discord.js 15.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@imnaiyar @Jiralite @vladfrangu @SpaceEEC