refactor: convert subcircuit.js to TypeScript#813
refactor: convert subcircuit.js to TypeScript#813pantha704 wants to merge 1 commit intoCircuitVerse:mainfrom
Conversation
Part of CircuitVerse#661 (1 file per PR rule) - Added class property declarations with types - Added method return type annotations (: void, : boolean, : string, : any) - Replaced var with const/let throughout - Added Record<string, any[]> for temp_map objects - Added JSDoc documentation for all methods - Applied same changes to v1/src No logic changes - strictly TypeScript migration.
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughThis change introduces comprehensive TypeScript type annotations to the SubCircuit class in subcircuit.ts. The modifications add explicit types to function signatures, constructor parameters, class fields, method return types, and local variables. Variable declarations are updated from 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
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/subcircuit.ts (1)
200-212: Potential NPE: Inconsistent optional chaining usage.Line 202 uses optional chaining (
?.) suggestingthis.localScope.Input[i]could be undefined, but line 205 accessesthis.localScope.Input[i].output1without the guard. If the optional chaining on line 202 prevents an error, line 205 will still throw.Suggested fix
makeConnections(): void { for (let i = 0; i < this.inputNodes.length; i++) { this.localScope.Input[i]?.output1.connectWireLess( this.inputNodes[i] ) - this.localScope.Input[i].output1.subcircuitOverride = true + if (this.localScope.Input[i]) { + this.localScope.Input[i].output1.subcircuitOverride = true + } }src/simulator/src/subcircuit.ts (1)
200-212: Potential NPE: Inconsistent optional chaining usage.Same issue as in v1: Line 202 uses
?.but line 205 does not. Ifthis.localScope.Input[i]can be undefined, line 205 will throw.Suggested fix
makeConnections(): void { for (let i = 0; i < this.inputNodes.length; i++) { this.localScope.Input[i]?.output1.connectWireLess( this.inputNodes[i] ) - this.localScope.Input[i].output1.subcircuitOverride = true + if (this.localScope.Input[i]) { + this.localScope.Input[i].output1.subcircuitOverride = true + } }
🧹 Nitpick comments (6)
v1/src/simulator/src/subcircuit.ts (3)
479-498: Unused variables:rX,lX,uY,dYare declared but never used.These variables are assigned from
layoutPropertiesbut are not referenced anywhere in the method body.Remove unused variables
getElementHover(): any { - const rX = this.layoutProperties.rightDimensionX - const lX = this.layoutProperties.leftDimensionX - const uY = this.layoutProperties.upDimensionY - const dY = this.layoutProperties.downDimensionY - for (const el of circuitElementList) {
583-588: Consider a more precise tuple type fordetermine_label.The first element is always
'left','right', or'center'. A literal union type would provide better type safety when used withctx.textAlign.Suggested type refinement
- determine_label(x: number, y: number): [string, number, number] { + determine_label(x: number, y: number): ['left' | 'right' | 'center', number, number] {
272-276: Deprecation note acknowledged.The
reBuildmethod is marked for deprecation. Consider adding a@deprecatedJSDoc tag to trigger IDE warnings when this method is called.Add `@deprecated` tag
/** - * Needs to be deprecated, removed + * `@deprecated` Needs to be deprecated, removed */ reBuild(): void { }src/simulator/src/subcircuit.ts (3)
479-498: Unused variables:rX,lX,uY,dYare never used.These variables are declared but not referenced in the method body.
Remove unused variables
getElementHover(): any { - const rX = this.layoutProperties.rightDimensionX - const lX = this.layoutProperties.leftDimensionX - const uY = this.layoutProperties.upDimensionY - const dY = this.layoutProperties.downDimensionY - for (const el of circuitElementList) {
583-588: Consider using literal union type for better type safety.Suggested type refinement
- determine_label(x: number, y: number): [string, number, number] { + determine_label(x: number, y: number): ['left' | 'right' | 'center', number, number] {
272-276: Consider adding@deprecatedJSDoc tag.Add deprecation annotation
/** - * Needs to be deprecated, removed + * `@deprecated` Needs to be deprecated, removed */ reBuild(): void { }
1 participant
Part of #661 (1 file per PR rule)
Changes
Converted
subcircuit.jsto TypeScript in bothsrc/andv1/src/.TypeScript Improvements
: void,: boolean,: string,: any,: [string, number, number])varwithconst/letthroughoutRecord<string, any[]>type for temp_map objectsNo Logic Changes
SubCircuit extends CircuitElementloadSubCircuit,createSubCircuitPromptbun run build)Code Understanding and AI Usage
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.