Skip to content
This repository has been archived by the owner on Dec 24, 2021. It is now read-only.

fix: replace resolveString with verifyString #402

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

monbrey
Copy link
Member

@monbrey monbrey commented Jun 4, 2021

Replaces a reference to the now removed Util.resolveString with Util.verifyString
Introduced by discordjs/discord.js#4880

src/extensions/message.js Outdated Show resolved Hide resolved
@ghost
Copy link

ghost commented Jun 4, 2021

I just tested this PR and looks like this.options is giving the error TypeError: Cannot read property 'content' of undefined, without the this.options is working fine

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Take out this.options

@ghost
Copy link

ghost commented Jun 9, 2021

Shouldn't StringResolvable be Replaced with string in the Typings?

@kyranet
Copy link
Member

kyranet commented Jun 9, 2021

StringResolvable is being used for message sending and editing, both of which are changing in discordjs/discord.js#5758

Once the PR has landed, this can be updated and fix two breaking changes together.

@monbrey
Copy link
Member Author

monbrey commented Jun 9, 2021

I don't believe this PR should address both changes, no. I will update the typing though.

@kyranet
Copy link
Member

kyranet commented Jun 9, 2021

Oh, yeah mb, I thought this was a PR for updating Commando for v13.

@ghost
Copy link

ghost commented Jun 9, 2021

Oh, yeah mb, I thought this was a PR for updating Commando for v13.

There are a lot of things missing to support v13, This is getting kinda off-topic lets discuss on library-discussion if You want.

@arifali123
Copy link

can this be merged so we can use commando with the master version of discord.js

@ghost
Copy link

ghost commented Jun 29, 2021

This PR just Broke lmao, now You get verifyString is not a function

@kyranet
Copy link
Member

kyranet commented Jun 29, 2021

It's still in there: https://github.com/discordjs/discord.js/blob/master/src/util/Util.js#L403

Are you sure you didn't change back to 12.5.3?

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Make Separate imports to escapeMarkdown, splitMessage, verifyString
const { // Imports } = require('discord.js').Util;

@ghost
Copy link

ghost commented Jun 29, 2021

It's still in there: https://github.com/discordjs/discord.js/blob/master/src/util/Util.js#L403

Are you sure you didn't change back to 12.5.3?

Yea I didn't changed back to 12, He Just forgot to add .Util after requiring d.js

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants