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

types: fix regressions #7649

Merged
merged 10 commits into from Apr 3, 2022
Merged

types: fix regressions #7649

merged 10 commits into from Apr 3, 2022

Conversation

ImRodry
Copy link
Contributor

@ImRodry ImRodry commented Mar 13, 2022

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

This PR fixes a bunch of regressions and missed changes from #7584

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

@almeidx
Copy link
Member

almeidx commented Mar 13, 2022

Can you fix this too?

this.components = data.data.components?.map(c => createComponent(c)) ?? [];

Should be Components.createComponent(c) with const Components = require('../util/Components');

And the createComponent() method itself is missing support for text input components

switch (data.type) {
case ComponentType.ActionRow:
return new ActionRow(data);
case ComponentType.Button:
return new ButtonComponent(data);
case ComponentType.SelectMenu:
return new SelectMenuComponent(data);
default:
throw new Error(`Found unknown component type: ${data.type}`);
}

packages/discord.js/typings/index.d.ts Outdated Show resolved Hide resolved
packages/discord.js/typings/index.d.ts Outdated Show resolved Hide resolved
packages/discord.js/typings/index.d.ts Outdated Show resolved Hide resolved
packages/discord.js/typings/index.d.ts Outdated Show resolved Hide resolved
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.

Can you make tests for this and make sure it works with the suggested changes? 🙏

packages/discord.js/typings/index.d.ts Show resolved Hide resolved
packages/discord.js/typings/index.d.ts Outdated Show resolved Hide resolved
packages/discord.js/typings/index.d.ts Outdated Show resolved Hide resolved
packages/discord.js/typings/index.d.ts Outdated Show resolved Hide resolved
@kyranet kyranet linked an issue Mar 15, 2022 that may be closed by this pull request
@ImRodry ImRodry requested a review from vladfrangu March 15, 2022 20:41
Copy link
Member

@suneettipirneni suneettipirneni left a comment

Choose a reason for hiding this comment

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

packages/discord.js/typings/index.d.ts Outdated Show resolved Hide resolved
packages/discord.js/typings/index.d.ts Show resolved Hide resolved
packages/discord.js/typings/index.d.ts Outdated Show resolved Hide resolved
packages/discord.js/typings/index.d.ts Show resolved Hide resolved
@ImRodry
Copy link
Contributor Author

ImRodry commented Mar 19, 2022

Rebased and pushed a new commit that fixes a bug in the MessagePayload class that wouldn't allow for objects to be mixed with builder classes

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.

Small request, but doesn't block merge

packages/discord.js/typings/index.d.ts Show resolved Hide resolved
@KirmiwDev

This comment was marked as off-topic.

@Syjalo

This comment was marked as off-topic.

@iCrawl iCrawl added this to the discord.js v14 milestone Mar 26, 2022
@KirmiwDev

This comment was marked as off-topic.

@Mysterious-Dev
Copy link

Any news for this ?

@skaneprime
Copy link
Contributor

Looks to me That it's ready to go
Waiting for the last reviewer?

@iCrawl iCrawl merged commit 5748dbe into discordjs:main Apr 3, 2022
@ImRodry ImRodry deleted the types/fix-regressions branch April 3, 2022 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[bug] createComponent is not a function