Feat/handlers and types #3
Conversation
📝 WalkthroughWalkthroughThis pull request refactors the framework by removing the BaseClient class and introducing a comprehensive new architecture. It adds a Client class, abstract Command and Event structures, command and event handlers for loading and orchestration, a Context abstraction for unified message/interaction handling, and supporting types. The main index.ts re-exports these new modules to form a public API. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Discord Client
participant CmdHandler as CommandHandler
participant Cmd as Command
participant Ctx as Context
participant User as Discord User
User->>Client: sends message or interaction
Client->>CmdHandler: delegates (messageCreate/interactionCreate)
CmdHandler->>CmdHandler: parse command & resolve aliases
CmdHandler->>Cmd: find matching Command
CmdHandler->>Ctx: create Context with parsed args
Ctx->>Cmd: execute(ctx)
Cmd->>Ctx: ctx.reply() / ctx.send()
Ctx->>User: send response (reply/followUp/message.reply)
sequenceDiagram
participant Client as Client
participant EvtHandler as EventHandler
participant EvtFile as Event File
participant Evt as Event
participant DiscordClient as Discord Client
Client->>EvtHandler: loadEvents(dir)
EvtHandler->>EvtHandler: getFiles(dir) recursively
EvtHandler->>EvtFile: require event file
EvtFile->>EvtHandler: export Event class
EvtHandler->>Evt: new Event(client)
EvtHandler->>DiscordClient: register via .once() or .on()
DiscordClient->>Evt: emit event with args
Evt->>Evt: execute(...args)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
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: 10
🤖 Fix all issues with AI agents
In `@src/handlers/CommandHandler.ts`:
- Around line 130-142: The getFiles function uses blocking fs calls; convert it
to an async function (e.g., async getFiles(dir: string, fileList: string[] =
[]): Promise<string[]>) and replace fs.readdirSync/fs.statSync with
fs.promises.readdir and fs.promises.stat (awaiting results), recursively await
calls to this.getFiles for subdirectories, push .ts/.js file paths to fileList,
and propagate errors or handle them gracefully; update all call sites to await
the new Promise-returning getFiles and adjust types accordingly (retain use of
path.join and the same file extension checks).
- Around line 41-42: The registerCommands method currently returns silently when
this.client.token or this.client.application is missing; update registerCommands
to log a clear warning before returning so callers can see why registration was
skipped (e.g., use an existing logger or console.warn). Locate the check in
registerCommands that inspects this.client.token and this.client.application and
add a descriptive warning message including which property is missing (token
and/or application) prior to the early return.
- Around line 18-24: The loader uses require()/require.resolve which breaks for
ESM; switch to dynamic import() for cross-compatibility: replace delete
require.cache/require.resolve and await require(file) with await import(file)
(use a cache-bust query string if you need to avoid import() caching), then
extract the class with something like const CommandClass = module.default ??
module and keep the instanceof check (CommandClass.prototype instanceof Command)
to validate the class; update the CommandHandler loading logic to use import()
and remove require.cache usage.
In `@src/handlers/EventHandler.ts`:
- Around line 25-29: The event handlers currently call event.execute(...)
directly on this.client.once/on which can throw or return a rejected promise;
wrap the handler call in an async wrapper that catches both sync and async
errors (e.g., await Promise.resolve(event.execute(...args)) inside try/catch)
and log the error (use an existing logger like this.logger.error if available,
falling back to console.error) so failures are handled and don't become
unhandled rejections.
In `@src/structures/Argument.ts`:
- Around line 13-34: The Argument class currently accepts choices regardless of
option type; add a validation in the constructor (or a small private validator
called from the constructor) to ensure when data.choices is provided the
data.type is one of the allowed types (ApplicationCommandOptionType.String,
Integer, Number) and that each choice.value matches the expected JS type (string
for String, number for Integer/Number); if validation fails, either throw a
clear Error (e.g., "choices are only allowed for STRING/INTEGER/NUMBER options")
or remove data.choices so toJSON won't include invalid choices. Update the
constructor (and keep toJSON unchanged) to perform this guard and reference the
Argument class, constructor, choices property, and ApplicationCommandOptionType
constants.
In `@src/structures/Client.ts`:
- Around line 9-39: The constructor currently calls
this.eventHandler.loadEvents(...) and only attaches .catch(), causing async
initialization to run unnoticed; refactor by removing the async call from the
constructor and add a public async init() or start() method on Client (e.g.,
async init(): Promise<void>) that awaits
this.eventHandler.loadEvents(path.join(__dirname, "..", "events")) and rethrows
or logs errors for callers to handle, or alternatively expose a public ready
Promise property (e.g., this.ready = this.eventHandler.loadEvents(...)) that
callers can await; ensure all references to loadEvents are moved to the new
init/start/ready mechanism and the constructor only synchronously sets up
properties and listeners (keep this.eventHandler and this.commandHandler
instantiation in constructor but do not call loadEvents there).
- Around line 35-38: The constructor currently auto-loads events from a
hardcoded path using this.eventHandler.loadEvents(path.join(__dirname, "..",
"events")), which points inside the framework package and breaks consumer
projects; update the Client constructor to accept a configurable eventsDir via
ClientOptionsWithFramework (or remove the auto-load) and only call
this.eventHandler.loadEvents(eventsDir) when that option is provided, or instead
remove the load call entirely and document that consumers must call
eventHandler.loadEvents(...) themselves; modify the constructor signature to
read eventsDir from ClientOptionsWithFramework and use that value (or skip
calling this.eventHandler.loadEvents) and ensure error handling remains in the
catch block for eventHandler.loadEvents.
In `@src/structures/Context.ts`:
- Around line 37-43: The getters author and guild in Context use non-null
assertions on this.message which will throw when Context contains only an
interaction; change the implementations to use safe optional chaining and
nullish coalescing (e.g., author -> this.interaction?.user ??
this.message?.author and guild -> this.interaction?.guild ??
this.message?.guild) and update the return types on the author and guild getters
to allow undefined/null (e.g., author: User | undefined, guild: Guild | null |
undefined) to match the channel getter's safe behavior.
- Around line 60-63: The send method in Context.ts currently casts this.channel
to any and calls send, losing type safety; change it to narrow the channel type
and guard before calling send: check whether this.channel is a text-capable
channel (e.g., implement a type guard for TextBasedChannel or check for the
existence and type of a send function) and only call channel.send(options) when
the guard passes, returning undefined otherwise; ensure ReplyOptions and Message
types are preserved and avoid using any when referencing the channel variable or
its send method.
📜 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)
src/classes/base/BaseClient.tssrc/handlers/CommandHandler.tssrc/handlers/EventHandler.tssrc/index.tssrc/structures/Argument.tssrc/structures/Client.tssrc/structures/Command.tssrc/structures/Context.tssrc/structures/Event.tssrc/types/index.tssrc/utils/Logger.ts
💤 Files with no reviewable changes (1)
- src/classes/base/BaseClient.ts
🧰 Additional context used
🧬 Code graph analysis (6)
src/handlers/EventHandler.ts (2)
src/structures/Client.ts (1)
Client(9-40)src/utils/Logger.ts (1)
error(36-39)
src/types/index.ts (2)
src/utils/Logger.ts (1)
LogLevel(2-2)src/structures/Argument.ts (1)
Argument(6-36)
src/structures/Context.ts (1)
src/structures/Client.ts (1)
Client(9-40)
src/handlers/CommandHandler.ts (3)
src/structures/Client.ts (1)
Client(9-40)src/utils/Logger.ts (1)
error(36-39)src/structures/Context.ts (1)
Context(17-64)
src/structures/Event.ts (1)
src/structures/Client.ts (1)
Client(9-40)
src/structures/Command.ts (3)
src/types/index.ts (1)
CommandOptions(11-18)src/structures/Argument.ts (1)
Argument(6-36)src/structures/Context.ts (1)
Context(17-64)
🔇 Additional comments (7)
src/index.ts (1)
1-9: Public re-exports look consistent.The API surface is clean and aligns with the new architecture.
src/structures/Command.ts (1)
16-32: LGTM for option normalization and defaults.Clear mapping of
Argumentto JSON and sensible defaults for aliases/prefix/slash.src/types/index.ts (1)
5-18: Types additions look good.Interfaces align with the new Client/Command structures.
src/structures/Event.ts (1)
4-16: LGTM!Well-designed abstract class with proper type parameterization using
ClientEvents. The generic constraint ensures type-safe event handling, and theonceparameter default is sensible.src/handlers/CommandHandler.ts (3)
67-94: LGTM on message handling flow.The prefix parsing, alias resolution, and error handling logic is well-structured. Good use of Context to unify the execution interface.
108-127: LGTM on interaction error handling.Correctly checks
repliedanddeferredstate before deciding whether to usefollowUporreply. Ephemeral error messages are appropriate.
96-97:isCommand()is not deprecated and serves a different purpose thanisChatInputCommand().
isCommand()returns true for any CommandInteraction (including chat input and context menu commands), whileisChatInputCommand()returns true only for slash commands. UseisChatInputCommand()only if the handler specifically handles slash commands; otherwiseisCommand()is the appropriate choice.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| try { | ||
| delete require.cache[require.resolve(file)]; | ||
| const { default: CommandClass } = await require(file); | ||
|
|
||
| if (!CommandClass || !(CommandClass.prototype instanceof Command)) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Use dynamic import() instead of require() for ESM compatibility.
Using require() won't work if consumer projects use ESM modules. Dynamic import() is the cross-compatible approach.
Proposed fix
try {
- delete require.cache[require.resolve(file)];
- const { default: CommandClass } = await require(file);
+ const modulePath = `file://${file}?update=${Date.now()}`;
+ const { default: CommandClass } = await import(modulePath);
if (!CommandClass || !(CommandClass.prototype instanceof Command)) {
continue;
}Note: Cache busting with ESM requires query string or other techniques since import() caches modules.
📝 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.
| try { | |
| delete require.cache[require.resolve(file)]; | |
| const { default: CommandClass } = await require(file); | |
| if (!CommandClass || !(CommandClass.prototype instanceof Command)) { | |
| continue; | |
| } | |
| try { | |
| const modulePath = `file://${file}?update=${Date.now()}`; | |
| const { default: CommandClass } = await import(modulePath); | |
| if (!CommandClass || !(CommandClass.prototype instanceof Command)) { | |
| continue; | |
| } |
🤖 Prompt for AI Agents
In `@src/handlers/CommandHandler.ts` around lines 18 - 24, The loader uses
require()/require.resolve which breaks for ESM; switch to dynamic import() for
cross-compatibility: replace delete require.cache/require.resolve and await
require(file) with await import(file) (use a cache-bust query string if you need
to avoid import() caching), then extract the class with something like const
CommandClass = module.default ?? module and keep the instanceof check
(CommandClass.prototype instanceof Command) to validate the class; update the
CommandHandler loading logic to use import() and remove require.cache usage.
| public async registerCommands() { | ||
| if (!this.client.token || !this.client.application) return; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Silent early return obscures why command registration was skipped.
When token or application is missing, the method returns silently. Callers have no indication that registration didn't happen. Consider logging a warning.
Suggested improvement
public async registerCommands() {
- if (!this.client.token || !this.client.application) return;
+ if (!this.client.token) {
+ this.client.logger.warn("Cannot register commands: no token provided.");
+ return;
+ }
+ if (!this.client.application) {
+ this.client.logger.warn("Cannot register commands: client application not ready. Call this after the 'ready' event.");
+ return;
+ }📝 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.
| public async registerCommands() { | |
| if (!this.client.token || !this.client.application) return; | |
| public async registerCommands() { | |
| if (!this.client.token) { | |
| this.client.logger.warn("Cannot register commands: no token provided."); | |
| return; | |
| } | |
| if (!this.client.application) { | |
| this.client.logger.warn("Cannot register commands: client application not ready. Call this after the 'ready' event."); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In `@src/handlers/CommandHandler.ts` around lines 41 - 42, The registerCommands
method currently returns silently when this.client.token or
this.client.application is missing; update registerCommands to log a clear
warning before returning so callers can see why registration was skipped (e.g.,
use an existing logger or console.warn). Locate the check in registerCommands
that inspects this.client.token and this.client.application and add a
descriptive warning message including which property is missing (token and/or
application) prior to the early return.
| private getFiles(dir: string, fileList: string[] = []): string[] { | ||
| if (!fs.existsSync(dir)) return []; | ||
| const files = fs.readdirSync(dir); | ||
| for (const file of files) { | ||
| const filePath = path.join(dir, file); | ||
| if (fs.statSync(filePath).isDirectory()) { | ||
| this.getFiles(filePath, fileList); | ||
| } else if (file.endsWith(".ts") || file.endsWith(".js")) { | ||
| fileList.push(filePath); | ||
| } | ||
| } | ||
| return fileList; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Synchronous file system operations may block the event loop.
fs.readdirSync and fs.statSync are blocking calls. For large command directories, consider using async alternatives (fs.promises.readdir, fs.promises.stat) to avoid blocking.
🤖 Prompt for AI Agents
In `@src/handlers/CommandHandler.ts` around lines 130 - 142, The getFiles function
uses blocking fs calls; convert it to an async function (e.g., async
getFiles(dir: string, fileList: string[] = []): Promise<string[]>) and replace
fs.readdirSync/fs.statSync with fs.promises.readdir and fs.promises.stat
(awaiting results), recursively await calls to this.getFiles for subdirectories,
push .ts/.js file paths to fileList, and propagate errors or handle them
gracefully; update all call sites to await the new Promise-returning getFiles
and adjust types accordingly (retain use of path.join and the same file
extension checks).
| delete require.cache[require.resolve(file)]; | ||
| const { default: EventClass } = await require(file); | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Potential ESM incompatibility: replace require if the build outputs ESM.
Line 17-19 uses require, which is undefined in ESM. Please confirm the module target (type: "module" / TS module setting). If ESM is possible, switch to import() (or createRequire) to avoid runtime failure.
♻️ Suggested ESM-safe import (preserves cache-busting)
-import { Client } from "../structures/Client";
-import { Event } from "../structures/Event";
-import fs from "fs";
-import path from "path";
+import { Client } from "../structures/Client";
+import { Event } from "../structures/Event";
+import fs from "fs";
+import path from "path";
+import { pathToFileURL } from "url";
@@
- delete require.cache[require.resolve(file)];
- const { default: EventClass } = await require(file);
+ const moduleUrl = `${pathToFileURL(file).href}?update=${Date.now()}`;
+ const { default: EventClass } = await import(moduleUrl);📝 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.
| delete require.cache[require.resolve(file)]; | |
| const { default: EventClass } = await require(file); | |
| const moduleUrl = `${pathToFileURL(file).href}?update=${Date.now()}`; | |
| const { default: EventClass } = await import(moduleUrl); |
| if (event.once) { | ||
| this.client.once(event.name, (...args) => event.execute(...args)); | ||
| } else { | ||
| this.client.on(event.name, (...args) => event.execute(...args)); | ||
| } |
There was a problem hiding this comment.
Handle sync/async errors from event handlers to avoid unhandled rejections.
event.execute can throw or reject, which will bubble as unhandled errors on the event loop. Wrap execution so failures are logged and don’t crash the process.
🐛 Suggested guard for handler execution
- if (event.once) {
- this.client.once(event.name, (...args) => event.execute(...args));
- } else {
- this.client.on(event.name, (...args) => event.execute(...args));
- }
+ const run = (...args: any[]) => {
+ void Promise.resolve()
+ .then(() => event.execute(...args))
+ .catch((err) =>
+ this.client.logger.error(`Error in event ${event.name}:`, err)
+ );
+ };
+ if (event.once) {
+ this.client.once(event.name, run);
+ } else {
+ this.client.on(event.name, run);
+ }📝 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.
| if (event.once) { | |
| this.client.once(event.name, (...args) => event.execute(...args)); | |
| } else { | |
| this.client.on(event.name, (...args) => event.execute(...args)); | |
| } | |
| const run = (...args: any[]) => { | |
| void Promise.resolve() | |
| .then(() => event.execute(...args)) | |
| .catch((err) => | |
| this.client.logger.error(`Error in event ${event.name}:`, err) | |
| ); | |
| }; | |
| if (event.once) { | |
| this.client.once(event.name, run); | |
| } else { | |
| this.client.on(event.name, run); | |
| } |
🤖 Prompt for AI Agents
In `@src/handlers/EventHandler.ts` around lines 25 - 29, The event handlers
currently call event.execute(...) directly on this.client.once/on which can
throw or return a rejected promise; wrap the handler call in an async wrapper
that catches both sync and async errors (e.g., await
Promise.resolve(event.execute(...args)) inside try/catch) and log the error (use
an existing logger like this.logger.error if available, falling back to
console.error) so failures are handled and don't become unhandled rejections.
| constructor(data: { | ||
| name: string; | ||
| description: string; | ||
| type: ApplicationCommandOptionType; | ||
| required?: boolean; | ||
| choices?: { name: string; value: string | number }[]; | ||
| }) { | ||
| this.name = data.name; | ||
| this.description = data.description; | ||
| this.type = data.type; | ||
| this.required = data.required ?? false; | ||
| this.choices = data.choices; | ||
| } | ||
|
|
||
| public toJSON(): ApplicationCommandOptionData { | ||
| return { | ||
| name: this.name, | ||
| description: this.description, | ||
| type: this.type, | ||
| required: this.required, | ||
| choices: this.choices, | ||
| } as ApplicationCommandOptionData; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Validate choices against supported option types.
Discord only accepts choices for specific option types (e.g., STRING/INTEGER/NUMBER). A guard here would fail fast and avoid 400s during command registration.
♻️ Suggested validation guard
constructor(data: {
name: string;
description: string;
type: ApplicationCommandOptionType;
required?: boolean;
choices?: { name: string; value: string | number }[];
}) {
this.name = data.name;
this.description = data.description;
this.type = data.type;
this.required = data.required ?? false;
this.choices = data.choices;
+ if (
+ this.choices &&
+ ![
+ ApplicationCommandOptionType.String,
+ ApplicationCommandOptionType.Integer,
+ ApplicationCommandOptionType.Number,
+ ].includes(this.type)
+ ) {
+ throw new Error(
+ "`choices` is only valid for String/Integer/Number option types."
+ );
+ }
}📝 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.
| constructor(data: { | |
| name: string; | |
| description: string; | |
| type: ApplicationCommandOptionType; | |
| required?: boolean; | |
| choices?: { name: string; value: string | number }[]; | |
| }) { | |
| this.name = data.name; | |
| this.description = data.description; | |
| this.type = data.type; | |
| this.required = data.required ?? false; | |
| this.choices = data.choices; | |
| } | |
| public toJSON(): ApplicationCommandOptionData { | |
| return { | |
| name: this.name, | |
| description: this.description, | |
| type: this.type, | |
| required: this.required, | |
| choices: this.choices, | |
| } as ApplicationCommandOptionData; | |
| constructor(data: { | |
| name: string; | |
| description: string; | |
| type: ApplicationCommandOptionType; | |
| required?: boolean; | |
| choices?: { name: string; value: string | number }[]; | |
| }) { | |
| this.name = data.name; | |
| this.description = data.description; | |
| this.type = data.type; | |
| this.required = data.required ?? false; | |
| this.choices = data.choices; | |
| if ( | |
| this.choices && | |
| ![ | |
| ApplicationCommandOptionType.String, | |
| ApplicationCommandOptionType.Integer, | |
| ApplicationCommandOptionType.Number, | |
| ].includes(this.type) | |
| ) { | |
| throw new Error( | |
| "`choices` is only valid for String/Integer/Number option types." | |
| ); | |
| } | |
| } | |
| public toJSON(): ApplicationCommandOptionData { | |
| return { | |
| name: this.name, | |
| description: this.description, | |
| type: this.type, | |
| required: this.required, | |
| choices: this.choices, | |
| } as ApplicationCommandOptionData; |
🤖 Prompt for AI Agents
In `@src/structures/Argument.ts` around lines 13 - 34, The Argument class
currently accepts choices regardless of option type; add a validation in the
constructor (or a small private validator called from the constructor) to ensure
when data.choices is provided the data.type is one of the allowed types
(ApplicationCommandOptionType.String, Integer, Number) and that each
choice.value matches the expected JS type (string for String, number for
Integer/Number); if validation fails, either throw a clear Error (e.g., "choices
are only allowed for STRING/INTEGER/NUMBER options") or remove data.choices so
toJSON won't include invalid choices. Update the constructor (and keep toJSON
unchanged) to perform this guard and reference the Argument class, constructor,
choices property, and ApplicationCommandOptionType constants.
| export class Client extends DiscordClient { | ||
| public readonly logger: LoggerInstance; | ||
| public readonly commands: Collection<string, Command>; | ||
| public readonly aliases: Collection<string, string>; | ||
| public readonly prefix: string; | ||
|
|
||
| public readonly commandHandler: CommandHandler; | ||
| public readonly eventHandler: EventHandler; | ||
|
|
||
| constructor(opts: ClientOptionsWithFramework) { | ||
| super(opts); | ||
| this.logger = new LoggerInstance(opts.logLevel ?? "log"); | ||
| this.commands = new Collection(); | ||
| this.aliases = new Collection(); | ||
| this.prefix = opts.prefix ?? "!"; | ||
|
|
||
| if (opts.token) this.token = opts.token; | ||
|
|
||
| this.commandHandler = new CommandHandler(this); | ||
| this.eventHandler = new EventHandler(this); | ||
|
|
||
| this.on("messageCreate", (msg) => this.commandHandler.handleMessage(msg)); | ||
| this.on("interactionCreate", (int) => | ||
| this.commandHandler.handleInteraction(int) | ||
| ); | ||
|
|
||
| // Auto-load events | ||
| this.eventHandler | ||
| .loadEvents(path.join(__dirname, "..", "events")) | ||
| .catch((error) => this.logger.error("Error loading events:", error)); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Constructor performs async side effects without awaiting.
The constructor initiates async event loading via .catch() but doesn't provide a way for the caller to know when loading completes or if it failed. Consider providing a separate init() or start() method for async initialization, or expose a promise/ready event.
🤖 Prompt for AI Agents
In `@src/structures/Client.ts` around lines 9 - 39, The constructor currently
calls this.eventHandler.loadEvents(...) and only attaches .catch(), causing
async initialization to run unnoticed; refactor by removing the async call from
the constructor and add a public async init() or start() method on Client (e.g.,
async init(): Promise<void>) that awaits
this.eventHandler.loadEvents(path.join(__dirname, "..", "events")) and rethrows
or logs errors for callers to handle, or alternatively expose a public ready
Promise property (e.g., this.ready = this.eventHandler.loadEvents(...)) that
callers can await; ensure all references to loadEvents are moved to the new
init/start/ready mechanism and the constructor only synchronously sets up
properties and listeners (keep this.eventHandler and this.commandHandler
instantiation in constructor but do not call loadEvents there).
| // Auto-load events | ||
| this.eventHandler | ||
| .loadEvents(path.join(__dirname, "..", "events")) | ||
| .catch((error) => this.logger.error("Error loading events:", error)); |
There was a problem hiding this comment.
Hardcoded event path may not work for framework consumers.
The path path.join(__dirname, "..", "events") points to a location within the framework package itself, not the consumer's project. Framework users would expect to define their own events in their project directory.
Consider making the events directory configurable via ClientOptionsWithFramework, or removing auto-loading from the constructor and letting users call loadEvents() explicitly with their own path.
Suggested approach
constructor(opts: ClientOptionsWithFramework) {
super(opts);
// ... existing initialization ...
- // Auto-load events
- this.eventHandler
- .loadEvents(path.join(__dirname, "..", "events"))
- .catch((error) => this.logger.error("Error loading events:", error));
+ // Auto-load events if path provided
+ if (opts.eventsPath) {
+ this.eventHandler
+ .loadEvents(opts.eventsPath)
+ .catch((error) => this.logger.error("Error loading events:", error));
+ }
}🤖 Prompt for AI Agents
In `@src/structures/Client.ts` around lines 35 - 38, The constructor currently
auto-loads events from a hardcoded path using
this.eventHandler.loadEvents(path.join(__dirname, "..", "events")), which points
inside the framework package and breaks consumer projects; update the Client
constructor to accept a configurable eventsDir via ClientOptionsWithFramework
(or remove the auto-load) and only call this.eventHandler.loadEvents(eventsDir)
when that option is provided, or instead remove the load call entirely and
document that consumers must call eventHandler.loadEvents(...) themselves;
modify the constructor signature to read eventsDir from
ClientOptionsWithFramework and use that value (or skip calling
this.eventHandler.loadEvents) and ensure error handling remains in the catch
block for eventHandler.loadEvents.
| public get author(): User { | ||
| return this.interaction?.user ?? this.message!.author; | ||
| } | ||
|
|
||
| public get guild(): Guild | null { | ||
| return this.interaction?.guild ?? this.message!.guild; | ||
| } |
There was a problem hiding this comment.
Non-null assertions will throw when Context has only an interaction.
When Context is constructed with only an interaction (no message), accessing author or guild will crash because this.message! throws on undefined. The channel getter (line 46-47) correctly uses optional chaining, but these do not.
Proposed fix
public get author(): User {
- return this.interaction?.user ?? this.message!.author;
+ return (this.interaction?.user ?? this.message?.author)!;
}
public get guild(): Guild | null {
- return this.interaction?.guild ?? this.message!.guild;
+ return this.interaction?.guild ?? this.message?.guild ?? null;
}🤖 Prompt for AI Agents
In `@src/structures/Context.ts` around lines 37 - 43, The getters author and guild
in Context use non-null assertions on this.message which will throw when Context
contains only an interaction; change the implementations to use safe optional
chaining and nullish coalescing (e.g., author -> this.interaction?.user ??
this.message?.author and guild -> this.interaction?.guild ??
this.message?.guild) and update the return types on the author and guild getters
to allow undefined/null (e.g., author: User | undefined, guild: Guild | null |
undefined) to match the channel getter's safe behavior.
|
Duplicate of #2 |
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.