fix(typescript): upgrade and setup eslint and setup global variables #416
Conversation
WalkthroughThis pull request introduces a comprehensive update to the project's ESLint configuration, transitioning from a legacy Changes
Assessment against linked issues
Possibly related PRs
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/types/globalVariables.d.ts (1)
18-33: Reduce type duplication by using a common interface.The Window interface properties duplicate the global variable types. Consider creating a shared interface:
+ interface GlobalVars { + $: typeof import('jquery') + jQuery: typeof import('jquery') + // ... other properties + } + declare global { - var $: typeof import('jquery') - // ... other global vars + var: GlobalVars interface Window extends GlobalVars {} }🧰 Tools
🪛 eslint
[error] 22-22: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 23-23: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 24-24: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 26-26: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 27-27: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 30-30: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 31-31: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
src/globalVariables.ts (1)
25-37: Reduce repetition by using a helper function for global variable assignments.Consider creating a helper function to handle the assignment pattern:
+function assignGlobal<K extends keyof Window>(key: K, value: Window[K]) { + window[key] = value; + (globalThis as any)[key] = window[key]; +} + -globalThis.isUserLoggedIn = window.isUserLoggedIn -globalThis.logixProjectId = window.logixProjectId // ... more assignments +assignGlobal('isUserLoggedIn', window.isUserLoggedIn); +assignGlobal('logixProjectId', window.logixProjectId); // ... more assignmentssrc/simulator/src/circuit.ts (1)
131-133: Improve type handling in formattedId assignment.The current implementation could be improved:
- let formattedId - if (typeof scopeId === 'number') formattedId = scopeId - else formattedId = parseInt(scopeId) + const formattedId = typeof scopeId === 'number' ? scopeId : parseInt(scopeId, 10)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
.eslintrc.js(0 hunks)eslint.config.mjs(1 hunks)package.json(2 hunks)src/globalVariables.ts(2 hunks)src/simulator/src/circuit.ts(7 hunks)src/types/globalVariables.d.ts(1 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- .eslintrc.js
🧰 Additional context used
🪛 Biome (1.9.4)
src/simulator/src/circuit.ts
[error] 320-320: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
🪛 eslint
src/simulator/src/circuit.ts
[error] 295-295: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 297-297: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 312-312: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 320-320: The {} ("empty object") type allows any non-nullish value, including literals like 0 and "".
- If that's what you want, disable this lint rule with an inline comment or configure the 'allowObjectTypes' rule option.
- If you want a type meaning "any object", you probably want
objectinstead. - If you want a type meaning "any value", you probably want
unknowninstead.
(@typescript-eslint/no-empty-object-type)
[error] 321-321: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 322-322: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 323-323: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 324-324: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 325-325: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 326-326: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 327-327: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 328-328: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 329-329: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
src/types/globalVariables.d.ts
[error] 6-6: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 7-7: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 8-8: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 10-10: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 11-11: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 14-14: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 15-15: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 22-22: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 23-23: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 24-24: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 26-26: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 27-27: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 30-30: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 31-31: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🔇 Additional comments (9)
src/types/globalVariables.d.ts (1)
36-36: LGTM! The empty export is correctly used.The empty export statement is necessary to make this file a module, enabling global augmentation in TypeScript.
src/globalVariables.ts (1)
1-8: LGTM! Improved type safety with proper Window typing.The transition from using globalThis to window with proper typing enhances type safety.
eslint.config.mjs (2)
1-32: LGTM! Well-structured ESLint configuration with TypeScript support.The configuration properly sets up TypeScript checking with Vue and follows the new flat config format.
33-57: LGTM! Comprehensive globals configuration.The configuration properly declares all required global variables and includes necessary environments.
src/simulator/src/circuit.ts (1)
205-210: LGTM! Well-structured function signature with proper types.The newCircuit function signature is now more readable and properly typed.
tsconfig.json (1)
24-25: LGTM! Verify the src/types directory structure.The addition of
typeRootsconfiguration properly includes both standard and custom type definitions.Let's verify the directory structure:
✅ Verification successful
Directory structure verified successfully
The
src/typesdirectory exists and contains the expected TypeScript declaration file (globalVariables.d.ts), confirming proper setup of custom type definitions.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the src/types directory exists and contains type definitions fd -t d "types" src/ fd -e d.ts . src/types/Length of output: 88
package.json (3)
46-51: LGTM! Dependencies align with TypeScript and ESLint setup.The addition of
@vue/eslint-config-typescriptand@eslint/compatsupports the TypeScript linting setup for Vue.
10-10: Verify the new lint command configuration.The lint command has been simplified, removing explicit extensions and ignore paths. These configurations should be present in
eslint.config.mjs.Let's verify the ESLint configuration:
✅ Verification successful
The lint command configuration is properly set up
The simplified
lintcommand is valid as all necessary configurations are properly defined ineslint.config.mjs, including:
- File extensions (via Vue/TS configuration)
- Ignore patterns (via .gitignore and explicit ignores)
- Project-specific settings and integrations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if eslint.config.mjs exists and contains necessary configuration if [ -f "eslint.config.mjs" ]; then echo "Checking eslint.config.mjs for extensions and ignore patterns..." rg -i "extensions|ignore" eslint.config.mjs else echo "eslint.config.mjs not found!" fiLength of output: 396
Script:
#!/bin/bash # Check complete eslint.config.mjs content and look for other config files echo "=== Complete eslint.config.mjs content ===" cat eslint.config.mjs echo -e "\n=== Checking for other ESLint config files ===" fd -g '.eslint*'Length of output: 2177
Script:
#!/bin/bash # Check for hidden ESLint config files echo "=== Checking for hidden ESLint config files ===" fd -H -g '.eslint*'Length of output: 126
53-53: Verify ESLint v9 compatibility with existing plugins.The upgrade from ESLint v8 to v9 is a major version change that might have breaking changes.
Let's check compatibility:
✅ Verification successful
ESLint v9 upgrade is compatible with all dependencies
All ESLint-related dependencies are compatible with ESLint v9:
- @eslint/compat and @vue/eslint-config-typescript are specifically designed for v9
- eslint-config-prettier supports v9 (requires >=7.0.0)
- eslint-plugin-vue's recent versions include v9 support
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if ESLint plugins are compatible with v9 jq -r '.devDependencies | to_entries[] | select(.key | contains("eslint-plugin")) | .key + "@" + .value' package.json | while read plugin; do echo "Checking $plugin compatibility with ESLint 9..." npm view "$plugin" peerDependencies.eslint doneLength of output: 2867
Script:
#!/bin/bash # Check all ESLint-related dependencies jq -r '.devDependencies | to_entries[] | select(.key | contains("eslint") or contains("@typescript-eslint")) | .key + "@" + .value' package.json | while read dep; do echo "Checking $dep compatibility with ESLint 9..." npm view "$dep" peerDependencies.eslint doneLength of output: 4002
| var logixProjectId: any | ||
| var restrictedElements: any[] | ||
| var globalScope: any | ||
| var lightMode: boolean | ||
| var projectId: any | ||
| var id: any | ||
| var loading: boolean | ||
| var embed: boolean | ||
| var width: any | ||
| var height: any | ||
| var DPR: number |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety by replacing any types with more specific types.
Several variables are typed as any, which reduces TypeScript's effectiveness in catching type-related bugs. Consider using more specific types:
- var logixProjectId: any
- var restrictedElements: any[]
- var globalScope: any
- var projectId: any
- var id: any
- var width: any
- var height: any
+ var logixProjectId: string | number | null
+ var restrictedElements: string[]
+ var globalScope: Scope
+ var projectId: string | number | null
+ var id: string | number | null
+ var width: number
+ var height: numberCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 eslint
[error] 6-6: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 7-7: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 8-8: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 10-10: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 11-11: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 14-14: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 15-15: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/simulator/src/circuit.ts (1)
Line range hint
47-49: Add input validation for the clock time parameter.Consider adding validation to ensure the clock time value is within acceptable bounds:
function changeClockTime(t: number) { + if (t < 0) throw new Error('Clock time cannot be negative'); + if (!Number.isFinite(t)) throw new Error('Clock time must be a finite number'); simulationArea.changeClockTime(t) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/simulator/src/circuit.ts(1 hunks)
🔇 Additional comments (3)
src/simulator/src/circuit.ts (3)
Line range hint
35-45: LGTM! ThechangeClockTimefunction is properly added to thecircuitPropertyobject.The addition maintains consistency with other circuit-related functions in the object.
515-515: LGTM! Proper newline added at the end of the file.This follows the standard ESLint rule requiring a newline at the end of files.
Line range hint
295-329: Replaceanytypes with specific types in the Scope class.The class properties should use more specific types to improve type safety:
- restrictedCircuitElementsUsed: any[] - CircuitElement: any[] - stack: any[] - tunnelList?: {} - pending?: any[] - nodes?: any[] - allNodes?: any[] - wires?: any[] - Input?: any[] - Output?: any[] - Splitter?: any[] - SubCircuit?: any[] - Clock?: any[] + restrictedCircuitElementsUsed: string[] + CircuitElement: CircuitElement[] + stack: unknown[] + tunnelList?: Record<string, unknown> + pending?: CircuitElement[] + nodes?: Node[] + allNodes?: Node[] + wires?: Wire[] + Input?: InputElement[] + Output?: OutputElement[] + Splitter?: SplitterElement[] + SubCircuit?: SubCircuitElement[] + Clock?: ClockElement[]
Fixes #412
Describe the changes you have made in this PR -
9.18.0) to use @vue/eslint-config-typescript for enabling the essential rules for Vue 3 and the recommended rules for TypeScript.eslint.config.mjsinstead ofeslintrc.jssrc/types/globalVariables.d.tsto declare custom global variables and ensure compatibility with TypeScript.window.What this PR does:
What this PR DOES NOT do:
Screenshots of the changes (If any) -
Notice the number of errors and problems decrease.
Before: -
Output of
npx tsc --noEmit-Output of
npx eslint --no-fix srcAfter: -
Output of
npx tsc --noEmit-Output of
npx eslint --no-fix srcNote: Please check Allow edits from maintainers. if you would like us to assist in the PR.
Summary by CodeRabbit
Chores
Refactor
Documentation