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

fix(Message): make #channel and #guild getters #6271

Merged
merged 4 commits into from Aug 4, 2021

Conversation

almostSouji
Copy link
Member

@almostSouji almostSouji commented Aug 2, 2021

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

While the direct issue outlined in #6168 has been fixed in #6263, the same issue (Message#channel returning a text channel) still persists for old messages sent in this same channel before it was converted. As outlined in the issue the least intrusive way to fix this I could think of is making Message#channel a getter to retrieve the channel from cache on demand. I use the Client cache (versus the guild cache) to be robust against DMs, if that has any caveats I did not think of, please let me know

  • Make Message#channel a getter
  • ChangeMessage#guild getter (relied on Message#channel before)
  • remove the extra channel from the Message constructor and MessageManager#_add call
  • use ids within Message structure when possible
  • use Message#channelId whenever possible in other structures
  • shortcut Message#channel#guild to Message#guild
⚠️ technically this has the caveat of evicting the channel from cache and trying to do operations with the message that rely on its channel. to fix this a plethora of operations on the Message class would need to be changed to use optional chaining or even throw if the channel is not available. however for now we decided to handle it like this:

DO NOT UNCACHE CHANNELS IF YOU NEED THEM TO DO THINGS

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 changes the library's interface (methods or parameters added)

@iCrawl iCrawl added this to the Version 13 milestone Aug 2, 2021
@almostSouji almostSouji marked this pull request as draft August 2, 2021 09:16
@almostSouji almostSouji changed the title fix(Message): make #channel a getter fix(Message): make #channel and #guild getters Aug 2, 2021
@almostSouji almostSouji marked this pull request as ready for review August 2, 2021 22:43
src/structures/Message.js Show resolved Hide resolved
src/structures/MessageMentions.js Outdated Show resolved Hide resolved
@iCrawl
Copy link
Member

iCrawl commented Aug 3, 2021

This needs a rebase.

@iCrawl iCrawl merged commit 6e3236a into discordjs:master Aug 4, 2021
@almostSouji almostSouji deleted the msg-ch-getter branch January 7, 2022 20:00
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.

None yet

6 participants