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 Util#splitMessage when destructured #3456

Merged
merged 1 commit into from
Sep 3, 2019
Merged

Conversation

BannerBomb
Copy link
Contributor

@BannerBomb BannerBomb commented Sep 1, 2019

Please describe the changes this PR makes and why it should be merged:
this PR fixes the Utils#splitMessage method because resolveString should be called as Util.resolveString since it's a static method.


Update
kyranet posted a better reason why this PR should be merged here.

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.

resolveString should be called as `Util.resolveString` since it's a static method.
@monbrey
Copy link
Member

monbrey commented Sep 1, 2019

What's the point of this exactly? The method wasn't broken, and this refers to Util already, as per MDN

this would be undefined if the method is deconstructed from the class, so I now see the use case for this

@kyranet
Copy link
Member

kyranet commented Sep 1, 2019

I have to note, though, your reason for the fix is incorrect. The method works fine with this, it refers to the class itself (aka the Util class) as it's in a static method, as well as if it was an instance method, it would refer to the instance itself.

What breaks this is when you destructure the method, as in:

const { util: { splitMessage } } = require('discord.js');
splitMessage('');
// TypeError: Cannot read property 'resolveString' of undefined

But if you do not destructure the method itself, it will work just fine:

const { util } = require('discord.js');
util.splitMessage('');
// [ '' ]

That's because when destructuring the method, you're removing it's context, used for the this value, hence the error. I believe this PR should be merged as this problem makes the method inconsistent with all the others, which don't error when destructured.

@Gawdl3y Gawdl3y changed the title fixed Util#splitMessage error Fix Util#splitMessage when destructured Sep 3, 2019
@iCrawl iCrawl merged commit 5d95a4b into discordjs:master Sep 3, 2019
@BannerBomb BannerBomb deleted the patch-1 branch September 3, 2019 23:17
samsamson33 pushed a commit to samsamson33/discord.js that referenced this pull request Feb 27, 2020
samsamson33 added a commit to samsamson33/discord.js that referenced this pull request Feb 27, 2020
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.

8 participants