8000 fix: remove incorrect nullables from ApplicationEmoji and GuildEmoji, add ApplicationEmoji#managed by Amgelo563 · Pull Request #10912 · discordjs/discord.js · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: remove incorrect nullables from ApplicationEmoji and GuildEmoji, add ApplicationEmoji#managed #10912

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

fix: remove incorrect nullables from ApplicationEmoji and GuildEmoji, add ApplicationEmoji#managed #10912

wants to merge 4 commits into from

Conversation

Amgelo563
Copy link
Contributor
@Amgelo563 Amgelo563 commented May 25, 2025 •
edited
Loading

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

ApplicationEmoji and BaseGuildEmoji inherit some incorrect nullables from Emoji (since they are never null for their case). Also BaseGuildEmoji#url wasn't marked as deprecated in the index.d.ts and ApplicationEmoji#available was missing (though always true, added for consistency with the rest). Internal discussion at https://discord.com/channels/222078108977594368/1366526457064591440/1366526457064591440

This also addresses the pending changes at #10905

Fixes #10858

Specific (attempted) changes

Typings

  • Remove | null from ApplicationEmoji#author, managed, and requiresColons
  • Change ApplicationEmoji#managed's type to false and requiresColons to true
  • Add ApplicationEmoji#available: true
  • Type override ApplicationEmoji#name, animated, imageURL, url, createdAt and createdTimestamp to make them non nullable
  • Add @deprecated tag for BaseGuildEmoji#url
  • Type override BaseGuildEmoji#name, animated, and managed to make them non nullable (the other properties were already non nullable)

JS

  • Add jsdoc comments overriding ApplicationEmoji#name, animated, imageURL, url, createdAt and createdTimestamp's to remove the "or null if unicode" mentions
  • Add jsdoc comments overriding BaseGuildEmoji#name, animated, createdAt and createdTimestamp's to remove the "or null if unicode" mentions (url and imageURL already have them)
  • Add available on ApplicationEmoji:
    • Set to null in constructor
    • Set in _patch()
    • Add check to equals()

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
@Amgelo563 Amgelo563 requested a review from a team as a code owner May 25, 2025 22:02
@Amgelo563 Amgelo563 requested review from vladfrangu, iCrawl, kyranet and SpaceEEC and removed request for a team May 25, 2025 22:02
@vercel Vercel
Copy link
vercel bot commented May 25, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
discord-js ⬜️ Ignored (Inspect) Visit Preview May 26, 2025 3:12pm
discord-js-guide ⬜️ Ignored (Inspect) Visit Preview May 26, 2025 3:12pm

Qjuh
Qjuh requested changes May 26, 2025
View reviewed changes
@github-project-automation github-project-automation bot moved this from Todo to Review in Progress in discord.js May 26, 2025
@Amgelo563 @Qjuh
types(BaseGuildEmoji): fix incorrect JSDoc type for BaseGuildEmoji#name
9903aad 
Co-authored-by: Qjuh <76154676+Qjuh@users.noreply.github.com>
Qjuh
Qjuh requested changes May 26, 2025
View reviewed changes
Copy link
Member
@Qjuh Qjuh left a comment

Choose a reason for hiding this comment

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

Just two small nits.

@Jiralite Jiralite changed the base branch from v14 to main May 26, 2025 15:11
@Jiralite Jiralite requested review from a team as code owners May 26, 2025 15:11
@Jiralite Jiralite requested review from almeidx and removed request for a team May 26, 2025 15:15
@Amgelo563
Copy link
Contributor Author

Closing to now target main instead

@Amgelo563 Amgelo563 closed this May 26, 2025
@github-project-automation github-project-automation bot moved this from Review in Progress to Denied in discord.js May 26, 2025
@almeidx almeidx mentioned this pull request May 26, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Qjuh Qjuh Qjuh requested changes

@vladfrangu vladfrangu Awaiting requested review from vladfrangu vladfrangu is a code owner automatically assigned from discordjs/core

@iCrawl iCrawl Awaiting requested review from iCrawl iCrawl is a code owner automatically assigned from discordjs/core

@kyranet kyranet Awaiting requested review from kyranet kyranet is a code owner automatically assigned from discordjs/core

@SpaceEEC SpaceEEC Awaiting requested review from SpaceEEC SpaceEEC is a code owner automatically assigned from discordjs/core

@Jiralite Jiralite Awaiting requested review from Jiralite Jiralite was automatically assigned from discordjs/util

@almeidx almeidx Awaiting requested review from almeidx

Assignees
No one assigned
Labels
packages:discord.js
Projects
discord.js
Status: Denied
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Incorrect typing for ApplicationEmoji
3 participants
@Amgelo563 @Qjuh @Jiralite
0