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

refactor!: Remove InteractionResponse #10689

Merged
merged 5 commits into from
Jan 12, 2025
Merged

refactor!: Remove InteractionResponse #10689

merged 5 commits into from
Jan 12, 2025

Conversation

Jiralite
Copy link
Member

@Jiralite Jiralite commented Jan 4, 2025 •
edited
Loading

Please describe the changes this PR makes and why it should be merged:
Removing InteractionResponse, a class that has brought us many pains and bugs (still present) that are difficult or impossible to fix (we go back and forth sometimes). The existing solution is to simply use withResponse, a recent addition to the Discord API, which does not incur an extra API call.

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
  • This PR includes breaking changes (methods removed or renamed, parameters moved or removed)

@Jiralite
refactor!: remove class
d02f35f 
BREAKING CHANGE: `InteractionResponse` has been removed. Create interaction collectors via `with_response` or fetching the reply.
@Jiralite Jiralite added this to the discord.js 15.0.0 milestone Jan 4, 2025
@vercel Vercel
Copy link

vercel bot commented Jan 4, 2025 •
edited
Loading

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

Name Status Preview Comments Updated (UTC)
discord-js ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 12, 2025 9:43pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
discord-js-guide ⬜️ Ignored (Inspect) Visit Preview Jan 12, 2025 9:43pm

@Jiralite Jiralite changed the title refactor!: remove class refactor!: Remove InteractionResponse Jan 4, 2025
@Jiralite Jiralite marked this pull request as ready for review January 4, 2025 23:10
@Jiralite Jiralite requested a review from a team as a code owner January 4, 2025 23:10
SpaceEEC
SpaceEEC requested changes Jan 12, 2025
View reviewed changes
Copy link
Member

@SpaceEEC SpaceEEC left a comment

Choose a reason for hiding this comment

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

🙏

packages/discord.js/typings/index.d.ts Outdated Show resolved Hide resolved
@Jiralite Jiralite force-pushed the refactor/remove-interaction-response branch from 9bb3b46 to 7fd8540 Compare January 12, 2025 19:52
@Jiralite Jiralite force-pushed the refactor/remove-interaction-response branch from 7fd8540 to 31f23df Compare January 12, 2025 19:57
monbrey
monbrey approved these changes Jan 12, 2025
View reviewed changes
Copy link
Member

@monbrey monbrey left a comment

Choose a reason for hiding this comment

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

LGTM.

We'll probably want to test InteractionCollector on ephemeral InteractionCallbackResource messages now that this is fully cleaned up.

almeidx
almeidx requested changes Jan 12, 2025
View reviewed changes
Copy link
Member

@almeidx almeidx left a comment

Choose a reason for hiding this comment

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

Needs some more tests for not passing options 😅

packages/discord.js/typings/index.d.ts Outdated Show resolved Hide resolved
packages/discord.js/typings/index.d.ts Outdated Show resolved Hide resolved
packages/discord.js/typings/index.d.ts Outdated Show resolved Hide resolved
packages/discord.js/typings/index.d.ts Outdated Show resolved Hide resolved
packages/discord.js/typings/index.d.ts Outdated Show resolved Hide resolved
packages/discord.js/typings/index.d.ts Outdated Show resolved Hide resolved
packages/discord.js/typings/index.d.ts Outdated Show resolved Hide resolved
Jiralite and others added 2 commits January 12, 2025 21:38
@Jiralite @almeidx
types: optionality
c60bec4 
Co-authored-by: Almeida <github@almeidx.dev>
@Jiralite
test: add more tests
4e85777
@Jiralite Jiralite requested a review from almeidx January 12, 2025 21:43
@Jiralite Jiralite merged commit 7e81d3b into main Jan 12, 2025
25 checks passed
@Jiralite Jiralite deleted the refactor/remove-interaction-response branch January 12, 2025 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@almeidx almeidx almeidx approved these changes

@SpaceEEC SpaceEEC SpaceEEC approved these changes

@monbrey monbrey monbrey approved these changes

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

@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 refactor semver:major
Projects
discord.js
Status: Done
Milestone
discord.js 15.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@Jiralite @monbrey @SpaceEEC @almeidx