Boilerplate Refactor#3
Conversation
…ed logging The old architecture used a glob-based loadStructures() to discover commands and events at runtime, validating each with zod predicates. This made startup order implicit and required a separate deploy.ts script for command registration. Commands are now a static Collection built from explicit imports. Events are plain async functions registered directly on the client. Command registration moves into the ready handler so startup is self-contained. A BotContext object handles dependency injection (logger, and future shared resources) into handlers. Also: - add pino for structured logging (replaces console.log) - add @t3-oss/env-core + zod for validated env vars on startup - fix format/format:check script names (were swapped) - add --env-file=.env --watch to dev script - add .editorconfig for consistent editor settings
tj-dodd
left a comment
tj-dodd
left a comment
There was a problem hiding this comment.
this looks great, though i'd also like sync or rad to have a look at it as there's a lot of changes happening here
| /** | ||
| * Defines the structure of a command | ||
| * Shared state passed as the second argument to every command's `execute` handler. | ||
| * Add shared resources here as the bot grows (e.g. database clients, API wrappers). |
There was a problem hiding this comment.
I got this idea fom poise-rs, a rust discord library. In TS we I think we could have a global variable for this since we don't have to deal with the ownership and borrowing of rust? But I still like the way this works, so I'm glad others like it too :)
| * Secret environment variables only. Non-secret settings live in config/config.ts. | ||
| * Exits immediately with a clear error on startup if any required variable is missing. | ||
| */ | ||
| export default createEnv({ |
There was a problem hiding this comment.
something to think about is if discord changes the format for any of these this will be fail, but i don't really think this will be an issue
There was a problem hiding this comment.
That's something I thought about too, but I figured if that happens we would probably need to update discord.js and the codebase anyways.
| // once() so reconnects don't re-trigger startup logic like command registration | ||
| client.once( | ||
| Events.ClientReady, | ||
| async (readyClient) => await ready(readyClient, ctx), | ||
| ); |
There was a problem hiding this comment.
I wonder if we should just fire this as part of the deployment workflow instead of in the application?
There was a problem hiding this comment.
My thought is that this will automatically sync any new commands, which makes the dev workflow a easier and also simplifies deployment since we don't have to set up a script for every update. If you think it's better to make it part of the deployment workflow then we can definitely do that instead though!
There was a problem hiding this comment.
Yeah I think that's a good call out on the dev workflow side of things. Maybe let's keep this in mind as we move closer to actual deployment (cuz for serverless it might cause issues and realistically a separate step is easy to add into the CI).
| "dev": "tsx src/index.ts", | ||
| "format": "oxfmt --check", | ||
| "format:check": "oxfmt", | ||
| "dev": "tsx --env-file=.env --watch src/index.ts", |
There was a problem hiding this comment.
see if that mise fix works, otherwise, let's go with this :)
Co-authored-by: Elliot Alexander <elliot@kasa.au>
Co-authored-by: Elliot Alexander <elliot@kasa.au>
aarchangel64
force-pushed
the
template-refactor
branch
from
99c2a22 to
83307a9
Compare
- Fix zod import path: `from "zod/v4"` → `from "zod"` (zod v4 changed the subpath export) - Use ctx.logger.child() in event handlers instead of module-level import; use pino-conventional `err` key for Error objects - Exit process on command registration failure instead of swallowing it - Add unregister-commands utility script and npm run target - Fix emoji name typo: catsurpised → catsurprised - Remove unnecessary async/await wrappers on event handler lambdas
aarchangel64
force-pushed
the
template-refactor
branch
from
83307a9 to
227bc94
Compare
| @@ -1,2 +1,51 @@ | |||
| allowBuilds: | |||
| esbuild: true | |||
| minimumReleaseAgeExclude: | |||
There was a problem hiding this comment.
do we need these overrides? if we do, can we use a pattern instead please https://pnpm.io/settings#minimumreleaseageexclude
3 participants
This PR refactors a lot of the boilerplate, mainly around how commands and events are loaded (plus some other additions).
Instead of dynamically searching the source directories for events and commands at runtime, I feel it's a lot simpler and cleaner to just statically import them like you would with most other functions. I feel this also is a bit easier to understand, especially for contributors who might not be very familiar with ts/js (I saw a few people in the server who wanted to help out even though they hadn't used typescript before).
The bot now also automatically registers commands as guild commands on startup, so you don't need to run a separate deploy script every time you add or modify a command.
Minor Changes:
formatandformat:checkpackage.json script names were swapped? I'm not 100% sure on this one.--env-file=.envand--watchto the pnpm dev script (this fixes issues in fix: env loading and structure resolving #2 )