Skip to content

refactor: convert events.js to TypeScript#812

Open
pantha704 wants to merge 1 commit intoCircuitVerse:mainfrom
pantha704:ts-events#661
Open

refactor: convert events.js to TypeScript#812
pantha704 wants to merge 1 commit intoCircuitVerse:mainfrom
pantha704:ts-events#661

Conversation

@pantha704
Copy link
Contributor

@pantha704 pantha704 commented Jan 17, 2026

Part of #661 (1 file per PR rule)

Changes

Converted events.js to TypeScript in both src/ and v1/src/.

TypeScript Improvements

  • Added return type annotations for all functions (: void, : string | undefined)
  • Added parameter type annotations (copyData: string, copyList: any[], cutflag: boolean)
  • Replaced var with const/let throughout
  • Added Record<string, any> and Record<string, boolean> types for internal objects
  • Added function parameter types for nested functions (saveScope(id: string): void)
  • Updated JSDoc documentation

No Logic Changes

  • ✅ Same exported functions: paste, cut, copy, selectAll
  • ✅ Same behavior, only added type safety
  • ✅ Build passes (bun run build)

Code Understanding and AI Usage

  • I have reviewed every single line of the changes
  • I can explain the purpose of each type annotation
  • No logic was changed, only types added

Summary by CodeRabbit

  • Refactor

    • Enhanced type safety and improved internal code quality for simulator functions.
  • Documentation

    • Updated function documentation with clearer parameter and return type specifications.

✏️ Tip: You can customize this high-level summary in your review settings.

Part of CircuitVerse#661 (1 file per PR rule)

- Added return type annotations for all functions
- Added parameter type annotations (copyData: string, copyList: any[])
- Replaced var with const/let throughout
- Added Record types for internal objects
- Added JSDoc documentation
- Applied same changes to v1/src

No logic changes - strictly TypeScript migration.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 17, 2026

Walkthrough

The pull request adds explicit TypeScript typing to four exported functions in the events.ts file: paste, cut, copy, and selectAll. Each function signature now includes typed parameters and return type annotations. The changes also involve replacing variable declarations from var to const/let throughout the affected functions for improved scoping practices. JSDoc comments are refined to reflect the new type annotations. The overall modification affects +45 and -39 lines with medium code review effort.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'refactor: convert events.js to TypeScript' accurately and clearly describes the main change - converting JavaScript files to TypeScript across the codebase with added type annotations.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@netlify
Copy link

netlify bot commented Jan 17, 2026

Deploy Preview for circuitverse ready!

Name Link
🔨 Latest commit 62bfaa3
🔍 Latest deploy log https://app.netlify.com/projects/circuitverse/deploys/696b447c4d67a30008b7310f
😎 Deploy Preview https://deploy-preview-812--circuitverse.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 44 (no change from production)
Accessibility: 73 (no change from production)
Best Practices: 92 (no change from production)
SEO: 82 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (3)
v1/src/simulator/src/events.ts (2)

65-90: Potential division by zero when pasting empty or wire-only content.

If all objects in tempScope are Wires, count remains 0, causing approxX and approxY to become NaN. This would corrupt object positions.

Proposed fix
-    approxX /= count
-    approxY /= count
+    if (count > 0) {
+        approxX /= count
+        approxY /= count
+    }

193-199: Fix array iteration over getDependencies() result.

getDependencies() returns an array of dependency IDs, but Object.keys(dependencyList) iterates over array indices ('0', '1', etc.) rather than the IDs themselves. This causes scopeList[dependency] lookups to fail. Iterate directly over the array instead: dependencyList.forEach((dependency) => { ... })

src/simulator/src/events.ts (1)

65-90: Same division by zero risk as v1 version.

When all pasted objects are Wires, count is 0, leading to NaN coordinates.

Proposed fix
-    approxX /= count
-    approxY /= count
+    if (count > 0) {
+        approxX /= count
+        approxY /= count
+    }
🧹 Nitpick comments (4)
v1/src/simulator/src/events.ts (3)

139-140: Consider using explicit undefined return for clarity.

The implicit return when copyList.length === 0 works but could be more explicit given the string | undefined return type annotation.

Suggested improvement
-    if (copyList.length === 0) return
+    if (copyList.length === 0) return undefined

223-224: Same implicit return pattern as cut().

Consistency with explicit return undefined would improve readability.


331-341: Consider adding explicit type for scope parameter.

While the default value provides type inference, an explicit type annotation would improve clarity and catch type mismatches at call sites.

Suggested improvement
-export function selectAll(scope = globalScope): void {
+export function selectAll(scope: typeof globalScope = globalScope): void {

Based on learnings, globalScope is a global variable that should be typed by extending the Window interface. Using typeof globalScope here would leverage that typing.

src/simulator/src/events.ts (1)

331-341: Consider explicit type annotation for scope parameter.

Same suggestion as v1 version - using typeof globalScope would provide better type safety at call sites while maintaining the current default value behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant