Conversation
|
There was a problem hiding this comment.
Pull request overview
Release bump to 1.6.3, including dependency updates, ESLint configuration changes, and improved error logging ergonomics by accepting unknown errors.
Changes:
- Bump package version and update dependencies/devDependencies (notably ESLint v10, discord.js, i18next, TypeScript).
- Add
toError()helper and update logger/error handling to acceptunknowninstead ofany. - Update TypeScript/ESLint project configuration (tsconfig
include, neweslint.config.ts, removeeslint.config.mjs).
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adds include to ensure src/** is part of the TS program. |
| src/utils/logger.ts | Updates error logging to normalize unknown via toError(). |
| src/utils/index.ts | Re-exports the new error utility. |
| src/utils/error.ts | Introduces toError() helper for normalizing thrown values. |
| src/managers/EventManager.ts | Switches catch typing from any to unknown. |
| src/client/StelliaUtils.ts | Switches multiple catch typings from any to unknown. |
| package.json | Release version bump, dependency updates, and lint script change. |
| eslint.config.ts | Replaces previous ESLint flat config file with a TS-based config. |
| eslint.config.mjs | Removed in favor of eslint.config.ts. |
| pnpm-lock.yaml | Lockfile updates reflecting dependency changes. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| files: ["**/*.{js,mjs,cjs,ts,mts,cts}"], | ||
| plugins: { js }, | ||
| extends: ["js/recommended"], | ||
| languageOptions: { | ||
| globals: globals.node, | ||
| }, | ||
| }, | ||
| ts.configs.recommended, | ||
| importPlugin.flatConfigs.recommended, | ||
| importPlugin.flatConfigs.typescript, | ||
| prettierConfig, | ||
| { | ||
| files: ["**/*.{js,mjs,cjs,ts,mts,cts}"], | ||
| languageOptions: { | ||
| parserOptions: { | ||
| project: "./tsconfig.json", | ||
| sourceType: "module", | ||
| ecmaVersion: "latest", | ||
| }, | ||
| }, | ||
| rules: { | ||
| "import/namespace": "off", | ||
| "@typescript-eslint/explicit-function-return-type": "off", | ||
| "@typescript-eslint/no-unused-vars": ["warn", { argsIgnorePattern: "^_" }], | ||
| "@typescript-eslint/no-explicit-any": "warn", | ||
| "import/order": [ | ||
| "error", | ||
| { | ||
| groups: [ | ||
| "builtin", | ||
| "external", | ||
| "internal", | ||
| ["parent", "sibling", "index"], | ||
| "object", | ||
| "type", | ||
| ], | ||
| pathGroups: [ | ||
| { | ||
| pattern: "@/**", | ||
| group: "internal", | ||
| }, | ||
| ], | ||
| pathGroupsExcludedImportTypes: ["builtin"], | ||
| alphabetize: { | ||
| order: "asc", | ||
| caseInsensitive: true, | ||
| }, | ||
| }, | ||
| ], | ||
| "import/newline-after-import": ["error", { count: 1 }], | ||
| "import/no-unresolved": "error", | ||
| "import/no-duplicates": "error", | ||
| "no-console": "off", | ||
| "no-var": "error", | ||
| "prefer-const": "error", | ||
| }, | ||
| settings: { | ||
| "import/resolver": { | ||
| typescript: { | ||
| alwaysTryTypes: true, | ||
| project: "./tsconfig.json", | ||
| }, | ||
| node: { | ||
| extensions: [".js", ".ts"], | ||
| }, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
eslint.config.ts is formatted with 2-space indentation, but the repo Prettier config specifies tabs (useTabs: true, tabWidth: 4). Consider formatting this file with Prettier to keep indentation consistent across the repo.
| { | |
| files: ["**/*.{js,mjs,cjs,ts,mts,cts}"], | |
| plugins: { js }, | |
| extends: ["js/recommended"], | |
| languageOptions: { | |
| globals: globals.node, | |
| }, | |
| }, | |
| ts.configs.recommended, | |
| importPlugin.flatConfigs.recommended, | |
| importPlugin.flatConfigs.typescript, | |
| prettierConfig, | |
| { | |
| files: ["**/*.{js,mjs,cjs,ts,mts,cts}"], | |
| languageOptions: { | |
| parserOptions: { | |
| project: "./tsconfig.json", | |
| sourceType: "module", | |
| ecmaVersion: "latest", | |
| }, | |
| }, | |
| rules: { | |
| "import/namespace": "off", | |
| "@typescript-eslint/explicit-function-return-type": "off", | |
| "@typescript-eslint/no-unused-vars": ["warn", { argsIgnorePattern: "^_" }], | |
| "@typescript-eslint/no-explicit-any": "warn", | |
| "import/order": [ | |
| "error", | |
| { | |
| groups: [ | |
| "builtin", | |
| "external", | |
| "internal", | |
| ["parent", "sibling", "index"], | |
| "object", | |
| "type", | |
| ], | |
| pathGroups: [ | |
| { | |
| pattern: "@/**", | |
| group: "internal", | |
| }, | |
| ], | |
| pathGroupsExcludedImportTypes: ["builtin"], | |
| alphabetize: { | |
| order: "asc", | |
| caseInsensitive: true, | |
| }, | |
| }, | |
| ], | |
| "import/newline-after-import": ["error", { count: 1 }], | |
| "import/no-unresolved": "error", | |
| "import/no-duplicates": "error", | |
| "no-console": "off", | |
| "no-var": "error", | |
| "prefer-const": "error", | |
| }, | |
| settings: { | |
| "import/resolver": { | |
| typescript: { | |
| alwaysTryTypes: true, | |
| project: "./tsconfig.json", | |
| }, | |
| node: { | |
| extensions: [".js", ".ts"], | |
| }, | |
| }, | |
| }, | |
| }, | |
| { | |
| files: ["**/*.{js,mjs,cjs,ts,mts,cts}"], | |
| plugins: { js }, | |
| extends: ["js/recommended"], | |
| languageOptions: { | |
| globals: globals.node, | |
| }, | |
| }, | |
| ts.configs.recommended, | |
| importPlugin.flatConfigs.recommended, | |
| importPlugin.flatConfigs.typescript, | |
| prettierConfig, | |
| { | |
| files: ["**/*.{js,mjs,cjs,ts,mts,cts}"], | |
| languageOptions: { | |
| parserOptions: { | |
| project: "./tsconfig.json", | |
| sourceType: "module", | |
| ecmaVersion: "latest", | |
| }, | |
| }, | |
| rules: { | |
| "import/namespace": "off", | |
| "@typescript-eslint/explicit-function-return-type": "off", | |
| "@typescript-eslint/no-unused-vars": ["warn", { argsIgnorePattern: "^_" }], | |
| "@typescript-eslint/no-explicit-any": "warn", | |
| "import/order": [ | |
| "error", | |
| { | |
| groups: [ | |
| "builtin", | |
| "external", | |
| "internal", | |
| ["parent", "sibling", "index"], | |
| "object", | |
| "type", | |
| ], | |
| pathGroups: [ | |
| { | |
| pattern: "@/**", | |
| group: "internal", | |
| }, | |
| ], | |
| pathGroupsExcludedImportTypes: ["builtin"], | |
| alphabetize: { | |
| order: "asc", | |
| caseInsensitive: true, | |
| }, | |
| }, | |
| ], | |
| "import/newline-after-import": ["error", { count: 1 }], | |
| "import/no-unresolved": "error", | |
| "import/no-duplicates": "error", | |
| "no-console": "off", | |
| "no-var": "error", | |
| "prefer-const": "error", | |
| }, | |
| settings: { | |
| "import/resolver": { | |
| typescript: { | |
| alwaysTryTypes: true, | |
| project: "./tsconfig.json", | |
| }, | |
| node: { | |
| extensions: [".js", ".ts"], | |
| }, | |
| }, | |
| }, | |
| }, |
| return error; | ||
| } | ||
| if (typeof error === "object") { | ||
| return new Error(JSON.stringify(error)); |
There was a problem hiding this comment.
toError uses JSON.stringify(error) for non-Error objects. JSON.stringify can throw (e.g., circular references or BigInt values), which would make logger.errorWithInformation throw while handling an error. Wrap the stringify in a try/catch (or use a safe serializer like util.inspect) and fall back to String(error) when serialization fails.
| return new Error(JSON.stringify(error)); | |
| try { | |
| return new Error(JSON.stringify(error)); | |
| } catch { | |
| return new Error(String(error)); | |
| } |
| if (error instanceof Error) { | ||
| return error; | ||
| } | ||
| if (typeof error === "object") { | ||
| return new Error(JSON.stringify(error)); | ||
| } | ||
|
|
||
| return new Error(String(error)); |
There was a problem hiding this comment.
This new file is indented with spaces, but the repo Prettier config specifies useTabs: true/tabWidth: 4. Reformat with tabs (or run Prettier) to avoid inconsistent formatting and future diffs.
| if (error instanceof Error) { | |
| return error; | |
| } | |
| if (typeof error === "object") { | |
| return new Error(JSON.stringify(error)); | |
| } | |
| return new Error(String(error)); | |
| if (error instanceof Error) { | |
| return error; | |
| } | |
| if (typeof error === "object") { | |
| return new Error(JSON.stringify(error)); | |
| } | |
| return new Error(String(error)); |
| "types": "./dist/index.d.ts", | ||
| "scripts": { | ||
| "lint": "eslint src --ext .ts", | ||
| "lint": "eslint src", |
There was a problem hiding this comment.
The lint script was changed to eslint src, which (with the default ESLint CLI behavior) typically only lints JS extensions and may skip .ts files. Use eslint src --ext .ts (as before) or a glob like eslint "src/**/*.{ts,tsx}" to ensure TypeScript is actually linted.
| "lint": "eslint src", | |
| "lint": "eslint src --ext .ts", |
| { | ||
| files: ["**/*.{js,mjs,cjs,ts,mts,cts}"], | ||
| plugins: { js }, | ||
| extends: ["js/recommended"], | ||
| languageOptions: { | ||
| globals: globals.node, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
The first config block applies to **/*.{js,...,ts,...} and enables JS recommended rules without setting a TS parser. Depending on flat-config merging order, this can cause .ts files to be parsed by Espree (syntax errors) or make rule application unpredictable. Consider limiting this block to JS extensions only, and include js.configs.recommended directly (similar to the previous config) instead of plugins: { js } + extends: ["js/recommended"].
No description provided.