-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Switch from dynamic import to a more explicit approach #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i made some suggestions.
src/util/loaders.ts
Outdated
| import { docsCommands } from '../commands/docs/index.js'; | ||
| import { guidesCommand } from '../commands/guides/index.js'; | ||
| import { type Command, predicate as commandPredicate } from '../commands/index.js'; | ||
| import { pingCommand } from '../commands/ping.js'; | ||
| import { tipsCommands } from '../commands/tips/index.js'; | ||
| import { hasVarEvent } from '../events/has-var.js'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(suggestion) i recommend making a barrel file in /commands called index.ts which exports everything. then you don't need to import eveything in here, but instead import everything inside the command dir barrel file.
then you can do
// /command/index.ts
import { pingCommand } from "./ping";
// ... rest
export const commands = [pingCommand, tipsCommands, guidesCommand, docsCommands].flat();Here you can also then manually remove a command if it should not be used.
Same thing with events then.
and then in this file you would just do
import { commands } from "@/commands"
// ... rest of code
const loadCommands = (): Map<string, Command> => {
// no filter needed, since we know from the import that it only has the neseccary ones
return new Map(commands.map((command) => [command.data.name, command]));
};
src/util/loaders.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(question) What i don't understand, why do you import the commands and events here, just to manipulate them, and export them?
src/util/loaders.ts
Outdated
|
|
||
| /** | ||
| * | ||
| * @returns An array of events | ||
| */ | ||
| const loadEvents = (): DiscordEvent[] => { | ||
| const events = [hasVarEvent, readyEvent, justAskEvent, interactionCreateEvent].filter( | ||
| eventPredicate | ||
| ); | ||
| return events as DiscordEvent[]; | ||
| }; | ||
|
|
||
| /** | ||
| * | ||
| * @returns A map of command names to commands | ||
| */ | ||
| const loadCommands = (): Map<string, Command> => { | ||
| const commands = [pingCommand, tipsCommands, guidesCommand, docsCommands].flat(); | ||
| return new Map(commands.filter(commandPredicate).map((command) => [command.data.name, command])); | ||
| }; | ||
|
|
||
| export const commands = loadCommands(); | ||
| export const events = loadEvents(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(suggestion) as mentioned in my other comment, use a barrel file. in the barrel file you prepare the data as you need it.
so the loadCommands function then becomes this in the barrel file
export const commands = new Map([pingCommand, tipsCommands, guidesCommand, docsCommands].flat().map((command) => [command.data.name, command]));There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
No description provided.