-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
feat(structures): add Application structure #11393
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
📝 WalkthroughWalkthroughNew Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@packages/structures/src/application/Application.ts`:
- Around line 165-168: The getter Application.prototype.flags treats
this[kData].flags as missing when it's 0 due to a truthy check; change the check
in the get flags accessor to use a nullish check (e.g., compare against
null/undefined) so that zero is preserved and still returns new
ApplicationFlagsBitField(this[kData].flags as ApplicationFlags) instead of null;
update the logic in the get flags method that references this[kData].flags and
ApplicationFlagsBitField to perform the null/undefined check.
- Around line 220-224: The getter name eventsWebhookStatus is inconsistent with
eventWebhooksURL / eventWebhooksTypes and the API field event_webhooks_status;
rename the getter to eventWebhooksStatus (and update any references) so it
matches the camelCase convention, then update related typings, exports, and
documentation where eventsWebhookStatus is used to eventWebhooksStatus to keep
names consistent (search for eventsWebhookStatus, eventWebhooksURL,
eventWebhooksTypes and the kData access of event_webhooks_status to apply the
change).
In `@packages/structures/src/bitfields/ApplicationFlagsBitField.ts`:
- Around line 13-15: The toJSON override in ApplicationFlagsBitField is forcing
string serialization by calling super.toJSON(true); change it to use the base
numeric serialization (call super.toJSON() without the true flag) so numeric
bitflags are preserved, unless you intentionally want string output—in which
case update typings/docs to reflect string-returning toJSON; target the
ApplicationFlagsBitField class and its toJSON method and remove the true
argument from the super call.
- Around line 7-11: The implementation of ApplicationFlagsBitField uses the
wrong type parameter—change the extends clause from BitField<keyof
ApplicationFlags> to BitField<keyof typeof ApplicationFlags> to match the
exported ApplicationFlagsString typing; update the same pattern for
ChannelFlagsBitField, MessageFlagsBitField, and AttachmentFlagsBitField so all
use keyof typeof <FlagName> and ensure the static Flags = ApplicationFlags (and
respective Flags) remain unchanged.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
packages/structures/src/application/Application.tspackages/structures/src/application/index.tspackages/structures/src/bitfields/ApplicationFlagsBitField.tspackages/structures/src/bitfields/index.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/structures/src/bitfields/ApplicationFlagsBitField.ts (1)
packages/discord.js/typings/index.d.ts (2)
BitField(663-680)ApplicationFlagsBitField(480-483)
🔇 Additional comments (2)
packages/structures/src/bitfields/index.ts (1)
3-3: LGTM: export added for ApplicationFlagsBitFieldpackages/structures/src/application/index.ts (1)
1-1: LGTM: Application export added
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
AsadHumayun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a maintainer, just giving some suggestions based on the conventions I've seen so far in other structures and my own PRs.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11393 +/- ##
==========================================
- Coverage 31.62% 31.46% -0.16%
==========================================
Files 385 387 +2
Lines 13918 13988 +70
Branches 1096 1101 +5
==========================================
Hits 4401 4401
- Misses 9383 9453 +70
Partials 134 134
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This PR will add the
ApplicationAPI structure, per Discord's API documentation, as well as theApplicationFlagsBitFieldto/bitfields.Mentioning #10981 for visibility.