fix: Remove data conditional check #11250
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
There was a problem hiding this comment.
Pull Request Overview
This PR removes the conditional if (data) checks before calling _patch(data) in the constructors of multiple structures including Webhook, ThreadChannel, Role, PermissionOverwrites, and GuildMember. The changes standardize the initialization pattern by always calling _patch regardless of whether data is provided.
- Removes conditional data checks before
_patch()calls in constructors - Affects 5 structure classes: Webhook, ThreadChannel, Role, PermissionOverwrites, and GuildMember
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/discord.js/src/structures/Webhook.js | Removes if (data) check before calling _patch(data) in constructor |
| packages/discord.js/src/structures/ThreadChannel.js | Removes if (data) check before calling _patch(data) in constructor |
| packages/discord.js/src/structures/Role.js | Removes if (data) check before calling _patch(data) in constructor |
| packages/discord.js/src/structures/PermissionOverwrites.js | Removes if (data) check before calling _patch(data) in constructor |
| packages/discord.js/src/structures/GuildMember.js | Removes if (data) check before calling _patch(data) in constructor |
Comments suppressed due to low confidence (1)
packages/discord.js/src/structures/ThreadChannel.js:43
- The constructor accesses
data.owner_iddirectly on line 43 before calling_patch. With the removedif (data)check, ifdatais null or undefined, this will throw a TypeError. The same issue exists on line 36 withdata.guild_id. Either restore the conditional check or add null/undefined handling in the constructor.
this.ownerId = data.owner_id;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Even though these constructors are private, typings needs to be updated discord.js/packages/discord.js/typings/index.d.ts Lines 3527 to 3528 in 7e5accf discord.js/packages/discord.js/typings/index.d.ts Lines 3823 to 3824 in 7e5accf |
Qjuh
left a comment
Qjuh
left a comment
There was a problem hiding this comment.
Apart from typings already pointed out LGTM
Jiralite
force-pushed
the
fix/data-removal
branch
from
7e5accf to
47faf6b
Compare
vladfrangu
left a comment
vladfrangu
left a comment
There was a problem hiding this comment.
Lets hope nothing breaks :))
* fix: remove conditional * types: sort types
These should never be constructed without data...
The blame seems to go years and years back. Was likely ancient logic made redundant over time.