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(MessageComponent): emoji.name is not optional #179

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Dougley
Copy link

@Dougley Dougley commented Aug 8, 2021

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

[APIMessageComponentEmoji].name is always required.

Reference Discord API Docs PRs or commits:

https://discord.com/developers/docs/resources/emoji#emoji-object-emoji-structure

name: ?string (can be null only in reaction emoji objects)

@vladfrangu
Copy link
Member

We've tested it, and it's optional for custom emojis. Unless that changed recently.. I don't think this is valid 👀

@Dougley
Copy link
Author

Dougley commented Aug 8, 2021

Interesting, the docs seem to suggest it's required due to being supplied in all the examples and the docs stating it's only null for reactions. Incoming interactions will always have this field filled however, so ¯\_(ツ)_/¯

@vladfrangu
Copy link
Member

Oh, greatttt... its a send/receive difference... Can you change the one in POST then to be the Partial of the interface you modified?

@Dougley
Copy link
Author

Dougley commented Aug 8, 2021

Since components are part of action rows it's impossible to just Partial this easily, I think that changing how the types are referenced would overextend past the scope for this PR. I'm happy to close this PR and just patch around this quirk in my own code instead 😄

@vladfrangu
Copy link
Member

Hmm... yeah for REST you'd have to change it entirely... :notlikethis:...

In your code you can just slap a ! after .name or whatever property you need. I'd say keep the PR open for now, I can do it later if you'd like :D

@vladfrangu vladfrangu marked this pull request as draft August 8, 2021 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants