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: allow channels from uncached guilds to be returned from fetch #6034

Merged
merged 2 commits into from Jul 6, 2021

Conversation

ckohen
Copy link
Member

@ckohen ckohen commented Jul 4, 2021

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

This PR allows fetching channels from guilds that do not exist in the client cache (applicable mostly when sharding, but also possible to have guilds uncached via intents limiting or the new collection based limiting)

A big red flag warning here: channels obviously rely on guild internally a lot, however I did not want to change types when the majority of times the values will be present. I would expect people using this option to know exactly what they are doing and which properties can be accessed.

This also fixes up fetching single channels from the guild channel manager, making it cleaner and throwing when trying to fetch a channel from a different guild (this is the only breaking change)

closes: #4929

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

Copy link
Member

@kyranet kyranet left a comment

Choose a reason for hiding this comment

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

Just one thing, LGTM otherwise

src/errors/Messages.js Outdated Show resolved Hide resolved
@iCrawl iCrawl added this to the Version 13 milestone Jul 4, 2021
Copy link
Member

@iCrawl iCrawl left a comment

Choose a reason for hiding this comment

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

ID -> Id changes required.

src/structures/GuildChannel.js Outdated Show resolved Hide resolved
src/structures/ThreadChannel.js Outdated Show resolved Hide resolved
typings/index.d.ts Outdated Show resolved Hide resolved
@iCrawl
Copy link
Member

iCrawl commented Jul 4, 2021

Additionally this needs a rebase.

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.

Other than requested changes, LGTM

@iCrawl
Copy link
Member

iCrawl commented Jul 4, 2021

This needs a rebase.

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.

I would expect people using this option to know exactly what they are doing and which properties can be accessed.

We'll see about that 😄

@iCrawl
Copy link
Member

iCrawl commented Jul 5, 2021

This needs a rebase.

ckohen and others added 2 commits July 6, 2021 02:00
@iCrawl iCrawl merged commit 755c180 into discordjs:master Jul 6, 2021
@ckohen ckohen deleted the channel-no-guild branch July 7, 2021 05:07
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.

Allow ChannelManager#fetch without a guild
7 participants