-
Notifications
You must be signed in to change notification settings - Fork 0
Feat: i18next #7
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
📝 WalkthroughWalkthroughIntroduces internationalization support via i18next with file-based backend. Refactors module imports to use path aliases ( Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Client as Client
participant I18n as i18next
participant Backend as FS Backend
participant Context as Context
participant Handler as Event Handler
App->>Client: new Client({ i18n, includePaths })
Client->>Client: login(token)
Client->>Client: loadDir(includePaths)
Client->>Backend: load locale files
Backend-->>I18n: read JSON files
Client->>I18n: i18n.init()
I18n-->>Client: initialized
Client->>Client: proceed to login
Handler->>Handler: interact/message event
Handler->>Context: new Context(client, payload)
Context->>Context: locale = interaction.locale
Handler->>Context: t("key", args)
Context->>I18n: i18n.t(key, args)
I18n-->>Context: translated string
Context-->>Handler: result
Handler-->>App: response with translation
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 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 |
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.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@src/structures/Client.ts`:
- Around line 48-52: The constructor in Client currently calls the async method
i18n.loadLocales without awaiting it, causing locales to possibly be
unavailable; add an initialization mechanism (either an async init() method on
the Client class that callers must await, or a public readonly ready:
Promise<void> initialized from a private async initializeLocales() called in the
constructor) and move the loadLocales call into that async path (referencing
i18n.loadLocales, Client.constructor, and the new init/ready/initializeLocales
methods) so callers can await client.init() or await client.ready before using
client.i18n.t().
In `@src/structures/I18n.ts`:
- Around line 57-61: The interpolation loop in I18n.ts using new
RegExp(`{${k}}`, "g") can misbehave and risk ReDoS when keys contain regex
metacharacters; update the loop in the method containing this logic to either
perform a safe literal string replacement or escape the key before building the
RegExp (e.g., add an escapeRegex helper that replaces /[.*+?^${}()|[\]\\]/g with
an escaped form and use it when constructing the RegExp), then use that escaped
key in value.replace or switch to a non-regex string replace approach to
substitute String(v) for the placeholder.
- Line 21: The loadLocales function is failing because getFiles (in
src/utils/Files.ts) only looks for .ts/.js and therefore never returns .json
locale files; update the code by adding a JSON-specific file scanner (e.g., add
getJsonFiles(baseDir) in Files.ts that FastGlob.syncs "**/*.json" with the same
cwd/ignore/absolute options) and change loadLocales to call getJsonFiles(dir)
(or alternately pass a configurable glob pattern into getFiles) so loadLocales
actually discovers and loads .json locale files; reference getFiles,
getJsonFiles, and loadLocales when making the change.
- Around line 53-55: The method in src/structures/I18n.ts currently returns
non-string values (returns value when typeof value !== "string"), violating the
declared string return type; change the branch that handles non-strings to
coerce the value to a string (e.g., JSON.stringify(value)) and emit a warning
(console.warn or the existing logger) including the key and the original value,
so the function always returns a string; alternatively, if nested objects are
intentional, update the method signature to return string | Record<string, any>
and adjust callers accordingly.
In `@types/client.d.ts`:
- Around line 15-17: Add a short JSDoc comment to the I18nOptions interface
explaining the expected format for defaultLocale (e.g., BCP 47 language tags
like "en-US"); update the declaration for I18nOptions and the defaultLocale
property to include that comment so consumers immediately see the expected
locale format when viewing the type, referencing the I18nOptions interface and
the defaultLocale field.
📜 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 (7)
locales/en-US.jsonpackage.jsonsrc/structures/Client.tssrc/structures/Context.tssrc/structures/I18n.tstest-i18n.jstypes/client.d.ts
🧰 Additional context used
🧬 Code graph analysis (3)
src/structures/Client.ts (4)
types/client.d.ts (1)
FrameworkOptions(19-25)src/structures/I18n.ts (1)
I18n(5-65)test-i18n.js (1)
path(3-3)src/utils/Files.ts (1)
getProjectRoot(21-27)
src/structures/I18n.ts (3)
src/utils/logger/Logger.ts (2)
Logger(10-199)error(103-105)test-i18n.js (1)
logger(5-5)src/utils/Files.ts (1)
getFiles(4-20)
test-i18n.js (2)
src/utils/logger/Logger.ts (1)
Logger(10-199)src/structures/I18n.ts (1)
I18n(5-65)
🪛 ast-grep (0.40.5)
src/structures/I18n.ts
[warning] 58-58: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp({${k}}, "g")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (9)
locales/en-US.json (1)
1-9: LGTM!The locale file structure is correct with proper JSON format, placeholder syntax (
{user}), and nested key support that aligns with thelodash.getusage inI18n.t.test-i18n.js (1)
8-17: Test will fail due togetFilesnot finding JSON files.This test script demonstrates the i18n API correctly, but it will not work until the issue in
I18n.loadLocalesis fixed (seesrc/structures/I18n.tsreview). ThegetFilesfunction only scans for.tsand.jsfiles, so no locales will be loaded.Once the locale loading is fixed, the test logic looks correct for demonstrating translations and fallback behavior.
package.json (1)
46-46: LGTM!Minor patch version bump for
@types/nodetype definitions.src/structures/Client.ts (1)
34-34: LGTM!The
i18nproperty is correctly typed asI18nand declared as readonly.src/structures/Context.ts (3)
43-51: LGTM!The
t()method correctly uses the interaction's locale when available, falling back to the default locale for message-based contexts. This provides proper i18n support at the context level.
53-70: LGTM!The
toJSONmethod now consistently includesauthorin both interaction and message branches, providing a unified structure for serialization.
25-31: Type guards using property checks are reliable.The property checks
"user" in this.dataand"author" in this.datacorrectly distinguish between the two types. Per discord.js documentation,ChatInputCommandInteractionhas a documenteduserproperty (the User who invoked the command), whileMessagehas a documentedauthorproperty (a User object). These properties are mutually exclusive between the two types, making the type guards both safe and correct.types/client.d.ts (2)
4-8: LGTM:localespath fits the existing paths shape.This cleanly extends
FrameworkPathsfor i18n assets.
19-25: LGTM:i18noption added toFrameworkOptions.Matches the new configuration surface cleanly.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
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.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/structures/Client.ts (1)
47-74: AwaitincludePathsloading to avoid race conditions.
loadDiris async but invoked without awaiting, so commands/events may not be loaded when the client is used immediately after construction. Consider moving this into the asyncloginpath so it’s awaited.🐛 Proposed fix
- if (this.options.includePaths) { - for (const p of this.options.includePaths) { - this.loadDir(path.join(getProjectRoot(), p)).catch((error) => - this.logger.error("Error loading events:", error) - ); - } - } + // load in async flow (see login) ... override async login(token?: string) { + if (this.options.includePaths) { + for (const p of this.options.includePaths) { + await this.loadDir(path.join(getProjectRoot(), p)).catch((error) => + this.logger.error("Error loading events:", error) + ); + } + } if (this.i18n) { await this.i18n.init(); } return super.login(token); }types/client.d.ts (1)
5-22: MakeincludePathsoptional (default already exists) and clarifyFrameworkPaths.locales.
RequiringincludePathsforces TS users to set a value even thoughClientsupplies a default. Also,FrameworkPaths.localesis now unreachable sinceFrameworkOptionsdroppedpaths.♻️ Proposed tweak
- includePaths: string[]; + includePaths?: string[];If
localesis still intended to be configurable, consider re‑exposing it through options or removingFrameworkPathsto avoid unused API surface.src/structures/Context.ts (1)
1-25: Initializelocale(or mark it optional) to avoid undefined at runtime.
localeis declared as non-optional but never assigned in the constructor. WhilestrictPropertyInitializationis not enabled intsconfig.json, the field remainsundefinedat runtime unless set elsewhere. Thet()method handles this with a fallback operator, but the type contract is violated.Note: The proposed fix only initializes
localefor theChatInputCommandInteractioncase. SinceMessagetype has nolocaleproperty, you'll need to either:
- Keep
localeoptional (recommended)- Set a default fallback for the
Messagecase as well
🤖 Fix all issues with AI agents
In `@examples/basic_client/index.js`:
- Around line 1-29: The interpolation option was passed to the arox.Client
constructor but is ignored — move the interpolation config into the i18next
instance and remove it from the Client options: add the interpolation object
(e.g., escapeValue: false) into the myinstance = i18next.createInstance({...})
config and delete the interpolation property from the new Client({ ... }) call
so configuration lives with i18next (references: myinstance,
i18next.createInstance, Client).
In `@src/events/interaction.ts`:
- Line 13: The reply option is using the string literal "Ephemeral" for the
flags field which can break on discord.js versions that expect an enum/bitfield
or the ephemeral boolean; confirm your discord.js version and replace the string
usage (the flags property in the reply/interaction responses) with the correct
type—either set ephemeral: true on the InteractionReplyOptions or use the
MessageFlags/MessageFlagsBitField enum/flags constant (e.g.,
MessageFlags.Ephemeral or the appropriate BitField flag) for type-safety; apply
this change to every occurrence of flags: "Ephemeral" in this file (the flags
property references shown in the diff).
In `@src/structures/Context.ts`:
- Around line 45-58: Fix the typo in the error message in Context.t (change
"initalized" to "initialized") and harden how the fallback locale is derived: in
the t method of Context, when computing locale from
this.client.i18n.options.fallbackLng, handle the possible types (string, array,
object, function, false) and only pass a string, string[] or null to
this.client.i18n.getFixedT; e.g., if fallbackLng is an array use its first
string element (or null if empty/invalid), if it's a string use it, and for
object/function/false fall back to null (or a safe default string), then call
getFixedT(locale) and proceed as before.
In `@src/utils/logger/Logger.ts`:
- Line 9: The i18next Logger mapping currently routes the generic `log` level to
`debug`, which suppresses default i18next output; update the mapping so `log`
maps to `info` (or make this behavior configurable) in the LoggerModule
implementation (refer to the `LoggerModule` export and the `log` level handling
code in Logger.ts), and ensure the same change is applied to the other mapping
block referenced around lines 395-408; adjust any configuration/API (e.g., a
flag or option name) so callers can opt into `debug` routing if desired.
📜 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 (12)
.github/workflows/changelog.ymlexamples/basic_client/index.jsexamples/basic_client/locales/en-US/test.jsonpackage.jsonsrc/events/interaction.tssrc/events/message.tssrc/index.tssrc/structures/Client.tssrc/structures/Context.tssrc/utils/logger/Logger.tssrc/utils/util.tstypes/client.d.ts
💤 Files with no reviewable changes (1)
- .github/workflows/changelog.yml
🧰 Additional context used
🧬 Code graph analysis (4)
src/events/interaction.ts (2)
examples/basic_client/index.js (3)
ctx(42-42)ctx(46-46)command(32-37)src/structures/Context.ts (1)
Context(9-80)
src/structures/Context.ts (1)
src/structures/Client.ts (1)
Client(23-130)
src/events/message.ts (1)
src/utils/util.ts (1)
deleteMessageAfterSent(4-14)
examples/basic_client/index.js (1)
src/structures/Context.ts (2)
t(45-58)author(35-43)
🔇 Additional comments (9)
src/utils/util.ts (1)
4-14: Rename clarifies intent.
No behavior change; the new name better communicates the delayed deletion behavior.src/events/message.ts (1)
4-4: Updated helper name is consistent.
Import and usage match the renamed utility and preserve the cleanup flow.Also applies to: 34-34
package.json (1)
41-42: Verify new i18n dependency compatibility.
Please confirm these versions align with your supported Node runtime and CommonJS packaging expectations.Also applies to: 48-48
examples/basic_client/locales/en-US/test.json (1)
1-2: Example translation looks good.
Interpolation placeholder is clear and ready for runtime substitution.src/index.ts (1)
9-9: Confirm version token replacement in release flow.
Ensure your build/publish pipeline replaces the placeholder so consumers don’t see the raw token.src/structures/Context.ts (2)
27-33: Type-guard change looks good.
Using property checks keeps the guards lightweight and avoids brittleinstanceofpaths.
60-78:toJSONextension looks good.
Bindingtand includingauthor/argsmakes the payload more useful for consumers.src/structures/Client.ts (1)
15-45: i18n adapter wiring is clean and keeps logging consistent.
Nice integration with the internal logger for i18next visibility.examples/basic_client/index.js (1)
41-48: Contextt()usage is clear and concise.
Nice example of locale‑aware replies in both message and interaction paths.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/events/message.ts (1)
38-38: Consider propagating locale for message-based commands.The interaction handler at
src/events/interaction.ts(line 19) setsctx.locale = interaction.locale, but message-based commands don't have locale propagation. This meansContext.t()will fall back to the default language for all prefix commands.If locale support for message commands is desired, consider detecting locale from the guild or user settings, or documenting this as an intentional limitation.
🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 23-26: The imports field currently maps "#utils", "#structures",
and "#ctx" to "./src/…/*.ts" which leaves runtime consumers pointing at
TypeScript sources; update the package.json "imports" entries to point to the
compiled JavaScript in "dist/" for runtime (e.g. "#utils" ->
"./dist/utils/index.js", "#structures" -> "./dist/structures/index.js", "#ctx"
-> "./dist/context.js") and add a "types" condition for each that points to the
corresponding .d.ts in "dist/" (or your types output) so TypeScript still
resolves declarations; modify the imports block (the "imports" field) to
reference "dist/..." for runtime and "types" for declarations accordingly.
In `@src/structures/core/Client.ts`:
- Around line 61-73: The login override starts asynchronous loads for
this.options.includePaths via this.loadDir without awaiting them, so startup
proceeds before handlers are registered; change it to await the loads (e.g.,
await Promise.all(this.options.includePaths.map(p =>
this.loadDir(path.join(getProjectRoot(), p)).catch(err =>
this.logger.error("Error loading events:", err)))) ) before running i18n.init()
and returning super.login(token) so all includePaths are loaded (and errors
logged) prior to login; keep using this.loadDir, this.options.includePaths,
this.logger, i18n.init(), and super.login in the updated flow.
♻️ Duplicate comments (3)
examples/basic_client/index.js (1)
7-29: Initialize i18next only after registering the backend.
createInstance({ ... })may initialize beforeuse(backend), which can prevent the FS backend from loading and leave the instance uninitialized when passed to the client. PrefercreateInstance()→use(backend)→init({ ... })(e.g., insideinit()before login).🐛 Proposed fix
-const myinstance = i18next.createInstance({ - supportedLngs: ["en-US", "tr"], - fallbackLng: "en-US", - defaultNS: "translation", - ns: ["translation", "test"], - backend: { - loadPath: path.join(__dirname, "locales/{{lng}}/{{ns}}.json"), - }, - interpolation: { - escapeValue: false, - }, -}); -myinstance.use(backend); +const myinstance = i18next.createInstance(); +myinstance.use(backend);Also initialize before login (outside this line range):
// inside init() await myinstance.init({ supportedLngs: ["en-US", "tr"], fallbackLng: "en-US", defaultNS: "translation", ns: ["translation", "test"], backend: { loadPath: path.join(__dirname, "locales/{{lng}}/{{ns}}.json") }, interpolation: { escapeValue: false }, });In i18next v25, does createInstance(options) automatically call init, and must use(backend) be called before init for i18next-fs-backend?src/events/interaction.ts (1)
12-12: LGTM!Using
MessageFlags.Ephemeralinstead of the string literal or boolean is the type-safe approach for discord.js v14+.src/structures/builder/Context.ts (1)
45-58: Guard non‑stringfallbackLngbeforegetFixedT.
fallbackLngcan be object/function/false; the current logic may pass those intogetFixedT, causing runtime errors.✅ Proposed fix
- let locale = - this.locale ?? - (Array.isArray(this.client.i18n.options.fallbackLng) - ? this.client.i18n.options.fallbackLng[0] - : this.client.i18n.options.fallbackLng) ?? - "en"; - - const t = this.client.i18n.getFixedT(locale); + const i18n = this.client.i18n; + const fallback = + Array.isArray(i18n.options.fallbackLng) + ? i18n.options.fallbackLng[0] + : typeof i18n.options.fallbackLng === "string" + ? i18n.options.fallbackLng + : undefined; + const locale = + this.locale ?? i18n.resolvedLanguage ?? i18n.language ?? fallback ?? "en"; + const t = i18n.getFixedT(locale);i18next fallbackLng type definition and getFixedT parameter types (v25)
📜 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 (16)
examples/basic_client/index.jspackage.jsonsrc/context.tssrc/events/interaction.tssrc/events/message.tssrc/events/ready.tssrc/index.tssrc/structures/builder/Argument.tssrc/structures/builder/Command.tssrc/structures/builder/Context.tssrc/structures/builder/Event.tssrc/structures/builder/index.tssrc/structures/core/Client.tssrc/structures/core/index.tssrc/structures/index.tssrc/utils/index.ts
🧰 Additional context used
🧬 Code graph analysis (6)
src/events/message.ts (1)
src/utils/util.ts (1)
deleteMessageAfterSent(4-14)
src/structures/builder/Command.ts (2)
src/structures/builder/Context.ts (1)
Context(9-81)types/extra.d.ts (1)
MaybePromise(1-1)
src/events/interaction.ts (1)
src/structures/builder/Context.ts (1)
Context(9-81)
src/structures/builder/Context.ts (1)
src/structures/core/Client.ts (1)
Client(28-134)
src/structures/core/Client.ts (3)
types/client.d.ts (1)
FrameworkOptions(16-22)src/utils/logger/Logger.ts (1)
I18nLoggerAdapter(395-409)src/utils/Files.ts (1)
getProjectRoot(21-27)
examples/basic_client/index.js (1)
src/structures/builder/Context.ts (2)
t(45-59)author(35-43)
🔇 Additional comments (24)
src/structures/builder/Argument.ts (1)
28-37: LGTM — the validation is a solid guardrail.src/utils/index.ts (1)
1-3: LGTM — clean barrel export surface.examples/basic_client/index.js (1)
41-51: LGTM — localized replies withctx.tread cleanly.src/structures/builder/index.ts (1)
1-4: LGTM — builder barrel exports are clean.src/events/ready.ts (1)
2-2: LGTM.package.json (1)
44-45: Confirm target Node.js version for@types/nodealignment.
i18next@^25.8.0andi18next-fs-backend@^2.6.1are confirmed compatible (the backend package has no peer dependency constraints, and i18next's own devDependencies list this version). However,@types/node@^25.0.9provides type definitions for Node.js 25.x specifically. Without an explicit"engines"field or.nvmrcfile in the repository, the target Node version is unclear. Ensure@types/nodemajor version aligns with your actual runtime Node version (e.g., Node 20 →@types/node@20, Node 22 →@types/node@22).Also applies to: 51-51
src/structures/core/index.ts (1)
1-1: LGTM!Clean barrel export aligning with the centralized module structure.
src/events/message.ts (1)
2-3: LGTM!Import updates align with the new alias-based module structure.
src/structures/index.ts (1)
1-2: LGTM!Proper barrel file aggregating core and builder exports for the
#structuresalias.src/context.ts (1)
1-1: LGTM!Import path updated to use the centralized
#structuresalias.src/structures/builder/Event.ts (2)
3-5: LGTM!Import paths updated to use centralized aliases (
#ctx,#structures,#utils), consistent with the project-wide refactor.
55-56: Good improvement.Moving the debug log after
this.bound = trueensures the log accurately reflects that the event has been successfully registered.src/events/interaction.ts (2)
1-2: LGTM!Good use of
MessageFlagsenum import for type-safe ephemeral flag handling.
17-22: LGTM!Locale propagation and debug logging are well-implemented. Setting
ctx.localeenables theContext.t()translation method to use the user's Discord client locale.src/structures/core/Client.ts (4)
8-21: Import consolidation looks good.Centralized aliases and i18n-related imports keep this module consistent with the new structure.
23-26: Sensible default forincludePaths.Defaults preserve expected events/commands loading behavior.
35-50: i18n wiring onClientis clean.Attaching the logger adapter before initialization is a good touch.
75-84: Directory existence guard is helpful.Logging missing directories at debug level is a good operational touch.
src/structures/builder/Context.ts (2)
1-17: Locale plumbing and constructor update look good.Cleanly threads Client and locale into Context.
61-79: Bindingtinto JSON is a nice touch.This makes downstream translation ergonomic without extra wiring.
src/structures/builder/Command.ts (3)
6-15: Import aliasing and context type aliases are clear.Improves readability and centralizes access.
36-38: Handler fields now typed to the context JSON.Keeps handler contracts aligned with Context output.
102-109: Setter signatures align with new context types.Consistent with
_onMessage/_onInteractiontyping.src/index.ts (1)
1-3: The alias configuration is properly set up. Package.json'simportsfield correctly maps#structuresand#ctxto their respective source files, and withmoduleResolution: "nodenext"in tsconfig.json, TypeScript will resolve these aliases at compile time. Runtime resolution will work correctly—tsconfig.jsonpathsconfiguration is not required when using package.jsonimportswith thenodenextmodule resolution strategy.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| "#types/*": "./types/*", | ||
| "#utils": "./src/utils/index.ts", | ||
| "#structures": "./src/structures/index.ts", | ||
| "#ctx": "./src/context.ts" |
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.
🧩 Analysis chain
🏁 Script executed:
# Check for build config files
find . -maxdepth 2 -type f \( -name ".swcrc" -o -name "swc.config.*" -o -name "tsconfig.json" -o -name "tsconfig*.json" -o -name "webpack.config.*" -o -name "rollup.config.*" -o -name "vite.config.*" \) | head -20Repository: AroxBot/framework
Length of output: 85
🏁 Script executed:
# Check package.json for build script and main/exports configuration
cat -n package.json | head -60Repository: AroxBot/framework
Length of output: 2129
🏁 Script executed:
# Check if aliases are used in source
rg -n "#utils|#structures|#ctx" src -g "*.ts" | head -30Repository: AroxBot/framework
Length of output: 1213
🏁 Script executed:
cat -n .swcrcRepository: AroxBot/framework
Length of output: 449
🏁 Script executed:
cat -n tsconfig.jsonRepository: AroxBot/framework
Length of output: 1566
Imports aliases must resolve to compiled output, not TypeScript sources.
The imports field points to ./src/.../*.ts files, but after the SWC build, the compiled JavaScript in dist/ will still contain unresolved #utils, #structures, and #ctx imports (SWC's config does not rewrite aliases). Consumers without TypeScript support will fail at runtime. Update imports to reference dist/ with types conditions for declaration files:
Example fix
"imports": {
"#types/*": "./types/*",
- "#utils": "./src/utils/index.ts",
- "#structures": "./src/structures/index.ts",
- "#ctx": "./src/context.ts"
+ "#utils": {
+ "types": "./dist/utils/index.d.ts",
+ "default": "./dist/utils/index.js"
+ },
+ "#structures": {
+ "types": "./dist/structures/index.d.ts",
+ "default": "./dist/structures/index.js"
+ },
+ "#ctx": {
+ "types": "./dist/context.d.ts",
+ "default": "./dist/context.js"
+ }
}📝 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.
| "#types/*": "./types/*", | |
| "#utils": "./src/utils/index.ts", | |
| "#structures": "./src/structures/index.ts", | |
| "#ctx": "./src/context.ts" | |
| "#types/*": "./types/*", | |
| "#utils": { | |
| "types": "./dist/utils/index.d.ts", | |
| "default": "./dist/utils/index.js" | |
| }, | |
| "#structures": { | |
| "types": "./dist/structures/index.d.ts", | |
| "default": "./dist/structures/index.js" | |
| }, | |
| "#ctx": { | |
| "types": "./dist/context.d.ts", | |
| "default": "./dist/context.js" | |
| } |
🤖 Prompt for AI Agents
In `@package.json` around lines 23 - 26, The imports field currently maps
"#utils", "#structures", and "#ctx" to "./src/…/*.ts" which leaves runtime
consumers pointing at TypeScript sources; update the package.json "imports"
entries to point to the compiled JavaScript in "dist/" for runtime (e.g.
"#utils" -> "./dist/utils/index.js", "#structures" ->
"./dist/structures/index.js", "#ctx" -> "./dist/context.js") and add a "types"
condition for each that points to the corresponding .d.ts in "dist/" (or your
types output) so TypeScript still resolves declarations; modify the imports
block (the "imports" field) to reference "dist/..." for runtime and "types" for
declarations accordingly.
| override async login(token?: string) { | ||
| if (this.options.includePaths) { | ||
| for (const p of this.options.includePaths) { | ||
| this.loadDir(path.join(getProjectRoot(), p)).catch((error) => | ||
| this.logger.error("Error loading events:", error) | ||
| ); | ||
| } | ||
| } | ||
| if (this.i18n && !this.i18n.isInitialized) { | ||
| await this.i18n.init(); | ||
| } | ||
| return super.login(token); | ||
| } |
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.
Await includePaths loading before login.
Line 61-67 starts async loads without awaiting them, so login can proceed before events/commands are registered, causing missed handlers on startup.
✅ Proposed fix
override async login(token?: string) {
if (this.options.includePaths) {
- for (const p of this.options.includePaths) {
- this.loadDir(path.join(getProjectRoot(), p)).catch((error) =>
- this.logger.error("Error loading events:", error)
- );
- }
+ await Promise.all(
+ this.options.includePaths.map(async (p) => {
+ try {
+ await this.loadDir(path.join(getProjectRoot(), p));
+ } catch (error) {
+ this.logger.error(`Error loading directory ${p}:`, error);
+ }
+ })
+ );
}
if (this.i18n && !this.i18n.isInitialized) {
await this.i18n.init();
}
return super.login(token);
}🤖 Prompt for AI Agents
In `@src/structures/core/Client.ts` around lines 61 - 73, The login override
starts asynchronous loads for this.options.includePaths via this.loadDir without
awaiting them, so startup proceeds before handlers are registered; change it to
await the loads (e.g., await Promise.all(this.options.includePaths.map(p =>
this.loadDir(path.join(getProjectRoot(), p)).catch(err =>
this.logger.error("Error loading events:", err)))) ) before running i18n.init()
and returning super.login(token) so all includePaths are loaded (and errors
logged) prior to login; keep using this.loadDir, this.options.includePaths,
this.logger, i18n.init(), and super.login in the updated flow.
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.