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: consider #nsfw false if not present in data #4593

Merged
merged 1 commit into from
Aug 11, 2020

Conversation

almostSouji
Copy link
Member

@almostSouji almostSouji commented Jun 28, 2020

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

As #4296 brought up, discord.js has text type channels in cache where the .nsfw property returns undefined.

Problem breakdown:

  • After some investigation the initial guess of this being caused by discord.js handling channel updates can be dismissed
  • This is indeed caused by discord not sending nsfw in its channel data on initial guild payloads
  • There are no updates sent after ready to provide these missing properties

This PR attempts to fix this inconsistency by considering the channel to not be nsfw if the .nsfw key is not present in the initial channel data.

Changes breakdown:

TextChannel

  • assign .nsfw in the constructor with this.nsfw = Boolean(data.nsfw);
  • patch .nsfw in _patch with if (typeof data.nsfw !== 'undefined') this.nsfw = Boolean(data.nsfw);
  • move .nsfw docstring into the constructor call

StoreChannel

  • assign .nsfw in the constructor with this.nsfw = Boolean(data.nsfw);
  • patch .nsfw in _patch with if (typeof data.nsfw !== 'undefined') this.nsfw = Boolean(data.nsfw);
  • move .nsfw docstring into the constructor call
  • explicit constructor call due to above changes
  • document constructor akin to TextChannel
Further considerations:

From my own tests (sample size: 237 channels out of which 44 are affected) i can say that after requesting said channels through the raw API endpoint GET /channels/:id none of the "pending" nsfw properties were true.

Sinister Rectus, dev of discordia could also reproduce this behavior and they seem to handle this the same way.

According to sinister this is also done on user.bot where we do handle this properly by assigning the Boolean(data.bot) in the constructor

this.bot = Boolean(data.bot);

and only patching if the value is not undefined

if (typeof data.bot !== 'undefined') this.bot = Boolean(data.bot);

I have applied this same reasoning to TextChannel and NewsChannel in this PR.

Typings considerations:
  • Both TextChannel as well as NewsChannel are typed boolean instead of ?boolean already, so this PR (fix) is consistent with the already applied typings.
  • The constructor for NewsChannel has already been typed, but is now also documented accordingly

Status

  • 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

Semantic versioning classification:

  • This PR changes the library's interface (methods or parameters added)
    • This PR includes breaking changes (methods removed or renamed, parameters moved or removed)
  • This PR only includes non-code changes, like changes to documentation, README, etc.

@iCrawl iCrawl removed the request for review from SpaceEEC August 11, 2020 19:00
@iCrawl iCrawl merged commit fab3153 into discordjs:master Aug 11, 2020
@almostSouji almostSouji deleted the fix-nsfw-undefined branch September 27, 2020 10:40
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.

channel.nsfw returns undefined if the channel is not nsfw
8 participants