refactor: implement class-based structure with hybrid command support #2
Conversation
📝 WalkthroughWalkthroughRemoves the legacy BaseClient and adds a new framework: a Client extending discord.js, Command/Event/Argument/Context structures, file loading and command registration, runtime event handlers, a configurable Logger, utilities, types, examples, and build/config updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DiscordJS as "Discord.js"
participant Client
participant Registry as "context.currentClient"
participant Command as "CommandBuilder"
participant Context
participant Logger
participant REST as "Discord REST"
User->>DiscordJS: Send message / interaction
DiscordJS->>Client: Emit MessageCreate / InteractionCreate
Client->>Registry: set currentClient
Client->>Client: lookup command by name/alias
Client->>Command: check supportsPrefix/supportsSlash
alt command found and supported
Client->>Context: create Context(payload)
Context->>Logger: log execution
Client->>Command: invoke onMessage/onInteraction with Context
Command->>User: respond
else not found or unsupported
Client->>User: reply not found/disabled
end
opt registering slash commands
Client->>REST: upsert application commands
REST-->>Client: ack / error
Client->>Logger: log registration result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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: 5
🤖 Fix all issues with AI agents
In `@src/structures/Client.ts`:
- Around line 112-136: registerCommands currently silently returns when
this.token or this.application are missing; update registerCommands to surface
this failure by checking these prerequisites and logging a clear warning or
error (e.g., this.logger.warn or this.logger.error) that includes which value is
missing (this.token and/or this.application) and then return or throw as
appropriate; modify the early-exit path in registerCommands so callers and logs
can detect why commands were not registered.
- Around line 98-110: The getFiles method is including TypeScript declaration
files because file.endsWith(".ts") matches ".d.ts"; update the file-filtering
logic in getFiles to skip files that end with ".d.ts" (e.g., require .ts files
but exclude those matching ".d.ts") so only runtime .ts/.js sources are pushed
to fileList and declaration files are ignored.
- Around line 138-159: The catch block in handleMessage currently awaits
message.reply which can itself reject and escape the handler; update the catch
to guard the reply by either wrapping the message.reply call in its own
try/catch or by chaining .catch(...) on the promise so any reply failure is
logged via this.logger.error without throwing, e.g. keep logging the original
execution error with this.logger.error (using command.name) and then call
message.reply(...).catch(err => this.logger.error(`Failed to send error reply
for ${command?.name}`, err)) to ensure no unhandled rejections from
message.reply.
- Around line 161-190: The handler currently uses interaction.isCommand() which
allows non-chat-input interactions; update handleInteraction to guard with
interaction.isChatInputCommand() so it only processes slash/chat-input commands,
and ensure the interaction passed into new Context(this, { interaction }) is
typed/treated as a ChatInputCommandInteraction (replace checks/usages relying on
isCommand with isChatInputCommand and adjust any type narrowing if needed);
reference symbols: handleInteraction, Interaction, isCommand,
isChatInputCommand, Context, CommandInteraction/ChatInputCommandInteraction.
In `@src/structures/Context.ts`:
- Around line 23-58: The Context class can be constructed with neither
interaction nor message leading to runtime ! dereferences; fix by changing the
constructor signature to accept a discriminated union (e.g. data: { interaction:
CommandInteraction; args?: string[] } | { message: Message; args?: string[] })
and add a runtime guard at the start of the constructor that throws a
descriptive Error if both are missing; update the class properties to reflect
the union (allow undefined) and remove non-null assertions in the getters
reply/author/guild/channel by either returning nullable types or explicitly
throwing when neither interaction nor message is present so the getters (author,
guild, channel) and reply handle the validated state safely.
📜 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 (8)
src/classes/base/BaseClient.tssrc/index.tssrc/structures/Argument.tssrc/structures/Client.tssrc/structures/Command.tssrc/structures/Context.tssrc/structures/Event.tssrc/utils/Logger.ts
💤 Files with no reviewable changes (1)
- src/classes/base/BaseClient.ts
🧰 Additional context used
🧬 Code graph analysis (4)
src/structures/Command.ts (3)
src/structures/Argument.ts (1)
Argument(6-36)src/structures/Client.ts (1)
Client(23-191)src/structures/Context.ts (1)
Context(17-64)
src/structures/Event.ts (1)
src/structures/Client.ts (1)
Client(23-191)
src/structures/Client.ts (2)
src/utils/Logger.ts (3)
LogLevel(2-2)LoggerInstance(11-45)error(36-39)src/structures/Context.ts (1)
Context(17-64)
src/structures/Context.ts (1)
src/structures/Client.ts (1)
Client(23-191)
🔇 Additional comments (8)
src/structures/Argument.ts (1)
1-35: Looks good. The Argument wrapper cleanly normalizes option data and defaultsrequiredsafely.src/index.ts (1)
1-6: LGTM. Public API exports look consistent with the new structure.src/structures/Command.ts (1)
1-43: Solid base class. Normalization ofArgumentinstances is clean and predictable.src/structures/Event.ts (1)
1-16: Looks good. The generic event contract is minimal and well-typed.src/structures/Client.ts (4)
17-27: Public framework options look coherent.Optional
logLevel,prefix, andtokenplus readonly collections provide a clear API surface.
47-71: Command loader flow looks good.Cache-busting, type-guarding, and alias mapping are clear and easy to follow.
73-96: Event loader wiring looks correct.
oncevsonhandling is tidy and consistent with expected event lifecycles.
29-45: No action needed—module system is CommonJS.The project is explicitly configured for CommonJS with
"type": "commonjs"in package.json and"module": "CommonJS"in tsconfig.json. In this CommonJS context,__dirnameis a standard global variable and is fully available. The code uses it correctly without any compatibility issues.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Co-authored-by: Fhyrox <fhyrox@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Fix all issues with AI agents
In `@examples/basic_client/index.js`:
- Around line 5-12: The example uses the old Command callback signature; update
the arox.Command constructor's callback to the current API by replacing the
parameters (interaction, message, args) with a single (ctx: Context) parameter
and use ctx.interaction and ctx.message (and ctx.args if needed) inside the
handler; modify the anonymous function passed to arox.Command accordingly so it
matches the Context-based execute signature.
In `@src/context.ts`:
- Around line 3-7: The module-global mutable currentClient and setClient allow
only one Client and will be overwritten; replace this pattern by introducing a
per-instance registry (e.g., a WeakMap or Map) keyed by owner/context or an
explicit clientId and update the API accordingly: stop exporting a single
currentClient, add functions like registerClient(client, key) and getClient(key)
(or accept Client as an explicit parameter where used) and migrate callers that
use currentClient/setClient to use the new register/get or explicit-passing
approach so multiple Client instances can coexist without clobbering each other
(refer to currentClient and setClient to locate the code to change).
In `@src/handler/message.ts`:
- Around line 9-11: The code currently coerces this.client.options.prefix to a
string and calls message.content.startsWith(prefix), which causes false/other
non-string prefixes to become strings and incorrectly match; update the guard to
first verify that this.client.options.prefix is a non-empty string (e.g., typeof
prefix === 'string' && prefix.length > 0) and return early if not before calling
message.content.startsWith; reference the symbols this.client.options.prefix and
message.content.startsWith in handler/message.ts when implementing the check.
- Around line 12-31: The message handler currently rejects commands based solely
on supportsSlash and never forwards parsed args; update the command-check to
allow prefix-capable commands by testing a prefix flag (e.g., supportsPrefix or
!supportsSlash) instead of only supportsSlash, and instantiate the Context with
the parsed args (pass args into new Context(this.client, { message, args }))
before calling command.execute; locate the logic around commandAlias/command
retrieval and the try block where new Context is created and command.execute is
invoked to apply this change.
In `@src/structures/Client.ts`:
- Around line 47-49: The Client constructor currently only requires
"../handler/interaction" and thus never registers the MessageCreate listener;
update the constructor (around setClient(this) and the
require("../handler/interaction") call) to conditionally require the message
handler (e.g., require("../handler/message")) when a prefix is configured on the
client (check the Client's prefix or config property) so prefix-based commands
are registered; keep the interaction handler require as-is and only add the
conditional require for the message handler to enable hybrid command support.
- Around line 32-34: The constructor in the Client class mutates the
module-level defaultOpts via lodash.merge; change the merge call in the Client
constructor (constructor(opts: FrameworkOptions)) to merge into a fresh target
so defaultOpts isn't mutated — e.g., call merge({}, defaultOpts, opts) (or
equivalent non-mutating merge) before passing the result to super, ensuring each
Client instance gets its own merged options without option bleed.
In `@src/structures/Command.ts`:
- Around line 43-44: The constructor sets this.supportsSlash and
this.supportsPrefix from options but allows both to be false, making commands
uncallable; update the Command constructor to validate these flags
(supportsSlash and supportsPrefix) and either default one to true or throw a
clear error when both are false (e.g., check options.slash and options.prefix or
the resolved this.supports* values), so any Command instance always has at least
one execution path; include the check early in the constructor and surface a
descriptive error mentioning the command name or identifier if validation fails.
- Around line 46-52: Remove the process.nextTick wrapper and perform the command
registration synchronously: replace the process.nextTick(...) block so that the
Map checks and sets on this.client.commands and this.client.aliases happen
immediately (so duplicate-check throw for `this.name` occurs synchronously and
can be caught). Ensure you still check this.client.commands.has(this.name) and
throw the same Error if present, then call this.client.commands.set(this.name,
this) and this.client.aliases.set(this.name, new Set(this.aliases)) directly in
the constructor/initializer instead of inside process.nextTick.
In `@src/structures/Event.ts`:
- Around line 12-34: The Event constructor's listener currently invokes
this.execute(...args) without handling rejected promises; update the listener
inside the process.nextTick callback (the anonymous "listener" function created
for Event<K>) to call Promise.resolve(this.execute(...(args as EventArgs<K>)))
and attach a .catch(err => this.client.logger.error(...)) to surface errors via
the existing client.logger; ensure you preserve the correct this binding (use
the surrounding this or bind appropriately) and include contextual information
(event name / this.constructor.name) in the error log.
In `@src/utils/Files.ts`:
- Around line 4-9: getFiles currently uses FastGlob.sync(["**/*.ts","**/*.js"],
{ cwd: baseDir, ... }) which can match both source and built outputs when
baseDir is repo root; update getFiles to exclude common build/output folders
(e.g., "dist/**","lib/**","out/**","build/**",".next/**","coverage/**") by
adding them to the ignore array passed to FastGlob.sync (or alternatively
require callers to pass a narrower baseDir) so duplicate command/event
registrations are prevented; locate the getFiles function and modify the
FastGlob.sync call's ignore option accordingly.
In `@types/client.d.ts`:
- Around line 1-16: FrameworkOptions references an unresolved type LogLevel; add
a declaration or import so the emitted .d.ts contains the symbol. Open the types
file and either (A) add an explicit exported declaration (e.g., export type
LogLevel = ... or export enum LogLevel) above FrameworkOptions, or (B) add an
`import type { LogLevel } from "..."` line that points to the module that
defines LogLevel and use that imported type; ensure the chosen import path is
valid for declaration emission and update FrameworkOptions to use the
now-resolved LogLevel.
📜 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 (14)
examples/basic_client/index.jspackage.jsonsrc/context.tssrc/handler/interaction.tssrc/handler/message.tssrc/structures/Client.tssrc/structures/Command.tssrc/structures/Context.tssrc/structures/Event.tssrc/utils/Files.tssrc/utils/util.tstsconfig.jsontypes/client.d.tstypes/extra.d.ts
🧰 Additional context used
🧬 Code graph analysis (8)
src/context.ts (2)
src/structures/Client.ts (1)
Client(24-103)examples/basic_client/index.js (1)
client(3-3)
types/client.d.ts (2)
src/utils/util.ts (1)
PrefixOptions(1-4)src/utils/Logger.ts (1)
LogLevel(2-2)
src/handler/interaction.ts (2)
src/structures/Event.ts (1)
Event(9-36)src/structures/Context.ts (1)
Context(17-65)
src/structures/Client.ts (5)
types/client.d.ts (2)
FrameworkOptions(13-17)PrefixOptions(8-11)src/utils/Logger.ts (2)
LoggerInstance(11-45)error(36-39)src/utils/util.ts (2)
getPrefix(6-18)PrefixOptions(1-4)src/utils/Files.ts (1)
getFiles(4-10)src/context.ts (1)
setClient(6-8)
src/handler/message.ts (3)
src/structures/Event.ts (1)
Event(9-36)src/structures/Context.ts (1)
Context(17-65)src/utils/Logger.ts (1)
error(36-39)
src/structures/Command.ts (5)
src/structures/Argument.ts (1)
Argument(6-36)src/structures/Client.ts (1)
Client(24-103)src/structures/Context.ts (1)
Context(17-65)types/extra.d.ts (1)
MaybePromise(1-1)src/context.ts (1)
currentClient(4-4)
src/structures/Context.ts (2)
src/structures/Client.ts (1)
Client(24-103)examples/basic_client/index.js (1)
client(3-3)
src/structures/Event.ts (3)
src/structures/Client.ts (1)
Client(24-103)types/extra.d.ts (1)
MaybePromise(1-1)src/context.ts (1)
currentClient(4-4)
🔇 Additional comments (8)
types/extra.d.ts (1)
1-1: LGTM — helpful async/sync alias.package.json (1)
22-44: LGTM — dependency and imports updates align with new utilities.src/utils/util.ts (1)
1-17: LGTM — clear prefix resolution logic.tsconfig.json (1)
8-9: NodeNext configuration with CommonJS type is valid and does not require changes.
The tsconfig.json settings are compatible with"type": "commonjs"in package.json. TypeScript will treat.tsfiles as CommonJS modules, and the configuration includes appropriate compatibility flags (allowSyntheticDefaultImportsandesModuleInterop). No problematic extensionless relative imports were found in the codebase that would break under NodeNext resolution rules.src/handler/interaction.ts (1)
5-38: LGTM — clear slash-command flow with Context + error feedback.The handler properly narrows to chat-input interactions, validates command availability, and provides error feedback on failure.
src/structures/Context.ts (1)
23-64: LGTM — clean unified Context with guarded construction.The discriminated-union constructor and reply/follow-up handling look solid.
src/structures/Event.ts (1)
6-7: EventArgs typing looks good.Clear mapping of known
ClientEventsto argument tuples.src/structures/Command.ts (1)
8-15: CommandOptions interface is clear.Good coverage for aliases, options, and hybrid flags.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/structures/Client.ts`:
- Around line 39-41: The assignment is overwriting this.options.prefix (typed as
PrefixOptions) with a value from getPrefix() that can be string | false; stop
casting and creating a type mismatch by introducing a new normalized field
(e.g., prefixNormalized or normalizedPrefix: string | false) on the client
instance, keep this.options.prefix as the original PrefixOptions, set the new
field from getPrefix(this.options.prefix ?? { enabled: false }), and update the
message-handler and any other logic that currently reads this.options.prefix
(including the other occurrence around the same area) to read the new normalized
field instead; reference getPrefix, this.options.prefix, and the message-handler
usage to locate and change all affected spots.
♻️ Duplicate comments (1)
src/structures/Client.ts (1)
34-35: Avoid mutatingdefaultOptsduring merge.
merge(defaultOpts, opts)mutates the module-level object, so laterClientinstances inherit prior instance overrides. Use a fresh target to prevent option bleed.🛠️ Proposed fix
- super(merge(defaultOpts, opts) as FrameworkOptions); + super(merge({}, defaultOpts, opts) as FrameworkOptions);
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
src/structures/Client.tssrc/structures/Context.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/structures/Client.ts (5)
types/client.d.ts (2)
FrameworkOptions(13-17)PrefixOptions(8-11)src/utils/Logger.ts (2)
LoggerInstance(11-45)error(36-39)src/utils/util.ts (2)
getPrefix(6-18)PrefixOptions(1-4)src/utils/Files.ts (2)
getProjectRoot(11-17)getFiles(4-10)src/context.ts (1)
setClient(6-8)
🔇 Additional comments (5)
src/structures/Client.ts (3)
54-59: LGTM: deterministic file loading order.
61-69: LGTM: cache bust + error logging are solid.
72-105: LGTM: command registration flow and logging are clear.src/structures/Context.ts (2)
22-35: LGTM: constructor guard and discriminated union are solid.
37-47: LGTM: getters provide safe fallbacks.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In `@examples/basic_client/events/ready.js`:
- Around line 3-7: The EventBuilder is being called with the handler as the
second argument, which the constructor interprets as the `once` flag, so the
callback isn't registered; change the constructor call to pass a boolean false
as the second parameter (e.g., EventBuilder("clientReady", false, handler)) or
use the EventBuilder.onExecute(...) method to register the handler instead so
the function provided is actually set as the event callback.
In `@src/handler/ready.ts`:
- Around line 3-7: The EventBuilder handler for "clientReady" checks a
misspelled option name; change the conditional from
this.client.options.autoRegiserCommands to the correctly spelled
this.client.options.autoRegisterCommands to match the updated types, keeping the
call to this.client.registerCommands() intact; update any references in the same
block (EventBuilder("clientReady"), onExecute) to use autoRegisterCommands
consistently.
In `@src/structures/Client.ts`:
- Line 31: The Client class's public property prefix is typed as `string |
boolean` but the utility `getPrefix` returns `string | false`, causing a
mismatch and allowing `true` incorrectly; change the type of the `prefix`
property on the `Client` class (symbol: prefix) to `string | false` to match the
`getPrefix` return type (symbol: getPrefix in src/utils/util.ts) and update any
usages that assume a boolean `true` to handle `false` explicitly.
- Around line 103-105: The success message currently uses this.logger.warn for
the "Loaded ${slashCommands.length} application (/) commands." log; change the
call to this.logger.log (or this.logger.debug to match lines 97–98) so the
message uses an appropriate info/debug level instead of warn, and update any
identical occurrences of the same message if present; target the
this.logger.warn invocation in Client.ts that formats the slashCommands.length
string.
- Around line 17-23: There is a misspelled property name
"autoRegiserCommands"—update all occurrences to "autoRegisterCommands": change
the key in the defaultOpts object (symbol: defaultOpts) to autoRegisterCommands,
update the corresponding property in the FrameworkOptions/type declaration
(symbol: FrameworkOptions or the interface in types/client.d.ts), and update any
usages/destructuring that reference autoRegiserCommands (e.g., in ready handler
code that reads this flag) to use autoRegisterCommands so the property name is
consistent across defaultOpts, the type definition, and the ready handler.
In `@src/structures/Command.ts`:
- Around line 36-58: The debug log in the Command constructor currently uses
__filename which always points to Command.ts; update the constructor to either
accept an optional source file path from CommandOptions (e.g.,
options.sourceFile or options.filePath) and use that when calling
this.logger.debug(`Loaded Command ${this.name}(${sourceFile})`), or remove the
filename from the log entirely and log only the command name; also update the
CommandOptions type to include the new optional sourceFile/filePath property and
ensure any call-sites that instantiate new Command pass the source file if
available so logs reflect the real command file.
In `@src/structures/Event.ts`:
- Line 24: The log is using __filename (always Event.ts) instead of the actual
event file path; update the Event class to log the instance's stored file path
property (e.g., replace __filename with the property you store the event file
under such as this.file, this.filePath, or this.path) in the logger.debug call
(the line with logger.debug(`Loaded Event ${this.name}(${__filename})`)); if the
Event constructor currently does not save the incoming file/path argument,
modify the constructor to accept and assign that file path to the chosen
property so logger.debug can reference the real event file.
In `@src/utils/util.ts`:
- Around line 15-27: The getPrefix function has a redundant runtime check "opts
&& typeof opts === 'object'"; simplify by relying on TypeScript's PrefixOptions
narrowing: keep the initial typeof opts === 'string' branch, then use the
narrowed object form (e.g., directly check opts.enabled or use the 'enabled' in
opts guard) to return opts.prefix when enabled, otherwise return false; update
the logic in getPrefix to remove the extra typeof/object truthiness check while
preserving the same return behavior.
- Around line 4-14: The deleteMessage function currently swallows all errors via
`.catch()`, losing visibility into failures; update deleteMessage (and its
return type) to report success/failure and surface/log errors: call
message.delete().then(() => r(true)).catch(err => { /* log via console.warn or
existing logger with context mentioning deleteMessage and the
message/interaction type/id */ r(false) }); and update the Promise<void>
signature to Promise<boolean> (or a Result type) so callers can react to
permission or already-deleted errors instead of them being ignored. Ensure you
reference the Message | InteractionResponse types in the signature and keep the
timeout behavior.
In `@types/client.d.ts`:
- Line 18: Rename the misspelled optional property autoRegiserCommands in the
ClientOptions/type declaration to autoRegisterCommands and update all
references/usages (e.g., places like ready handler in src/handler/ready.ts and
any code reading ClientOptions) to use the corrected identifier; ensure
TypeScript types and any destructuring or defaulting logic that referenced
autoRegiserCommands are updated to autoRegisterCommands to avoid type and
runtime breakage.
♻️ Duplicate comments (3)
src/handler/message.ts (1)
8-10: Guard against non-string/empty prefix beforestartsWith.Line 8 still coerces the prefix to a string;
false/empty prefixes can accidentally match and trigger commands. Add a type/length guard first.🐛 Proposed fix
- const prefix = this.client.prefix as string; - if (!message.content.startsWith(prefix)) return; + const prefix = this.client.prefix; + if (typeof prefix !== "string" || prefix.length === 0) return; + if (!message.content.startsWith(prefix)) return;src/structures/Command.ts (1)
48-49: Commands can be registered with no execution path.Both
_supportsSlashand_supportsPrefixdefault tofalse, allowing commands to be registered but never invoked. Consider enforcing at least one handler or defaulting one flag totrue.src/structures/Event.ts (1)
26-38: Missing error handling for async event handlers.The async wrapper awaits
this.handler()but has no try-catch. Rejected promises will become unhandled rejections that can crash the process.🛠️ Proposed fix
process.nextTick(() => { const wrapper = async (...args: EventArgs<K>) => { - if (this.handler) { - await this.handler(...args); + try { + if (this.handler) { + await this.handler(...args); + } + } catch (error) { + this.logger.error(`Error in event "${String(this.name)}":`, error); } }; if (this.once) { this.client.once(this.name as string, wrapper); } else { this.client.on(this.name as string, wrapper); } });
📜 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 (16)
.gitignoreexamples/basic_client/events/ready.jsexamples/basic_client/index.jspackage.jsonsrc/handler/interaction.tssrc/handler/message.tssrc/handler/ready.tssrc/structures/Client.tssrc/structures/Command.tssrc/structures/Context.tssrc/structures/Event.tssrc/utils/Files.tssrc/utils/Logger.tssrc/utils/util.tstypes/client.d.tstypes/logger.d.ts
🧰 Additional context used
🧬 Code graph analysis (11)
examples/basic_client/index.js (1)
examples/basic_client/events/ready.js (1)
require(1-1)
src/handler/ready.ts (1)
src/structures/Event.ts (1)
EventBuilder(10-45)
src/handler/interaction.ts (4)
src/structures/Event.ts (1)
EventBuilder(10-45)examples/basic_client/index.js (1)
command(9-14)src/structures/Context.ts (1)
Context(9-61)src/utils/Logger.ts (1)
error(36-39)
src/handler/message.ts (3)
src/structures/Event.ts (1)
EventBuilder(10-45)src/utils/util.ts (1)
deleteMessage(4-14)src/structures/Context.ts (1)
Context(9-61)
src/structures/Client.ts (6)
types/client.d.ts (1)
FrameworkOptions(14-19)src/utils/Logger.ts (2)
LoggerInstance(11-45)error(36-39)src/structures/Command.ts (1)
CommandBuilder(20-81)src/utils/util.ts (1)
getPrefix(15-27)src/utils/Files.ts (2)
getProjectRoot(11-17)getFiles(4-10)src/context.ts (1)
setClient(6-8)
src/structures/Context.ts (1)
src/structures/Client.ts (1)
Client(25-110)
src/structures/Command.ts (6)
src/structures/Argument.ts (1)
Argument(6-36)src/structures/Client.ts (1)
Client(25-110)src/utils/Logger.ts (1)
LoggerInstance(11-45)src/structures/Context.ts (1)
Context(9-61)types/extra.d.ts (1)
MaybePromise(1-1)src/context.ts (1)
currentClient(4-4)
examples/basic_client/events/ready.js (1)
src/structures/Event.ts (1)
EventBuilder(10-45)
src/utils/util.ts (1)
types/client.d.ts (1)
PrefixOptions(9-12)
src/structures/Event.ts (4)
src/structures/Client.ts (1)
Client(25-110)src/utils/Logger.ts (1)
LoggerInstance(11-45)types/extra.d.ts (1)
MaybePromise(1-1)src/context.ts (1)
currentClient(4-4)
types/client.d.ts (1)
types/logger.d.ts (1)
LogLevel(1-1)
🔇 Additional comments (16)
.gitignore (2)
1-3: LGTM — reasonable env script ignore.
7-10: LGTM — standard log ignores.types/logger.d.ts (1)
1-1: LGTM!Clean type declaration for log levels. Centralizing this type in a dedicated declaration file allows for consistent reuse across the codebase.
types/client.d.ts (1)
1-17: LGTM on the type structure.The
LogLevelimport issue from the previous review has been addressed. ThePrefixOptionsunion type nicely supports both shorthand (string) and explicit object forms.src/utils/Logger.ts (1)
1-1: LGTM!Good refactor to import
LogLevelfrom the centralized type declaration. This keeps types consistent across the codebase.src/handler/interaction.ts (1)
5-33: LGTM — interaction handling and error flow look solid.package.json (1)
22-45: LGTM — imports map and dependency updates look fine.src/utils/Files.ts (1)
4-16: LGTM — file discovery and project-root fallback are reasonable.src/structures/Context.ts (1)
4-60: LGTM — Context construction and serialization look consistent.src/structures/Command.ts (2)
1-18: Imports and interface look good.The imports are well-organized and the
CommandOptionsinterface provides a clean contract for command configuration.
60-80: Builder methods look good.The
onMessageandonInteractionmethods correctly set the support flags and store handlers, enabling the fluent builder pattern with properreturn this.src/structures/Client.ts (3)
37-54: Constructor implementation looks solid.The merge fix (
merge({}, defaultOpts, opts)) prevents mutation of the module-level defaults. Handler registration is properly conditional on prefix configuration.
56-72: File loading methods are well-structured.Proper cache invalidation, context setting, and error handling with logging.
74-109:registerCommandshandles edge cases appropriately.Guard clauses with warnings for missing token/application, and proper error handling around the REST call.
src/structures/Event.ts (2)
1-8: Imports and type definition look good.The
EventArgsconditional type correctly maps knownClientEventskeys to their argument types while falling back toany[]for custom events.
41-44: Builder method looks good.The
onExecutemethod correctly stores the handler and returnsthisfor chaining.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/changelog.yml (1)
31-40: Fix push target branch reference.
Line 40 pushes a local branch namedchangelog, but the checkout is onmain, so the push will fail with “src refspec changelog does not match any.” Push the current HEAD to the remote branch instead, or create/switch to a localchangelogbranch before committing.🛠️ Proposed fix
- git push origin changelog + git push origin HEAD:changelog
🤖 Fix all issues with AI agents
In `@src/structures/Client.ts`:
- Around line 44-48: The constructor currently fire-and-forgets the async
loadFiles call (see loadFiles and the constructor block checking
this.options.paths?.events) which means event loading may not finish before
consumers proceed; either document this non-blocking behavior or add an explicit
ready promise: create and assign a public promise (e.g., this.ready or
this.eventsLoadedPromise) that awaits the loadFiles(...) call and rejects on
error (use this.logger.error inside the catch), and update any consumers or
README to await that promise when they require events to be fully loaded.
In `@src/structures/Command.ts`:
- Around line 28-35: The underscored fields _supportsSlash, _supportsPrefix,
_onMessage, and _onInteraction are declared public but use a private naming
convention; pick one consistent pattern: either remove the underscores and keep
them public (e.g., supportsSlash, supportsPrefix, onMessage, onInteraction) or
make the backing fields private (private _supportsSlash, private
_supportsPrefix, private _onMessage, private _onInteraction) and add public
readonly getters (get supportsSlash(), get supportsPrefix(), get onMessage(),
get onInteraction()) to expose read-only access; update usages in
Client.registerCommands() and any handler code to use the new public
names/getters so external code still reads the same values without depending on
underscored internals.
- Around line 51-56: When registering a command in the Command constructor
(symbols: this.name, this.aliases, this.client.commands, this.client.aliases),
add collision checks both ways: first ensure this.name does not exist as an
alias in this.client.aliases; then for each alias in this.aliases ensure it does
not exist as a command name in this.client.commands and does not exist as an
alias in this.client.aliases. If any collision is found, throw an Error
describing the conflicting identifier and the existing command; otherwise
proceed to set this.client.commands.set(this.name, this) and register each alias
into this.client.aliases (mapping the alias to this.name or a set containing
this.name) so future checks work correctly.
♻️ Duplicate comments (1)
src/utils/util.ts (1)
4-14: Silent error swallowing hides failures.The
.catch(() => {})silently ignores all errors. This is a pre-existing nitpick from a past review that remains unaddressed.
📜 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 (11)
.github/workflows/changelog.ymlCHANGELOG.mdexamples/basic_client/events/ready.jspackage.jsonsrc/handler/ready.tssrc/structures/Client.tssrc/structures/Command.tssrc/utils/Logger.tssrc/utils/util.tstypes/client.d.tstypes/logger.d.ts
💤 Files with no reviewable changes (1)
- CHANGELOG.md
🧰 Additional context used
🧬 Code graph analysis (7)
src/handler/ready.ts (1)
src/structures/Event.ts (1)
EventBuilder(10-45)
src/structures/Client.ts (6)
types/client.d.ts (1)
FrameworkOptions(14-19)src/utils/Logger.ts (2)
LoggerInstance(12-50)error(37-40)src/structures/Command.ts (1)
CommandBuilder(20-81)src/utils/util.ts (1)
getPrefix(15-25)src/utils/Files.ts (2)
getProjectRoot(11-17)getFiles(4-10)src/context.ts (1)
setClient(6-8)
src/utils/Logger.ts (1)
types/logger.d.ts (1)
LogLevel(1-1)
src/structures/Command.ts (6)
src/structures/Argument.ts (1)
Argument(6-36)src/structures/Client.ts (1)
Client(25-110)src/utils/Logger.ts (1)
LoggerInstance(12-50)src/structures/Context.ts (1)
Context(9-61)types/extra.d.ts (1)
MaybePromise(1-1)src/context.ts (1)
currentClient(4-4)
src/utils/util.ts (1)
types/client.d.ts (1)
PrefixOptions(9-12)
examples/basic_client/events/ready.js (1)
src/structures/Event.ts (1)
EventBuilder(10-45)
types/client.d.ts (1)
types/logger.d.ts (1)
LogLevel(1-1)
🔇 Additional comments (14)
.github/workflows/changelog.yml (1)
5-5: Daily schedule change looks good.examples/basic_client/events/ready.js (1)
1-7: Event registration usage is correct.package.json (1)
3-3: Dependency and imports updates align with the new framework surface.Also applies to: 22-24, 37-40, 44-44
src/handler/ready.ts (1)
1-7: Ready handler wiring looks solid.types/logger.d.ts (1)
1-1: LogLevel union type is clear and consistent.types/client.d.ts (1)
1-18: Framework option typings are well-structured.src/utils/Logger.ts (2)
1-10: LGTM! Type import and priority mapping are consistent.The import of
LogLevelfrom the centralized types file and the addition ofsuccess: 1priority (matchinglog) are well-aligned with the type definition intypes/logger.d.ts.
46-49: LGTM! Thesuccess()method follows established patterns.The implementation is consistent with other log methods, using the green background for visual distinction of success messages.
src/utils/util.ts (1)
15-25: LGTM! Type narrowing is now cleaner.The redundant
opts && typeof opts === "object"check from the previous review has been addressed. The current implementation correctly uses TypeScript's type narrowing with short-circuit evaluation.src/structures/Client.ts (3)
17-23: LGTM! Typo corrected.The
autoRegisterCommandsproperty name is now correctly spelled, addressing the previous review feedback.
37-54: LGTM! Previous issues addressed.The constructor correctly:
- Uses
merge({}, defaultOpts, opts)to avoid mutating the shareddefaultOptsobject- Stores the normalized prefix as
string | falsematchinggetPrefixreturn type- Conditionally loads the message handler only when a prefix is configured
74-109: LGTM! Command registration is well-implemented.The method properly:
- Logs warnings when prerequisites are missing instead of silently returning
- Uses the new
logger.success()for the completion message- Has appropriate error handling around the REST API call
src/structures/Command.ts (2)
36-58: LGTM! Previous issues addressed.The constructor now:
- Performs command registration synchronously (no
process.nextTick)- Uses simplified debug logging without the misleading
__filename
60-80: LGTM! Builder pattern implementation is clean.The
onMessageandonInteractionmethods correctly enable their respective flags and store handlers, returningthisfor method chaining.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@examples/basic_client/index.js`:
- Around line 3-7: Replace the magic number used for intents in the arox.Client
constructor with explicit named intent flags to improve readability and
maintainability: import and use discord.js's GatewayIntentBits (or
IntentsBitField) and set the intents option on the arox.Client configuration to
an array or bitfield composed of the specific GatewayIntentBits you need (e.g.,
Guilds, GuildMessages, MessageContent) instead of the numeric literal 37376.
- Line 26: Top-level await is used with client.login(process.env.BOT_TOKEN) in a
CommonJS module; wrap the call in an async IIFE (e.g., (async () => { await
client.login(...) })()) or convert to a promise chain
(client.login(process.env.BOT_TOKEN).then(...).catch(...)) to avoid a runtime
parse error, keeping error handling around the client.login call and preserving
the same process.env.BOT_TOKEN usage.
In `@src/context.ts`:
- Line 3: Replace the Turkish inline comment "Birden fazla client olursa hata
çıkartabilir ama aklıma gelen tek şey bu" in src/context.ts with an English
equivalent that preserves its meaning (e.g., "May throw an error if multiple
clients exist; this is the only approach I thought of"). Ensure the comment is
concise, clear, and placed next to the same code block in context.ts so
non-Turkish-speaking collaborators can understand it; optionally convert it into
a TODO or issue reference if further investigation is needed.
- Around line 3-14: The Map registry uses any and implicitly sets the
last-registered client as default which is unsafe; change the API to use a typed
key (make clients a Map<K, Client> via a generic or a concrete union/string type
instead of any), remove the side-effect of setting activeKey inside
registerClient, add an explicit setActiveClient(key: K) (or registerClient(...,
makeDefault: boolean) flag) to control the active client, update registerClient,
getClient and the activeKey declaration to use the new key type (replace usages
of clients, activeKey, registerClient, getClient) and ensure getClient falls
back to an explicit defaultKey value rather than the last-registered client.
In `@src/handler/interaction.ts`:
- Around line 19-20: When a command declares supportsSlash but has no
onInteractionCallback, add a warning instead of silently continuing: keep the
existing defensive check around command.onInteractionCallback but if
command.supportsSlash is true and onInteractionCallback is falsy, emit a clear
warning that includes the command identifier (e.g., command.name or command.id)
so misconfigured commands are visible; then proceed with the existing behavior
(await call only when onInteractionCallback exists). Use the module's logger
(logger.warn or processLogger.warn) if available, otherwise console.warn, and
ensure the log message references supportsSlash and onInteractionCallback to aid
debugging.
In `@src/structures/Client.ts`:
- Around line 56-72: Add validation in loadFiles to ensure the passed dir exists
and is a directory before calling getFiles: check existence (e.g., fs.existsSync
or fs.stat) and if invalid call this.logger.error with a clear message and
return (or throw) so callers get an immediate, descriptive failure; keep this
behavior consistent with loadFile's error logging (refer to loadFiles, getFiles,
and loadFile to locate where to insert the check).
In `@src/structures/Command.ts`:
- Around line 108-129: The methods onMessage and onInteraction currently
override any explicit support configuration by unconditionally setting
_supportsPrefix/_supportsSlash to true; change them to only set the flag when it
wasn't explicitly disabled by previous configuration (i.e., only assign
this._supportsPrefix = true if this._supportsPrefix is !== false or undefined,
and only assign this._supportsSlash = true if this._supportsSlash is !== false
or undefined) so calling onMessage/onInteraction doesn't inadvertently enable a
mode that was explicitly turned off; update the onMessage and onInteraction
implementations to check the existing flag before setting it.
In `@src/structures/Event.ts`:
- Around line 46-49: The current race is that the constructor binds the listener
via process.nextTick but onExecute can be called later, so the event may fire
before handler is set; fix by deferring binding until a handler exists: remove
the immediate process.nextTick binding in the constructor and instead implement
binding logic inside onExecute (or a new register() method) so that
onExecute(func: ...) sets this.handler and, if not already bound (use a
this.bound boolean), attaches the event listener and marks this.bound = true;
reference the constructor, onExecute, handler and a new bound/register symbol
when making the change.
In `@src/utils/Files.ts`:
- Around line 21-27: getProjectRoot currently returns
path.dirname(require.main.filename) which resolves to a compiled/dist folder for
nested entry points; update the getProjectRoot function to walk up the directory
tree (starting from require.main?.filename || process.cwd()) searching for a
package.json and return the directory that contains it as the project root,
falling back to process.cwd() if none is found, and add an optional override
(e.g., an optional parameter or env/config value) so callers can explicitly set
the base path for resolving events/commands.
♻️ Duplicate comments (1)
src/structures/Event.ts (1)
9-25:__filenamestill logs the wrong file path.Line 24 uses
__filenamewhich will always resolve toEvent.tsrather than the actual event file being loaded. This was flagged in a previous review and remains unaddressed.🛠️ Proposed fix
- this.logger.debug(`Loaded Event ${this.name}(${__filename})`); + this.logger.debug(`Loaded Event ${String(this.name)}`);
📜 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 (9)
examples/basic_client/index.jssrc/context.tssrc/handler/interaction.tssrc/handler/message.tssrc/structures/Client.tssrc/structures/Command.tssrc/structures/Event.tssrc/utils/Files.tstypes/client.d.ts
🧰 Additional context used
🧬 Code graph analysis (6)
src/handler/message.ts (4)
src/structures/Event.ts (1)
EventBuilder(9-50)src/utils/util.ts (1)
deleteMessage(4-14)src/structures/Context.ts (1)
Context(9-61)src/utils/Logger.ts (1)
error(37-40)
examples/basic_client/index.js (1)
examples/basic_client/events/ready.js (1)
require(1-1)
src/structures/Client.ts (6)
types/client.d.ts (1)
FrameworkOptions(14-19)src/utils/Logger.ts (2)
LoggerInstance(12-50)error(37-40)src/structures/Command.ts (1)
CommandBuilder(22-129)src/utils/util.ts (1)
getPrefix(15-25)src/utils/Files.ts (2)
getProjectRoot(21-27)getFiles(4-20)src/context.ts (1)
registerClient(7-10)
src/context.ts (1)
src/structures/Client.ts (1)
Client(25-110)
src/structures/Command.ts (6)
src/structures/Argument.ts (1)
Argument(6-36)src/structures/Client.ts (1)
Client(25-110)src/utils/Logger.ts (1)
LoggerInstance(12-50)src/structures/Context.ts (1)
Context(9-61)types/extra.d.ts (1)
MaybePromise(1-1)src/context.ts (1)
getClient(12-14)
src/handler/interaction.ts (4)
src/structures/Event.ts (1)
EventBuilder(9-50)examples/basic_client/index.js (1)
command(9-14)src/structures/Context.ts (1)
Context(9-61)src/utils/Logger.ts (1)
error(37-40)
🪛 Biome (2.1.2)
examples/basic_client/index.js
[error] 26-26: await is only allowed within async functions and at the top levels of modules.
(parse)
🔇 Additional comments (15)
src/utils/Files.ts (1)
4-19: LGTM ongetFiles.The ignore patterns now properly exclude common build directories (
dist/**,lib/**,out/**,build/**,.next/**,coverage/**), addressing the previous concern about duplicate loading.types/client.d.ts (1)
1-19: LGTM!The type definitions are well-structured.
LogLevelis now properly defined, theautoRegisterCommandstypo has been fixed, andPrefixOptionsprovides a clean discriminated union for prefix configuration.src/handler/interaction.ts (1)
5-35: LGTM on the overall handler structure.The error handling correctly distinguishes between replied/deferred states, and the command validation logic is sound.
src/handler/message.ts (1)
6-45: LGTM!The handler correctly addresses previous review feedback:
- Properly guards against non-string or empty prefixes (lines 9-14)
- Uses
supportsPrefixfor command validation (line 25)- Passes
argsto the Context constructor (line 36)The error handling and debug logging are well implemented.
src/structures/Client.ts (5)
1-16: LGTM!Imports are well-organized and cover all necessary dependencies for the Client implementation.
17-23: LGTM!The typo
autoRegiserCommandshas been corrected toautoRegisterCommands. Default options are properly structured.
25-36: LGTM!The Client class properly extends DiscordClient with appropriate type parameter. The
prefixproperty is correctly typed asstring | falseto matchgetPrefix()return type.
37-54: LGTM!Constructor addresses previous review feedback:
- Uses
merge({}, defaultOpts, opts)to avoid mutating the shareddefaultOptsobject- Message handler is conditionally loaded only when
this.prefixis truthy- Client registration and handler setup are properly sequenced
74-109: LGTM!The
registerCommandsmethod properly addresses previous review feedback:
- Guards for missing token and application with clear warning messages
- Uses
logger.successfor the success message (appropriate log level)- Error handling wraps the REST call properly
src/structures/Event.ts (2)
1-7: LGTM!Imports are appropriate and the
EventArgstype alias correctly extracts event argument types fromClientEvents.
26-44: LGTM!The event binding logic correctly addresses the previous review feedback:
- Uses
process.nextTickto defer event registration- Wraps handler execution in
Promise.resolve().catch()to prevent unhandled rejections- Error logging includes event name and class name for debugging
src/structures/Command.ts (4)
1-20: LGTM!Imports are well-organized and the
CommandOptionsinterface clearly defines the command configuration schema with appropriate optional fields.
22-50: LGTM!The class structure properly addresses previous feedback:
- Private fields (
_supportsSlash,_supportsPrefix,_onMessage,_onInteraction) with public getters provide clean encapsulation- Handler callback types correctly reference
Context<T>["toJSON"]return types
51-71: LGTM!Constructor initialization properly addresses previous feedback:
- Command registration is now synchronous (no
process.nextTick)- Enforces at least one of
slashorprefixmust be enabled- Options are correctly normalized, converting
Argumentinstances to JSON
73-106: LGTM!Excellent collision detection implementation addressing previous feedback:
- Checks command name against existing commands
- Checks command name against existing aliases
- Checks each alias against existing commands and aliases
- Clear, descriptive error messages identify the conflicting command
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Co-authored-by: Fhyrox <fhyrox@users.noreply.github.com> Co-authored-by: Coderabbit <support@coderabbit.ai>
vrdons
force-pushed
the
refactor/class-based-structure
branch
from
c945842 to
d91d2ca
Compare
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Fix all issues with AI agents
In `@examples/basic_client/events/ready.js`:
- Around line 3-7: The event file uses a hard-coded string "clientReady" in the
EventBuilder call which won't match Discord.js events; replace the string with
the Events.ClientReady constant (i.e., use Events.ClientReady as the first
argument to EventBuilder) and ensure you import Events from "discord.js" at the
top of the file; update the EventBuilder invocation (the EventBuilder symbol)
and keep the existing handler body (context.logger.log, context.logger.warn,
context.client references) unchanged.
In `@examples/basic_client/index.js`:
- Around line 22-29: Replace the Turkish reply strings in the example handlers
with English text to improve accessibility: update the callbacks registered with
onMessage and onInteraction so that message.reply(...) and
interaction.reply(...) use concise English examples (e.g., "I'm working, please
leave me alone" or a friendly "I'm alive!" message) while keeping the same
handlers and return/void usage.
- Around line 31-34: The init function calls client.login(process.env.BOT_TOKEN)
without handling failures, so add error handling around the login call (either
wrap client.login in a try/catch inside async function init or append a .catch
to the promise returned by void init()) to surface and log the error (include
the token absence/invalid message) and exit or rethrow as appropriate; update
references in this file to handle errors from client.login and ensure void
init() is replaced or augmented with a .catch handler that logs the error and
stops the process.
In `@src/events/interaction.ts`:
- Around line 21-25: The handler function in src/events/interaction.ts is a
regular async function that incorrectly uses this (unbound); update the handler
to use the context parameter passed by EventBuilder.listener instead of this.
Concretely, replace any occurrences of this.client.logger (and any other this.*
usages) inside the handler (including the catch block currently calling
this.client.logger.error) with context.client.logger (or context.<...) and
ensure the handler signature and all internal references rely on the provided
context parameter rather than this.
In `@src/events/message.ts`:
- Around line 23-27: The handler in message.ts uses unbound this (e.g.,
this.client.aliases.findKey and this.client.commands.get) which is undefined in
strict mode; update the function to use the provided first parameter (the
EventBuilder instance) instead of this — replace references like this.client,
this.client.aliases, and this.client.commands with the eventBuilder (or whatever
the first parameter name is) and then compute commandAlias via
eventBuilder.client.aliases.findKey(...) and lookup the command via
eventBuilder.client.commands.get(commandAlias ?? commandName).
- Around line 38-49: Replace the unbound this.logger/this.client.logger usage by
extracting the logger from the bound client before the try block (e.g. const {
logger } = this.client) and use logger.debug/logger.error instead; remove the
redundant if (command) guard before awaiting command.onMessageCallback since
command is guaranteed by the earlier return; and avoid shadowing the outer
context parameter by renaming the inner new Context(...) variable to ctx (use
ctx.toJSON() when calling command.onMessageCallback) so the code uses Context,
command.onMessageCallback, logger, and ctx unambiguously.
In `@src/structures/Client.ts`:
- Around line 57-78: The loadFile function currently sets the client via
setClient(this) and requires the module, but if require(file) throws the catch
path logs the error and never calls clearClient(), leaking the global client;
refactor loadFile to move setClient(this) before the try and put the
require(file) and delete require.cache/require.resolve(file) inside a try block
with a finally that always calls clearClient(), ensuring clearClient() runs
regardless of errors (keep the existing delete of require.cache and the
logger.error in catch to preserve behavior).
- Around line 25-55: In the Client constructor the temporary client context is
set with setClient(this) but clearClient() can be skipped if loadFiles or any
require("../events/...") throws; wrap the context setup and the require(...)
calls in a try/finally so clearClient() always runs: call setClient(this) before
the try, perform require("../events/ready"), require("../events/interaction"),
and conditional require("../events/message") inside the try, and call
clearClient() in the finally block; leave the existing loadFiles(...).catch(...)
logic as-is.
In `@src/utils/logger/ILogger.ts`:
- Line 93: Remove the unnecessary empty default export (export default {}) from
the module; keep the named exports LogLevel and ILogger only so consumers import
the intended symbols via named imports and avoid confusion with a meaningless
default export.
In `@src/utils/logger/Logger.ts`:
- Around line 64-77: getInstance() currently always constructs Logger with
default options, ignoring caller-supplied configuration; update getInstance to
accept an optional options parameter (or add a static configure/init method)
that is used when creating the singleton and that can merge/override settings if
called before the instance exists, referencing the static instance, the Logger
constructor, and getInstance(); ensure subsequent calls without options return
the existing instance and add minimal validation/merge logic so provided options
are applied when creating the singleton.
- Around line 50-62: The Logger constructor currently defaults this.depth to 0
which causes util.inspect to truncate nested objects; update the Logger
constructor (class Logger, method constructor) to default depth to 2 (or null
for unlimited) by changing the default assignment for this.depth in the
constructor and ensure any callers relying on LoggerOptions.depth keep the same
override behavior; no other changes to createFormatMap or other properties are
needed.
In `@types/client.d.ts`:
- Around line 1-2: The import in types/client.d.ts pulls LoggerOptions from
../src which crosses into src; change this to use a proper type surface (e.g.,
re-export LoggerOptions from a dedicated types module or a TS path alias) so
.d.ts files don't reference src directly: create or use a types-only module that
exports LoggerOptions (or add a TS path alias in tsconfig and import via that
alias) and update the import in types/client.d.ts to reference that module
instead of ../src/utils/logger/Logger.
♻️ Duplicate comments (5)
src/utils/Files.ts (1)
21-27:getProjectRootreturns incorrect path for applications with nested entry points.This issue was previously flagged. When an application runs from a compiled subdirectory (e.g.,
dist/index.js),path.dirname(require.main.filename)returnsdist/rather than the actual project root. This causes relative paths for events/commands to resolve incorrectly.Consider walking up the directory tree to find
package.json, or allow users to explicitly configure the base path.🛠️ Proposed fix
+import fs from "fs"; + +function findPackageJsonDir(startDir: string): string { + let dir = startDir; + while (dir !== path.dirname(dir)) { + if (fs.existsSync(path.join(dir, "package.json"))) { + return dir; + } + dir = path.dirname(dir); + } + return startDir; +} + export function getProjectRoot(): string { if (!require.main?.filename) { return process.cwd(); } - return path.dirname(require.main.filename); + return findPackageJsonDir(path.dirname(require.main.filename)); }src/utils/util.ts (1)
4-14: Silent error swallowing hides failures.This issue was previously flagged. The
.catch(() => {})silently ignores all errors including permission issues or already-deleted messages. Consider logging failures or returning a result indicating success/failure.♻️ Proposed improvement
export function deleteMessage( message: Message | InteractionResponse, time = 15_000 ) { - return new Promise<void>((r) => { + return new Promise<boolean>((r) => { setTimeout(() => { - message.delete().catch(() => {}); - r(); + message.delete() + .then(() => r(true)) + .catch(() => r(false)); }, time); }); }src/context.ts (1)
3-12: Single-client registry pattern is functional but limited.The Turkish comment and multi-client concerns were addressed in previous reviews. The current simple implementation works for the single-client use case demonstrated in the PR but will need refactoring if multi-client support becomes necessary.
examples/basic_client/index.js (1)
3-10: Consider using named intent flags for readability.The magic number
37376is opaque. This concern was raised in a previous review.src/structures/Event.ts (1)
31-46:__filenamelogs the wrong file path.Line 45 uses
__filenamewhich always resolves toEvent.ts, not the actual event file being loaded. This makes debugging event loading issues misleading.🛠️ Proposed fix
- this.logger.debug(`Loaded Event ${this.name} (${__filename})`); + this.logger.debug(`Loaded Event ${String(this.name)}`);
📜 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 (18)
.oxfmtrc.json.swcrcexamples/basic_client/events/ready.jsexamples/basic_client/index.jspackage.jsonsrc/context.tssrc/events/interaction.tssrc/events/message.tssrc/events/ready.tssrc/index.tssrc/structures/Client.tssrc/structures/Command.tssrc/structures/Event.tssrc/utils/Files.tssrc/utils/logger/ILogger.tssrc/utils/logger/Logger.tssrc/utils/util.tstypes/client.d.ts
🧰 Additional context used
🧬 Code graph analysis (10)
examples/basic_client/events/ready.js (1)
src/structures/Event.ts (1)
EventBuilder(13-65)
src/events/ready.ts (1)
src/structures/Event.ts (1)
EventBuilder(13-65)
types/client.d.ts (1)
src/utils/logger/Logger.ts (1)
LoggerOptions(299-307)
src/utils/util.ts (1)
types/client.d.ts (1)
PrefixOptions(9-12)
src/structures/Client.ts (6)
types/client.d.ts (1)
FrameworkOptions(14-19)src/utils/logger/Logger.ts (2)
Logger(10-199)error(103-105)src/structures/Command.ts (1)
CommandBuilder(22-121)src/utils/util.ts (1)
getPrefix(15-25)src/utils/Files.ts (2)
getProjectRoot(21-27)getFiles(4-20)src/context.ts (2)
setClient(6-8)clearClient(10-12)
src/events/message.ts (4)
src/structures/Event.ts (1)
EventBuilder(13-65)src/utils/util.ts (1)
deleteMessage(4-14)src/structures/Context.ts (1)
Context(9-61)src/utils/logger/Logger.ts (1)
error(103-105)
src/events/interaction.ts (3)
src/structures/Event.ts (1)
EventBuilder(13-65)src/structures/Context.ts (1)
Context(9-61)src/utils/logger/Logger.ts (1)
error(103-105)
src/utils/logger/Logger.ts (1)
src/utils/logger/ILogger.ts (1)
ILogger(43-92)
examples/basic_client/index.js (1)
examples/basic_client/events/ready.js (1)
require(1-1)
src/context.ts (2)
src/structures/Client.ts (1)
Client(25-116)examples/basic_client/index.js (1)
client(3-10)
🔇 Additional comments (27)
.oxfmtrc.json (1)
15-16: LGTM!Adding
"endOfLine": "lf"is a good practice for ensuring consistent line endings across different operating systems, which helps avoid spurious diffs and merge conflicts in version control..swcrc (1)
16-16: Disabling minification is acceptable for development.Consider enabling minification for production releases to reduce bundle size. You could use environment-based configuration or separate build scripts if both development and production builds are needed.
package.json (3)
3-3: Version bump from alpha to stable release.Ensure this version change is intentional as it signals readiness for production use. The transition from
0.1.0-alpha.2to0.1.0indicates the API is considered stable.
22-24: Imports mapping configuration looks correct.The
#types/*mapping to./types/*enables cleaner imports throughout the codebase. Ensure this is compatible with TypeScript's module resolution settings intsconfig.json.
36-42: Dependency changes align with new features.The added dependencies support the new functionality:
fast-globfor file discoverycolorettefor logging colors (lighter alternative to chalk)lodashfor general utilities@sapphire/timestampfor timestamp formatting@swc/helpersrequired forexternalHelpers: truein.swcrcsrc/utils/Files.ts (1)
4-19: LGTM! File discovery with proper exclusions.The ignore list now correctly excludes common build/output directories to prevent duplicate command/event registration.
src/utils/util.ts (1)
15-25: LGTM! Type narrowing is now simplified.The previous redundant
typeof opts === "object"check has been removed, and the logic correctly handles all branches of thePrefixOptionsunion type.src/events/ready.ts (1)
4-8: LGTM! Event handler correctly usesonExecutepattern.The implementation properly uses
function()syntax to maintainthiscontext for accessingthis.client. BothautoRegisterCommandsoption andregisterCommands()method are correctly defined on the Client class with appropriate defaults and error handling.src/events/interaction.ts (1)
5-39: Overall structure looks good, pending thethisfix above.The handler correctly:
- Filters for chat input commands
- Validates command existence and slash support
- Creates a
Contextand invokes the command- Handles reply vs followUp based on interaction state
src/index.ts (1)
1-7: Barrel exports look good.All new structures (
Client,Command,Context,Event,Argument), the logger utility, and the client registry are properly re-exported for the public API.types/client.d.ts (1)
4-19: Type declarations are well-structured.The discriminated union for
PrefixOptionsand the extension ofClientOptionsforFrameworkOptionsare correctly typed. Previous issues (unresolvedLogLevel, typo) have been addressed.src/utils/logger/ILogger.ts (1)
1-92: LGTM!The
LogLevelenum with numeric values enables clean level comparison, and theILoggerinterface provides a well-documented contract for the logging system.src/structures/Client.ts (2)
1-24: LGTM!Imports are well-organized and
defaultOptscorrectly defines sensible defaults with the typo fix (autoRegisterCommands).
80-115: LGTM!The
registerCommands()method properly guards against missing token/application with clear warning messages, filters for slash-supporting commands, and handles REST errors gracefully.src/structures/Event.ts (3)
1-12: LGTM!Type definitions for
EventArgsandEventHandlerare well-designed, passing theEventBuilderinstance as context to handlers.
13-29: LGTM!The
listenerarrow function correctly handles async errors by wrapping the handler call in a try/catch and logging viaclient.logger.error.
48-64: LGTM!The
register()method correctly guards against double-binding with theboundflag, andonExecute()provides a clean fluent API for setting handlers after construction.src/structures/Command.ts (4)
1-20: LGTM!The
CommandOptionsinterface is well-defined, allowing bothApplicationCommandOptionDataandArgumenttypes for options, with clear optional flags for slash/prefix support.
45-100: LGTM!The constructor performs thorough validation:
- Ensures client context exists
- Validates at least one execution mode is enabled
- Checks for duplicate command names and alias collisions in both directions
- Registers synchronously to allow callers to catch errors
102-121: LGTM!The
onMessageandonInteractionmethods provide a clean fluent API. The getters correctly combine the flag check with handler presence, ensuring a command only reports support when both are configured.
22-44: Revert this suggestion;_onInteractionis accessed directly from outside the class.The properties cannot both be made private. In
src/events/interaction.ts, the code directly accessesif (command._onInteraction) await command._onInteraction(...), which requires the property to remain public. While_onMessageis accessed through theonMessageCallback()method instead of direct property access, the asymmetry means making only one private would be inconsistent and confusing.Likely an incorrect or invalid review comment.
src/utils/logger/Logger.ts (6)
1-62: LGTM!The
Loggerclass is well-structured with configurable streams, format options, and sensible defaults. The static maps for methods, colors, and names are cleanly organized.
79-138: LGTM!The level check in
has(), the convenience methods (trace,debug, etc.), and thewrite()dispatch logic are all correct. The switch statement cleanly maps to console methods.
140-198: LGTM!The
preprocessmethod correctly handles string passthrough vsinspectfor objects, and the format map creation properly applies defaults and level-specific overrides.
201-242: LGTM!
LoggerStyleprovides flexible color composition, handling both function-based and options-based style configuration with proper fallback tocolorette.reset.
244-295: LGTM!
LoggerTimestampandLoggerLevelare well-designed, supporting UTC, custom patterns, colorization, and per-line prefix application for multi-line content.
297-392: LGTM!The default export and comprehensive type definitions provide a complete, extensible logging configuration API.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Co-authored-by: Coderabbit <support@coderabbit.ai>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/events/message.ts`:
- Around line 23-36: The prefix command handler incorrectly checks
command.supportsSlash which rejects prefix-only commands; update the check in
the message handler that retrieves command via commandAlias/commandName to use
command.supportsPrefix instead of supportsSlash, so valid prefix commands are
accepted; keep the existing reply/deleteMessage flow and early return logic
unchanged, only replace the supportsSlash reference with supportsPrefix where
the command existence/availability is validated.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (7)
src/events/interaction.tssrc/events/message.tssrc/events/ready.tssrc/structures/Client.tssrc/structures/Event.tssrc/utils/logger/ILogger.tssrc/utils/logger/Logger.ts
🧰 Additional context used
🧬 Code graph analysis (5)
src/structures/Client.ts (5)
types/client.d.ts (1)
FrameworkOptions(14-19)src/structures/Command.ts (1)
CommandBuilder(22-121)src/utils/util.ts (1)
getPrefix(15-25)src/utils/Files.ts (2)
getProjectRoot(21-27)getFiles(4-20)src/context.ts (2)
setClient(6-8)clearClient(10-12)
src/structures/Event.ts (4)
types/extra.d.ts (1)
MaybePromise(1-1)src/structures/Client.ts (1)
Client(25-119)src/utils/logger/Logger.ts (2)
Logger(10-199)error(103-105)src/context.ts (1)
currentClient(4-4)
src/events/ready.ts (1)
src/structures/Event.ts (1)
EventBuilder(13-65)
src/events/message.ts (4)
src/structures/Event.ts (1)
EventBuilder(13-65)src/utils/util.ts (1)
deleteMessage(4-14)src/structures/Context.ts (1)
Context(9-61)src/utils/logger/Logger.ts (1)
error(103-105)
src/utils/logger/Logger.ts (1)
src/utils/logger/ILogger.ts (1)
ILogger(43-92)
🔇 Additional comments (9)
src/utils/logger/ILogger.ts (1)
1-92: LGTM: clear public logger contract.The LogLevel enum and ILogger surface are consistent and well-documented.
src/utils/logger/Logger.ts (1)
1-392: LGTM: solid, consistent logging pipeline and defaults.The formatting, level handling, and preprocessing logic look coherent and safe.
src/events/ready.ts (1)
1-8: LGTM: ready handler is concise and guarded.Auto-registration is properly gated by the option flag.
src/events/interaction.ts (1)
1-39: LGTM: interaction handler is robust.Command validation, error handling, and response flow are handled cleanly.
src/structures/Event.ts (1)
1-64: LGTM: event wiring and error capture look correct.The handler invocation is safely wrapped and registration is properly guarded.
src/structures/Client.ts (4)
37-58: Clean constructor initialization and context safety.
The merged options, normalized prefix, and try/finally around handler requires are well-structured.
60-69: Directory guard + sequential load is solid.
71-80: Robust per-file loading with guaranteed cleanup.
83-117: Command registration flow looks correct and well-logged.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/events/message.ts`:
- Around line 38-49: A command that declares supportsPrefix: true but lacks an
_onMessage handler silently no-ops; update the try block in the message handling
(where Context, ctx, and command._onMessage are used) to detect when
command.supportsPrefix is true and command._onMessage is falsy and then emit a
clear warning via context.client.logger.warn or context.logger.warn mentioning
command.name and that _onMessage is missing, otherwise proceed to call await
command._onMessage(ctx.toJSON()) as before.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
src/events/message.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/events/message.ts (5)
src/structures/Event.ts (1)
EventBuilder(13-65)examples/basic_client/index.js (3)
command(13-18)ctx(23-23)ctx(27-27)src/utils/util.ts (1)
deleteMessage(4-14)src/structures/Context.ts (1)
Context(9-61)src/utils/logger/Logger.ts (1)
error(103-105)
🔇 Additional comments (3)
src/events/message.ts (3)
1-4: LGTM!Imports are clean and all are used within the handler.
6-21: LGTM!Event handler setup is well-structured. The prefix validation is thorough, correctly handling edge cases like undefined or empty prefixes. Argument parsing is clean with proper early returns.
23-36: LGTM!Command resolution correctly handles aliases and validates
supportsPrefix. The error reply withdeleteMessagecleanup provides good UX without cluttering the channel.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.