Refactor: command builder #21
Conversation
📝 WalkthroughWalkthroughThis PR replaces module path aliases with relative paths, refactors the command builder system to use a new ApplicationCommandBuilder, removes the Argument class, updates event handlers with private ES fields, and adjusts Context's i18n API. Dependencies are reorganized, build output is modified, and example code is updated. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Failure to add the new IP will result in interrupted reviews. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/events/interaction.ts (1)
40-50: 🧹 Nitpick | 🔵 TrivialError messages in catch block are not localized.
The error handling uses hardcoded English strings while other error messages use
ctx.t(). Consider using localized messages for consistency.♻️ Proposed fix for consistency
if (interaction.replied || interaction.deferred) { await interaction.followUp({ - content: "There was an error while executing this command!", + content: ctx.t("error.command.execution", { + defaultValue: "There was an error while executing this command!", + }), flags: MessageFlags.Ephemeral, }); } else { await interaction.reply({ - content: "There was an error while executing this command!", + content: ctx.t("error.command.execution", { + defaultValue: "There was an error while executing this command!", + }), flags: MessageFlags.Ephemeral, }); }src/structures/core/Client.ts (1)
61-67:⚠️ Potential issue | 🟠 MajorFire-and-forget async call may cause race condition.
#loadDiris called with.catch()but withoutawait, meaninglogin()proceeds to initialize i18n and callsuper.login()before directory loading completes. Commands and events may not be loaded when the client becomes ready, causing handlers to be missing for early events.🐛 Proposed fix to await directory loading
override async login(token?: string) { if (this.options.includePaths) { for (const p of this.options.includePaths) { - this.#loadDir(path.join(getProjectRoot(), p)).catch((error) => - this.logger.error("Error loading events:", error) - ); + try { + await this.#loadDir(path.join(getProjectRoot(), p)); + } catch (error) { + this.logger.error("Error loading events:", error); + } } } if (this.i18n && !this.i18n.isInitialized) {
🤖 Fix all issues with AI agents
In `@examples/basic_client/locales/en-US/error.json`:
- Around line 1-4: The two locale entries "command.notfound" and
"command.disabled" contain accidental trailing digits; update their values to
user-facing text without placeholders (e.g., change "Command not found or
disabled1" and "Command not found or disabled2" to a clean message such as
"Command not found or disabled") so the UI shows the correct string for both
keys.
In `@src/events/interaction.ts`:
- Around line 11-28: Move the locale initialization (set ctx.locale) to run
before the command existence and supportsSlash checks, and update the two ctx.t
calls inside the interaction handler so their defaultValue strings are distinct
(e.g., "Command not found." for error.command.notfound and "Slash command
disabled." for error.command.disabled) so users see different fallback messages;
modify the interaction.reply calls that reference ctx.t for these keys and
ensure they execute after ctx.locale is set.
In `@src/events/message.ts`:
- Line 25: The Context created in message handler (new Context(context.client, {
message, args })) never has ctx.locale set, so ctx.t() falls back to default;
after creating ctx assign ctx.locale using available sources such as
message.guild?.preferredLocale or a stored user preference, e.g., ctx.locale =
message.guild?.preferredLocale ?? getUserLocale(message.author.id) ??
context.client.config.defaultLocale; update or add a helper (e.g.,
getUserLocale) if you rely on persisted user settings and ensure you reference
the Context variable name (ctx) and the message object when implementing this
assignment.
- Around line 29-51: The two error branches use identical defaultValue text so
missing translations can't distinguish them; update the first ctx.t call (the
!command branch) to use a distinct defaultValue like "Command not found." and
update the second ctx.t call (the !command.supportsPrefix branch) to use a
distinct defaultValue like "Command is disabled." while keeping the same
message.reply usage, allowedMentions, and .then(deleteMessageAfterSent)
behavior; locate the checks for !command and !command.supportsPrefix in the
message handler to apply the change.
In `@src/structures/builder/Builder.ts`:
- Around line 45-47: The toJSON() override in Builder currently returns
super.toJSON() cast to ApplicationJSONBody but omits the custom fields; update
Builder.toJSON() to take the object from super.toJSON(), add the custom
properties prefix_support, slash_support, and aliases from the Builder instance
(or delegate to toClientJSON() if that produces the complete shape), and return
the merged object as ApplicationJSONBody so consumers like
Client.registerCommands() receive the full serialized data.
- Around line 13-16: Remove the redundant empty constructor from the
ApplicationCommandBuilder class: delete the constructor() { super(); } block so
the class simply extends SlashCommandBuilder without defining a constructor
(TypeScript will call the parent constructor automatically); update any
references only if they rely on an explicit constructor (none should).
- Around line 49-55: The toClientJSON method declares a unused parameter client;
either remove the parameter from Builder.toClientJSON and update all call sites
that pass a Client (e.g., places that call cmd.data.toClientJSON(this)) to call
it without an argument, or keep the parameter but mark it as intentionally
unused (rename to _client or add a concise comment above toClientJSON stating
"client parameter reserved for future use / required by registerCommands call")
so linting/CI no longer complains; update the signature for Builder.toClientJSON
and matching call sites (or add the comment) accordingly.
- Around line 21-44: The setters/getters (setAliases, addAliases,
setPrefixSupport, setSlashSupport) are using Reflect.get/Reflect.set
unnecessarily; replace those with direct property access (e.g., this.aliases,
this.prefix_support, this.slash_support) and use the existing
normalizeArray(...) call in addAliases to merge current aliases (default to []
when undefined) with new ones; ensure you keep the same return-this chaining and
type expectations for normalizeArray and existing class properties.
In `@src/structures/builder/Command.ts`:
- Around line 37-41: The constructor currently validates
`#supportsPrefix/`#supportsSlash before handlers are attached, so add a runtime
warning in registerCommands(): for each Command instance, if the configuration
flag (the private flags or the data-derived flag tested by
supportsSlash/supportsPrefix) is true but no handler has been attached via
onInteraction() (for slash) or onMessage() (for prefix), emit a warning (e.g.,
logger.warn or console.warn) naming the Command (data.name) and which handler is
missing; keep existing constructor validation but use registerCommands() to
detect and warn about mismatches between the declared support flags
(`#supportsSlash/`#supportsPrefix or supportsSlash/supportsPrefix) and the
presence of handlers (onInteraction/onMessage).
In `@src/structures/builder/Context.ts`:
- Around line 62-64: The current fallback uses "result === key" which can be a
false positive; instead, call i18n.exists(key, { lng: locale }) to determine
whether the key is resolvable and only return options.defaultValue when
i18n.exists(...) is false (i.e., key missing for the given locale); update the
branch around result, key, options?.defaultValue to use !i18n.exists(key, { lng:
locale }) and ensure i18n and locale are in scope before returning the default.
In `@src/utils/normalizeArray.ts`:
- Around line 7-11: The normalizeArray function incorrectly unwraps when
ItemType itself is an array because it only checks Array.isArray(arr[0]); change
the unwrap guard in normalizeArray (exported function
normalizeArray<ItemType>(arr: RestOrArray<ItemType>)) to require that arr has
exactly one element and that element is an array (e.g., check arr.length === 1
&& Array.isArray(arr[0])) before returning the unwrapped value, so rest-style
calls like [["a"], ["b"]] are not collapsed.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
.github/workflows/release.yml.npmignoreexamples/basic_client/index.jsexamples/basic_client/locales/en-US/error.jsonpackage.jsonscripts/utils/build.jssrc/context.tssrc/events/interaction.tssrc/events/message.tssrc/events/ready.tssrc/index.tssrc/structures/builder/Argument.tssrc/structures/builder/Builder.tssrc/structures/builder/Command.tssrc/structures/builder/Context.tssrc/structures/builder/Event.tssrc/structures/builder/index.tssrc/structures/core/Client.tssrc/utils/normalizeArray.ts
💤 Files with no reviewable changes (1)
- src/structures/builder/Argument.ts
🧰 Additional context used
🧬 Code graph analysis (6)
src/events/interaction.ts (2)
src/structures/builder/Event.ts (1)
EventBuilder(13-64)src/structures/builder/Context.ts (1)
Context(10-89)
src/structures/builder/Builder.ts (2)
src/utils/normalizeArray.ts (1)
normalizeArray(7-12)src/structures/core/Client.ts (1)
Client(27-129)
src/structures/builder/Command.ts (5)
src/structures/builder/Context.ts (1)
Context(10-89)src/structures/core/Client.ts (1)
Client(27-129)types/extra.d.ts (1)
MaybePromise(1-1)src/structures/builder/Builder.ts (1)
ApplicationCommandBuilder(13-56)src/context.ts (1)
currentClient(4-4)
src/events/message.ts (2)
src/structures/builder/Context.ts (1)
Context(10-89)src/utils/util.ts (1)
deleteMessageAfterSent(4-14)
src/structures/builder/Event.ts (2)
src/structures/core/Client.ts (1)
Client(27-129)src/utils/logger/Logger.ts (1)
Logger(11-200)
src/structures/core/Client.ts (4)
src/structures/builder/Command.ts (1)
CommandBuilder(13-87)types/client.d.ts (1)
FrameworkOptions(16-22)src/context.ts (1)
clearClient(10-12)src/utils/Files.ts (2)
getProjectRoot(21-27)getFiles(4-20)
🔇 Additional comments (32)
.npmignore (1)
6-6: LGTM — packaging ignore update is clear.src/events/ready.ts (1)
2-2: Import path update looks safe.scripts/utils/build.js (1)
19-25: LGTM — package.json pruning and minified write are consistent.package.json (3)
22-24: Imports mapping simplification looks fine.
30-32: Build script update is straightforward.
39-46: i18next-fs-backend placement indevDependenciesis correct.This package is only used in
examples/basic_client/index.jsand does not appear in any production source code (src/). It is appropriately placed indevDependenciesand will not causeMODULE_NOT_FOUNDerrors in production installs since the main application code does not import it.Likely an incorrect or invalid review comment.
.github/workflows/release.yml (1)
68-74: No action needed. The scripts do not depend on npm-provided environment variables and will function correctly with directnodeexecution. Bothgithub.jsandnpm.jsaccess only explicitly-passed environment variables (GITHUB_TOKEN,NPM_ORG,GITHUB_SHA) and readpackage.jsonvia Node'srequire()mechanism. Thenpm publishcommands use explicit--registryarguments, so npm's environment configuration is not relied upon.src/structures/builder/index.ts (1)
1-4: LGTM!The barrel export properly includes the new
Buildermodule while maintaining clean re-exports from the builder directory.src/context.ts (1)
1-1: LGTM!Import path correctly updated to relative path, consistent with the PR's refactoring goals.
src/index.ts (1)
1-3: LGTM!Export paths correctly updated to relative paths, aligning with the PR's refactoring goals. The module structure remains clean.
examples/basic_client/index.js (2)
37-42: LGTM!The example correctly demonstrates the new
ApplicationCommandBuilderpattern with fluent API chaining. This aligns well with the builder pattern introduced inBuilder.ts.
12-12: Good addition of "error" namespace.Adding the "error" namespace aligns with the new
error.command.notfoundanderror.command.disabledtranslation keys used in the event handlers.src/structures/builder/Context.ts (5)
2-3: LGTM!Import paths updated to use relative paths, aligning with the refactor.
11-13: LGTM!Using
readonlyforargsanddataimproves immutability guarantees while preserving public access.
28-34: LGTM!Type guard methods correctly narrow the generic type using
instanceofchecks.
36-44: LGTM!The
authorgetter now correctly handles bothMessage(viadata.author) andChatInputCommandInteraction(viadata.user), with a safenullfallback.
69-88: LGTM!The
toJSONmethod correctly bindsthis.tto preserve context when the translation function is extracted.src/structures/builder/Event.ts (5)
3-5: LGTM!Import paths updated to use relative paths consistent with the refactor.
14-17: LGTM!ES private fields (
#handler,#bound) provide true encapsulation, preventing external access and modification.
19-29: LGTM!The
#listenercorrectly guards against missing handler and properly propagates errors to the logger without crashing the event loop.
46-57: LGTM!The
#registermethod correctly guards against double registration via#boundand chooses betweenonceandonbased on the event configuration.
59-63: LGTM!The fluent API pattern with
onExecutereturningthisallows method chaining.src/structures/builder/Command.ts (4)
1-6: LGTM!Imports updated to use the new
ApplicationCommandBuilderand relative paths.
16-26: LGTM!ES private fields for support flags with getters that require both the flag AND the handler to be set. This ensures a command is only considered to support slash/prefix if it's both configured for it and has the corresponding handler.
43-69: LGTM!Comprehensive validation for command name and alias conflicts, preventing duplicate registrations and alias collisions.
71-75: LGTM!Command and aliases are registered correctly, with debug logging for observability.
src/structures/core/Client.ts (6)
8-18: LGTM!Import paths updated to use relative paths and re-exports from index files.
30-34: LGTM!Field declarations updated with appropriate visibility. The
readonlymodifier onloggerandprefixensures immutability.
40-41: LGTM!Changed from
lodash.mergeto spread operator. SincedefaultOptscontains only primitives and a flat array, shallow merging is sufficient here.
74-83: LGTM!Private method
#loadDirproperly checks directory existence and iterates through files.
85-95: LGTM!Private method
#loadFileproperly clears require cache, sets/clears the client context, and handles errors gracefully.
109-111: LGTM!Command serialization now uses the builder's
toClientJSONmethod, aligning with the newApplicationCommandBuilderpattern.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| { | ||
| "command.notfound": "Command not found or disabled1", | ||
| "command.disabled": "Command not found or disabled2" | ||
| } |
There was a problem hiding this comment.
Fix placeholder digits in user-facing strings.
The trailing “1/2” looks like accidental placeholders and will surface in UI.
✅ Suggested correction
{
- "command.notfound": "Command not found or disabled1",
- "command.disabled": "Command not found or disabled2"
+ "command.notfound": "Command not found.",
+ "command.disabled": "Command is disabled."
}🤖 Prompt for AI Agents
In `@examples/basic_client/locales/en-US/error.json` around lines 1 - 4, The two
locale entries "command.notfound" and "command.disabled" contain accidental
trailing digits; update their values to user-facing text without placeholders
(e.g., change "Command not found or disabled1" and "Command not found or
disabled2" to a clean message such as "Command not found or disabled") so the UI
shows the correct string for both keys.
| if (!command) { | ||
| await interaction.reply({ | ||
| content: "Command not found or disabled.", | ||
| content: ctx.t("error.command.notfound", { | ||
| defaultValue: "Command not found or disabled.", | ||
| }), | ||
| flags: MessageFlags.Ephemeral, | ||
| }); | ||
| return; | ||
| } | ||
| if (!command.supportsSlash) { | ||
| await interaction.reply({ | ||
| content: ctx.t("error.command.disabled", { | ||
| defaultValue: "Command not found or disabled.", | ||
| }), | ||
| flags: MessageFlags.Ephemeral, | ||
| }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Default values should be distinct for different error states.
Same issue as in message.ts: both error.command.notfound and error.command.disabled use identical defaultValue. Additionally, ctx.locale is set at line 30, after these checks, so error messages won't use the user's locale.
💡 Proposed fix
const ctx = new Context(context.client, { interaction });
+ ctx.locale = interaction.locale;
if (!command) {
await interaction.reply({
content: ctx.t("error.command.notfound", {
- defaultValue: "Command not found or disabled.",
+ defaultValue: "Command not found.",
}),
flags: MessageFlags.Ephemeral,
});
return;
}
if (!command.supportsSlash) {
await interaction.reply({
content: ctx.t("error.command.disabled", {
- defaultValue: "Command not found or disabled.",
+ defaultValue: "This command is not available as a slash command.",
}),
flags: MessageFlags.Ephemeral,
});
return;
}
try {
- ctx.locale = interaction.locale;
context.logger.debug(🤖 Prompt for AI Agents
In `@src/events/interaction.ts` around lines 11 - 28, Move the locale
initialization (set ctx.locale) to run before the command existence and
supportsSlash checks, and update the two ctx.t calls inside the interaction
handler so their defaultValue strings are distinct (e.g., "Command not found."
for error.command.notfound and "Slash command disabled." for
error.command.disabled) so users see different fallback messages; modify the
interaction.reply calls that reference ctx.t for these keys and ensure they
execute after ctx.locale is set.
| const commandAlias = context.client.aliases.findKey((cmd) => | ||
| cmd.has(commandName) | ||
| ); | ||
| const ctx = new Context(context.client, { message, args }); |
There was a problem hiding this comment.
Missing locale assignment for message context.
In interaction.ts (line 30), ctx.locale is set to interaction.locale before using the context. Here, ctx.locale remains undefined, causing ctx.t() to always fall back to the default language. Consider deriving locale from the guild or user preferences if available.
🤖 Prompt for AI Agents
In `@src/events/message.ts` at line 25, The Context created in message handler
(new Context(context.client, { message, args })) never has ctx.locale set, so
ctx.t() falls back to default; after creating ctx assign ctx.locale using
available sources such as message.guild?.preferredLocale or a stored user
preference, e.g., ctx.locale = message.guild?.preferredLocale ??
getUserLocale(message.author.id) ?? context.client.config.defaultLocale; update
or add a helper (e.g., getUserLocale) if you rely on persisted user settings and
ensure you reference the Context variable name (ctx) and the message object when
implementing this assignment.
| if (!command) { | ||
| await message | ||
| .reply({ | ||
| content: ctx.t("error.command.notfound", { | ||
| defaultValue: "Command not found or disabled.", | ||
| }), | ||
| allowedMentions: { repliedUser: false }, | ||
| }) | ||
| .then(deleteMessageAfterSent); | ||
| return; | ||
| } | ||
|
|
||
| if (!command.supportsPrefix) { | ||
| await message | ||
| .reply({ | ||
| content: "Command not found or disabled.", | ||
| content: ctx.t("error.command.disabled", { | ||
| defaultValue: "Command not found or disabled.", | ||
| }), | ||
| allowedMentions: { repliedUser: false }, | ||
| }) | ||
| .then(deleteMessageAfterSent); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Default values should be distinct for different error states.
Both error.command.notfound and error.command.disabled use the same defaultValue: "Command not found or disabled.". This makes it impossible to distinguish between the two error conditions when translations are missing. Consider using distinct messages.
💡 Proposed fix with distinct messages
if (!command) {
await message
.reply({
content: ctx.t("error.command.notfound", {
- defaultValue: "Command not found or disabled.",
+ defaultValue: "Command not found.",
}),
allowedMentions: { repliedUser: false },
})
.then(deleteMessageAfterSent);
return;
}
if (!command.supportsPrefix) {
await message
.reply({
content: ctx.t("error.command.disabled", {
- defaultValue: "Command not found or disabled.",
+ defaultValue: "This command is not available for prefix usage.",
}),
allowedMentions: { repliedUser: false },
})
.then(deleteMessageAfterSent);
return;
}🤖 Prompt for AI Agents
In `@src/events/message.ts` around lines 29 - 51, The two error branches use
identical defaultValue text so missing translations can't distinguish them;
update the first ctx.t call (the !command branch) to use a distinct defaultValue
like "Command not found." and update the second ctx.t call (the
!command.supportsPrefix branch) to use a distinct defaultValue like "Command is
disabled." while keeping the same message.reply usage, allowedMentions, and
.then(deleteMessageAfterSent) behavior; locate the checks for !command and
!command.supportsPrefix in the message handler to apply the change.
| export class ApplicationCommandBuilder extends SlashCommandBuilder { | ||
| constructor() { | ||
| super(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Redundant constructor can be removed.
The constructor only calls super() with no additional logic. TypeScript automatically calls the parent constructor when none is defined.
🔧 Proposed fix
export class ApplicationCommandBuilder extends SlashCommandBuilder {
- constructor() {
- super();
- }
protected prefix_support: boolean = true;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export class ApplicationCommandBuilder extends SlashCommandBuilder { | |
| constructor() { | |
| super(); | |
| } | |
| export class ApplicationCommandBuilder extends SlashCommandBuilder { | |
| protected prefix_support: boolean = true; |
🤖 Prompt for AI Agents
In `@src/structures/builder/Builder.ts` around lines 13 - 16, Remove the redundant
empty constructor from the ApplicationCommandBuilder class: delete the
constructor() { super(); } block so the class simply extends SlashCommandBuilder
without defining a constructor (TypeScript will call the parent constructor
automatically); update any references only if they rely on an explicit
constructor (none should).
| override toJSON(): ApplicationJSONBody { | ||
| return super.toJSON() as ApplicationJSONBody; | ||
| } |
There was a problem hiding this comment.
Critical: toJSON() does not include custom fields.
The override casts super.toJSON() to ApplicationJSONBody, but the custom fields (prefix_support, slash_support, aliases) are never added to the output. This means the serialized JSON will not contain these properties, breaking downstream consumers (e.g., Client.registerCommands() which expects toClientJSON() to return complete data).
🐛 Proposed fix to include custom fields
override toJSON(): ApplicationJSONBody {
- return super.toJSON() as ApplicationJSONBody;
+ return {
+ ...super.toJSON(),
+ prefix_support: this.prefix_support,
+ slash_support: this.slash_support,
+ aliases: this.aliases,
+ };
}🤖 Prompt for AI Agents
In `@src/structures/builder/Builder.ts` around lines 45 - 47, The toJSON()
override in Builder currently returns super.toJSON() cast to ApplicationJSONBody
but omits the custom fields; update Builder.toJSON() to take the object from
super.toJSON(), add the custom properties prefix_support, slash_support, and
aliases from the Builder instance (or delegate to toClientJSON() if that
produces the complete shape), and return the merged object as
ApplicationJSONBody so consumers like Client.registerCommands() receive the full
serialized data.
| if (!this.#supportsPrefix && !this.#supportsSlash) { | ||
| throw new Error( | ||
| `Command ${this.name} must support either slash or prefix commands.` | ||
| `Command ${data.name} must support either slash or prefix commands.` | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Validation occurs before handlers are attached.
The validation checks #supportsPrefix and #supportsSlash flags, but the actual supportsSlash/supportsPrefix getters also require handlers to be set. A command configured with slash_support: true but where onInteraction() is never called will pass this validation but won't actually register as a slash command. Consider adding a warning in registerCommands() if a command has the flag set but no handler.
🤖 Prompt for AI Agents
In `@src/structures/builder/Command.ts` around lines 37 - 41, The constructor
currently validates `#supportsPrefix/`#supportsSlash before handlers are attached,
so add a runtime warning in registerCommands(): for each Command instance, if
the configuration flag (the private flags or the data-derived flag tested by
supportsSlash/supportsPrefix) is true but no handler has been attached via
onInteraction() (for slash) or onMessage() (for prefix), emit a warning (e.g.,
logger.warn or console.warn) naming the Command (data.name) and which handler is
missing; keep existing constructor validation but use registerCommands() to
detect and warn about mismatches between the declared support flags
(`#supportsSlash/`#supportsPrefix or supportsSlash/supportsPrefix) and the
presence of handlers (onInteraction/onMessage).
| if (result === key && options?.defaultValue) { | ||
| return options.defaultValue; | ||
| } |
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
# First, let's examine the Context.ts file around lines 62-64
cat -n src/structures/builder/Context.ts | sed -n '50,75p'Repository: AroxBot/framework
Repository: AroxBot/framework
Exit code: 0
stdout:
50
51 const locale =
52 this.locale ??
53 (Array.isArray(this.client.i18n.options.fallbackLng)
54 ? this.client.i18n.options.fallbackLng[0]
55 : this.client.i18n.options.fallbackLng) ??
56 "en";
57
58 const t = this.client.i18n.getFixedT(locale);
59
60 const result = t(key, options);
61
62 if (result === key && options?.defaultValue) {
63 return options.defaultValue;
64 }
65
66 return result;
67 }
68
69 toJSON() {
70 const { data, args, author } = this;
71
72 if (this.isInteraction()) {
73 return {
74 kind: "interaction" as const,
75 interaction: data,
i18next check if translation key exists i18n.exists method
Use i18next.exists(key, options?) (aka i18n.exists)—it uses the same key resolution logic as t() and returns a boolean indicating whether the key can be resolved. [1][2]
Basic (i18next)
import i18next from "i18next";
if (i18next.exists("common.values.active")) {
// key exists
}[1]
With react-i18next
import { useTranslation } from "react-i18next";
function Comp() {
const { i18n } = useTranslation();
const ok = i18n.exists("common.values.active");
}[3][4]
Notes
optionsare the same ast()(e.g.,lng,ns,context,count, etc.), so existence checks respect namespaces/language overrides the same way translations do. [1][2]
i18next returnedObjectHandler configuration
returnedObjectHandler is an i18next option that is invoked when a translation resolves to an object/array but returnObjects is false (the default). It lets you decide what to do in that situation (e.g., warn, throw, or convert the object to a string). By default it’s a no-op. [1]
Configure globally
i18next.init({
returnObjects: false, // default
returnedObjectHandler: (key, value, options) => {
// key: the resolved key
// value: the resolved object/array
// options: t() options
console.warn(`i18next: "${key}" returned an object; enable returnObjects to use it.`);
return ''; // or JSON.stringify(value), or throw new Error(...)
}
});(Handler signature: function(key, value, options) {}) [1]
Recommended alternative (if you actually want the object)
Enable object returns instead of handling the fallback:
i18next.t('tree', { returnObjects: true });or globally:
i18next.init({ returnObjects: true });[1][2]
Sources: i18next configuration options (includes returnedObjectHandler, returnObjects) [1], Objects & Arrays docs (shows returnObjects: true usage) [2].
Use i18n.exists() to reliably check if a translation key has a value.
The fallback check result === key is unreliable because a translation file could legitimately map a key to itself, producing a false positive and skipping the default value when one should be used. Instead, use i18n.exists(key, { lng: locale }) to explicitly verify whether the translation key is resolvable before deciding whether to apply the default value.
🤖 Prompt for AI Agents
In `@src/structures/builder/Context.ts` around lines 62 - 64, The current fallback
uses "result === key" which can be a false positive; instead, call
i18n.exists(key, { lng: locale }) to determine whether the key is resolvable and
only return options.defaultValue when i18n.exists(...) is false (i.e., key
missing for the given locale); update the branch around result, key,
options?.defaultValue to use !i18n.exists(key, { lng: locale }) and ensure i18n
and locale are in scope before returning the default.
| export function normalizeArray<ItemType>( | ||
| arr: RestOrArray<ItemType> | ||
| ): ItemType[] { | ||
| if (Array.isArray(arr[0])) return [...arr[0]]; | ||
| return arr as ItemType[]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the complete normalizeArray implementation
cat -n src/utils/normalizeArray.tsRepository: AroxBot/framework
Length of output: 897
🏁 Script executed:
# Search for RestOrArray type definition
rg -n "RestOrArray" src --type tsRepository: AroxBot/framework
Length of output: 303
🏁 Script executed:
# Find all usages of normalizeArray
rg -n "normalizeArray" src --type ts -B 2 -A 2Repository: AroxBot/framework
Length of output: 1675
🏁 Script executed:
# Check for any actual usage patterns with array ItemTypes
rg -n "normalizeArray" src --type ts -B 5 -A 5Repository: AroxBot/framework
Length of output: 3269
🏁 Script executed:
# Search for any complex types being used with normalizeArray
ast-grep --pattern 'normalizeArray($_)'Repository: AroxBot/framework
Length of output: 254
🏁 Script executed:
# Search for other uses of RestOrArray or normalizeArray beyond what we've seen
rg -n "RestOrArray|normalizeArray" src --type ts | grep -v "normalizeArray.ts"Repository: AroxBot/framework
Length of output: 351
🏁 Script executed:
# Check if there are any type definitions or interfaces that might use array types with normalizeArray
rg -n "RestOrArray" src --type ts -B 3 -A 3Repository: AroxBot/framework
Length of output: 1116
Guard against unintended unwrapping when ItemType is an array type.
The RestOrArray<Type> type permits both rest parameters (Type[]) and wrapped arrays ([Type[]]). When ItemType itself is an array, the unwrap condition can drop a nesting level: if ItemType = string[] and arr = [["a"], ["b"]] (rest params), arr[0] is an array so the code returns [["a"]] instead of [["a"], ["b"]]. The suggested check prevents this:
🔧 Safer unwrap condition
export function normalizeArray<ItemType>(
arr: RestOrArray<ItemType>
): ItemType[] {
- if (Array.isArray(arr[0])) return [...arr[0]];
+ if (arr.length === 1 && Array.isArray(arr[0])) return [...arr[0]];
return arr as ItemType[];
}Current usage is only with string types, so this does not affect existing code. Apply the fix to prevent future edge cases.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function normalizeArray<ItemType>( | |
| arr: RestOrArray<ItemType> | |
| ): ItemType[] { | |
| if (Array.isArray(arr[0])) return [...arr[0]]; | |
| return arr as ItemType[]; | |
| export function normalizeArray<ItemType>( | |
| arr: RestOrArray<ItemType> | |
| ): ItemType[] { | |
| if (arr.length === 1 && Array.isArray(arr[0])) return [...arr[0]]; | |
| return arr as ItemType[]; | |
| } |
🤖 Prompt for AI Agents
In `@src/utils/normalizeArray.ts` around lines 7 - 11, The normalizeArray function
incorrectly unwraps when ItemType itself is an array because it only checks
Array.isArray(arr[0]); change the unwrap guard in normalizeArray (exported
function normalizeArray<ItemType>(arr: RestOrArray<ItemType>)) to require that
arr has exactly one element and that element is an array (e.g., check arr.length
=== 1 && Array.isArray(arr[0])) before returning the unwrapped value, so
rest-style calls like [["a"], ["b"]] are not collapsed.
1 participant
Summary by CodeRabbit
New Features
Refactor