refactor: convert circuitElement.js to TypeScript#814
refactor: convert circuitElement.js to TypeScript#814pantha704 wants to merge 2 commits intoCircuitVerse:mainfrom
Conversation
Part of CircuitVerse#661 (1 file per PR rule) - Added 40+ class property declarations with explicit types - Added method return type annotations for all 43 methods - Replaced var with const/let throughout - Added proper type casts (this as any) for subclass methods - Added JSDoc documentation - Applied same changes to v1/src No logic changes - strictly TypeScript migration.
WalkthroughThis change refactors CircuitElement to TypeScript by adding explicit public properties (e.g., x, y, hover, label, bitWidth, direction, layout and verilog metadata) and formalizing the constructor signature (constructor(x: number, y: number, scope: any, dir: string, bitWidth: number)). Many previously untyped methods were converted to typed signatures with explicit parameter and return types, newOrientation was added as a public method, and calls to optional overrides were guarded using typed forms. Documentation comments were updated to match the new typings. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify project 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/simulator/src/circuitElement.ts (1)
847-855: Validate numeric inputs to avoidNaNstate.
newBitWidthchecksthis.bitWidthinstead of the incomingbitWidth, andchangePropagationDelaycan setpropagationDelay = NaNif parsing fails. A small guard prevents invalid state.🐛 Suggested validation
newBitWidth(bitWidth: number): void { if (this.fixedBitWidth) return if (this.bitWidth === undefined) return - if (this.bitWidth < 1) return + if (!Number.isFinite(bitWidth) || bitWidth < 1) return this.bitWidth = bitWidth @@ changePropagationDelay(delay: any): void { if (this.propagationDelayFixed) return if (delay === undefined) return if (delay === '') return - const tmpDelay = parseInt(delay, 10) - if (tmpDelay < 0) return + const tmpDelay = Number.parseInt(delay, 10) + if (!Number.isFinite(tmpDelay) || tmpDelay < 0) return this.propagationDelay = tmpDelay }Also applies to: 862-869
v1/src/simulator/src/circuitElement.ts (1)
847-855: Validate numeric inputs to avoidNaNstate.
newBitWidthchecksthis.bitWidthinstead of the incomingbitWidth, andchangePropagationDelaycan setpropagationDelay = NaNif parsing fails. A small guard prevents invalid state.🐛 Suggested validation
newBitWidth(bitWidth: number): void { if (this.fixedBitWidth) return if (this.bitWidth === undefined) return - if (this.bitWidth < 1) return + if (!Number.isFinite(bitWidth) || bitWidth < 1) return this.bitWidth = bitWidth @@ changePropagationDelay(delay: any): void { if (this.propagationDelayFixed) return if (delay === undefined) return if (delay === '') return - const tmpDelay = parseInt(delay, 10) - if (tmpDelay < 0) return + const tmpDelay = Number.parseInt(delay, 10) + if (!Number.isFinite(tmpDelay) || tmpDelay < 0) return this.propagationDelay = tmpDelay }Also applies to: 862-869
🤖 Fix all issues with AI agents
In `@v1/src/simulator/src/circuitElement.ts`:
- Around line 389-390: The code calls subcircuitDraw() unguarded which throws on
subclasses that don't implement it; update the call to only invoke
subcircuitDraw when it exists (e.g., check typeof this.subcircuitDraw ===
'function' or use optional chaining) similar to the existing guard for
releaseClick() that checks this.objectType === 'SubCircuit'; locate the
unguarded call to subcircuitDraw() in CircuitElement and wrap it with the
existence check so non-implementing subclasses won't raise a TypeError.
🧹 Nitpick comments (2)
v1/src/simulator/src/circuitElement.ts (1)
70-101: AlignbitWidthtype with the runtime fallback.Line 70–101: the implementation clearly tolerates
undefinedand prompts for a value, but the signature requiresnumber. While all current call sites pass this argument, makingbitWidthoptional would improve the API design and better reflect the implementation's behavior.🛠️ Proposed adjustment
- constructor(x: number, y: number, scope: any, dir: string, bitWidth: number) { + constructor(x: number, y: number, scope: any, dir: string, bitWidth?: number) { @@ - this.bitWidth = bitWidth || parseInt(prompt('Enter bitWidth') || '1', 10) || 1 + const resolvedBitWidth = + bitWidth || parseInt(prompt('Enter bitWidth') || '1', 10) || 1 + this.bitWidth = resolvedBitWidthsrc/simulator/src/circuitElement.ts (1)
70-101: Consider makingbitWidthoptional for consistency with the defensive fallback logic.Line 100 implements a fallback pattern (
bitWidth || parseInt(prompt(...)) || 1) suggestingundefinedis acceptable, but the signature requiresbitWidth: number. While all current call sites pass a value explicitly, aligning the signature with the implementation pattern would improve consistency and clarify intent if future code needs to omit the parameter.Proposed adjustment
- constructor(x: number, y: number, scope: any, dir: string, bitWidth: number) { + constructor(x: number, y: number, scope: any, dir: string, bitWidth?: number) { @@ - this.bitWidth = bitWidth || parseInt(prompt('Enter bitWidth') || '1', 10) || 1 + const resolvedBitWidth = + bitWidth || parseInt(prompt('Enter bitWidth') || '1', 10) || 1 + this.bitWidth = resolvedBitWidth
Addresses CodeRabbit feedback - guard subcircuitDraw() call to prevent runtime errors on subclasses that don't implement it. Similar to the existing guard on releaseClick() (line 389).
There was a problem hiding this comment.
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 (2)
v1/src/simulator/src/circuitElement.ts (1)
880-918: Add TypeScript declarations for global constants used without imports.
NODE_OUTPUTandNODE_INPUT(lines 885, 893, 951, 997) are currently defined onwindowinnode.jsand used throughoutcircuitElement.tswithout explicit imports. Similarly,widthandheightare defined onwindowinglobalVariables.tsbut lack type declarations. These globals should be declared in a TypeScript declaration file (.d.ts) to enable proper type checking and IDE support.Create or update a declaration file (e.g.,
src/simulator/src/globals.d.ts) to declare:declare global { var NODE_OUTPUT: number; var NODE_INPUT: number; var NODE_INTERMEDIATE: number; var width: number | undefined; var height: number | undefined; }src/simulator/src/circuitElement.ts (1)
880-918: Add TypeScript declarations for window globals in .d.ts file.
NODE_OUTPUT,NODE_INPUT,width, andheightare assigned to thewindowobject but lack proper TypeScript type declarations. Withstrict: truein tsconfig.json, extend theWindowinterface in a.d.tsfile (e.g.,src/env.d.ts) to declare these properties with their correct types:interface Window { NODE_INPUT: number NODE_OUTPUT: number NODE_INTERMEDIATE: number width: number | undefined height: number | undefined }This ensures type safety in strict mode for both
src/simulator/src/andv1/src/simulator/src/code paths.
🤖 Fix all issues with AI agents
In `@src/simulator/src/circuitElement.ts`:
- Around line 217-221: The JSDoc for the method checkHover() incorrectly states
"@return boolean" while the method signature returns void; update the JSDoc to
match by either removing the "@return" tag or replacing it with "@returns
{void}" (or an explanatory comment) so the comment for checkHover() accurately
reflects its void return type.
In `@v1/src/simulator/src/circuitElement.ts`:
- Around line 217-221: The JSDoc for the checkHover method is out of sync: it
declares "@return boolean" but the method signature is "checkHover(): void";
update the documentation to match the code by either removing the "@return" tag
or changing it to "@return void" (or "@returns void") to reflect that checkHover
does not return a value, and ensure the JSDoc sits immediately above the
checkHover() method declaration.
🧹 Nitpick comments (4)
v1/src/simulator/src/circuitElement.ts (3)
27-68: Well-structured TypeScript property declarations.The explicit type declarations for all class properties provide good type safety. However, two properties assigned on the prototype (lines 1024, 1036) are missing from these declarations:
🔧 Add missing property declarations
layoutProperties: any overrideDirectionRotation: boolean + tooltip: any + subcircuitMutableProperties: any constructor(x: number, y: number, scope: any, dir: string, bitWidth: number) {
100-100: Consider extracting the prompt fallback logic.The
prompt()call in the constructor is a blocking UI side-effect. While this is preserved behavior from the original JS, consider refactoring in a future PR to acceptbitWidthas a required parameter or use a callback/event pattern for missing values.
1022-1047: Consider migrating prototype assignments to class-level declarations.Prototype assignments after the class definition work but bypass TypeScript's type system for these properties. For future maintainability, consider moving these to static properties or initializing them in the constructor with proper class property declarations.
This is acceptable for the current migration scope but worth tracking for a future cleanup.
src/simulator/src/circuitElement.ts (1)
27-68: Same property declarations as v1 version - add missing properties.As with the v1 version,
tooltipandsubcircuitMutablePropertiesare assigned on the prototype (lines 1024, 1036) but not declared as class properties. Add these declarations for consistency.
| /** | ||
| * check hover over the element | ||
| * @return {boolean} | ||
| * @return boolean | ||
| */ | ||
| checkHover() { | ||
| checkHover(): void { |
There was a problem hiding this comment.
Same JSDoc inconsistency - @return boolean on void method.
Update or remove the @return tag to match the void return type.
🤖 Prompt for AI Agents
In `@src/simulator/src/circuitElement.ts` around lines 217 - 221, The JSDoc for
the method checkHover() incorrectly states "@return boolean" while the method
signature returns void; update the JSDoc to match by either removing the
"@return" tag or replacing it with "@returns {void}" (or an explanatory comment)
so the comment for checkHover() accurately reflects its void return type.
| /** | ||
| * check hover over the element | ||
| * @return {boolean} | ||
| * @return boolean | ||
| */ | ||
| checkHover() { | ||
| checkHover(): void { |
There was a problem hiding this comment.
JSDoc @return is inconsistent with method signature.
The JSDoc says @return boolean but the method is typed as checkHover(): void. Either remove the @return tag or update it to match the actual return type.
🔧 Fix JSDoc
/**
* check hover over the element
- * `@return` boolean
*/
checkHover(): void {📝 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.
| /** | |
| * check hover over the element | |
| * @return {boolean} | |
| * @return boolean | |
| */ | |
| checkHover() { | |
| checkHover(): void { | |
| /** | |
| * check hover over the element | |
| */ | |
| checkHover(): void { |
🤖 Prompt for AI Agents
In `@v1/src/simulator/src/circuitElement.ts` around lines 217 - 221, The JSDoc for
the checkHover method is out of sync: it declares "@return boolean" but the
method signature is "checkHover(): void"; update the documentation to match the
code by either removing the "@return" tag or changing it to "@return void" (or
"@returns void") to reflect that checkHover does not return a value, and ensure
the JSDoc sits immediately above the checkHover() method declaration.
1 participant
Part of #661 (1 file per PR rule)
Changes
Converted
circuitElement.jsto TypeScript in bothsrc/andv1/src/.TypeScript Improvements
: void,: boolean,: string,: number,: any)varwithconst/letthroughout(this as any)for subclass-specific methods (click, customDraw, subcircuitDraw, releaseClick)No Logic Changes
CircuitElementbase classbun run build)Code Understanding and AI Usage
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.