-
Notifications
You must be signed in to change notification settings - Fork 230
refactor: Convert events.js to TypeScript #858
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
base: main
Are you sure you want to change the base?
Conversation
- Convert events.js to events.ts with full TypeScript support - Add proper type annotations for function parameters and return types - Define CopyData interface for better type safety - Replace var with const/let for better scoping - Update variable comparisons to use strict equality (=== instead of ==) - Maintain all existing functionality and logic Relates to: CircuitVerse#661 (JavaScript to TypeScript conversion in src folder)
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughThe changes in this PR apply TypeScript conversion to the events.ts file. A new CopyData interface is introduced to type clipboard data. Function signatures are updated with parameter and return type annotations for paste, cut, copy, and selectAll. Variable declarations are refactored from var to const/let throughout. Loose equality comparisons (==, !=) are replaced with strict equality (===, !==). Explicit typing is added to function parameters, local variables, and loop variables. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
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: 1
🤖 Fix all issues with AI agents
In `@src/simulator/src/events.ts`:
- Around line 28-31: The paste function currently checks for the literal string
'undefined' which is incorrect for callers that pass localStorage.getItem(...)
(string|null) or clipboard API strings; update the paste signature to accept
string | null (change export function paste(copyData: string): void to export
function paste(copyData: string | null): void), remove the "undefined" string
comparison, and replace it with a proper null/falsy check such as if (copyData
== null || copyData === '') return; so JSON.parse is only called on real strings
and preserves compatibility with copy() returning an empty string; alternatively
add a brief comment if you intentionally need to preserve a legacy 'undefined'
literal.
🧹 Nitpick comments (3)
src/simulator/src/events.ts (3)
18-22: CopyData interface is acceptable for incremental migration, but consider future improvements.The interface uses
any[]forscopesand an index signature[key: string]: any, which provides basic type safety while allowing flexibility. This is reasonable for an incremental JS-to-TS conversion.For future iterations, consider defining a more specific type based on the structure returned by
backUp()(frombackupCircuit.js), which includes known properties likelayout,verilogMetadata,allNodes,testbenchData,id,name, etc.
149-150: Useconstinstead ofletfordsince it's not reassigned.The variable
dis only assigned once and never reassigned, so it should be declared withconstfor consistency with the other immutable variables in this function.Suggested fix
- let d = backUp(globalScope) + const d = backUp(globalScope)
333-338: Consider adding explicit type annotation for thescopeparameter.The
scopeparameter usesglobalScopeas a default value but lacks an explicit type annotation. TypeScript will infer the type fromglobalScope, but an explicit type would improve readability and maintainability.Based on the learnings,
globalScopeis a global variable that should be typed by extending the Window interface. If aScopetype is available (imported from./circuit), consider using it here.Suggested improvement
-export function selectAll(scope = globalScope): void { +export function selectAll(scope: Scope = globalScope): void {Note: This requires
Scopeto be exported from./circuitand theglobalScopetype to match.
|
@tachyons kindly review the changes and merge the pull request if possible |

Description
Convert src/simulator/src/events.js to TypeScript with proper type annotations.
Changes
varwithconst/letfor better scoping and block scope===instead of==)Type Improvements
Related to
Closes/relates to #661 (JavaScript to TypeScript conversion in src folder)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.