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

feat(Interactions)!: member objects for uncached guilds #10195

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

advaith1
Copy link
Contributor

@advaith1 advaith1 commented Mar 29, 2024

Please describe the changes this PR makes and why it should be merged:
fixes #10010
Discord thread

Currently, when an interaction is received from a guild that isn't cached*, raw API member objects are returned instead of a d.js GuildMember object. This PR creates a new MinimalGuildMember object that is returned instead of raw API member objects.

This new object tries to be compatible with GuildMember, but retains backwards compatibility with APIGuildMember as to not make this a breaking change. Therefore it is still somewhat awkward and not great, but it is much better than just receiving an APIGuildMember. Hopefully in v15 we can remove the backwards compatibility and align it better with GuildMember.

Since we're targeting v15 now, this PR updates GuildMember to extend MinimalGuildMember. all fields that do not require the guild are in MinimalGuildMember, and GuildMember adds the fields that use the cached guild.

This has not been fully tested and likely needs changes - please thoroughly review!

* This was always possible by adding apps with only the applications.commands scope, but user apps makes this a much more common appearance. This PR is based on the discord.js patch AdvaithBot is running to properly handle user app commands.

Status and versioning classification:

  • I know how to update typings and have done so, or typings don't need updating
  • This PR changes the library's interface (methods or parameters added)
  • This PR includes breaking changes (methods removed or renamed, parameters moved or removed)

@advaith1 advaith1 requested a review from a team as a code owner March 29, 2024 03:47
Copy link

vercel bot commented Mar 29, 2024

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 3, 2025 3:58am
discord-js-guide ⬜️ Ignored (Inspect) Visit Preview Jan 3, 2025 3:58am

Copy link
Member

@vladfrangu vladfrangu left a comment

Choose a reason for hiding this comment

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

There's a LOT of inconsistencies between trying to make this a non-semver major change (by also having snake_case properties) and a semver-major change (by not exposing all values under the same name). Let's pick one side (and gut says you want this in a sem-minor 👀) and roll with it!

  • nick isn't exposed as nick, only nickname
  • data.roles isn't exposed at all, only as roleIds
    • this one I understand why, but I'd at least try to make uncachedMember.roles be our role manager (it should work)

packages/discord.js/src/structures/UncachedGuildMember.js Outdated Show resolved Hide resolved
packages/discord.js/src/structures/UncachedGuildMember.js Outdated Show resolved Hide resolved
packages/discord.js/src/structures/UncachedGuildMember.js 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/src/structures/UncachedGuildMember.js Outdated Show resolved Hide resolved
packages/discord.js/src/structures/UncachedGuildMember.js Outdated Show resolved Hide resolved
packages/discord.js/src/structures/UncachedGuildMember.js Outdated Show resolved Hide resolved
packages/discord.js/src/structures/UncachedGuildMember.js Outdated Show resolved Hide resolved
advaith1 and others added 2 commits April 2, 2024 21:34
@vladfrangu
Copy link
Member

Marking the PR as both sem minor and major until I test it out and ensure this is a sem-minor PR 👀

packages/discord.js/src/structures/UncachedGuildMember.js Outdated Show resolved Hide resolved
packages/discord.js/src/structures/UncachedGuildMember.js Outdated Show resolved Hide resolved
packages/discord.js/typings/index.d.ts Outdated Show resolved Hide resolved
Copy link
Member

@vladfrangu vladfrangu left a comment

Choose a reason for hiding this comment

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

Thoughts on an AnyGuildMember type which is union of both Uncached and GuildMember and we have functions that assert whether a member instance is the uncached one?

@advaith1 advaith1 changed the title feat(Interactions): member objects for uncached guilds feat(Interactions)!: member objects for uncached guilds Jan 1, 2025
@advaith1
Copy link
Contributor Author

advaith1 commented Jan 3, 2025

Since this PR targets d.js v15 now, I've done some changes:

  • renamed UncachedGuildMember to MinimalGuildMember
  • removed backwards compatibility with the API object (since it was incorrect for the user field)
  • GuildMember now extends MinimalGuildMember and adds getters/functions that need the guild object
  • added member.isInCachedGuild() typeguard that assets it is a GuildMember

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Review in Progress
Development

Successfully merging this pull request may close these issues.

Return a d.js object instead of a raw member object for interactions from uncached guilds
7 participants