Skip to content

Conversation

@kaihere14
Copy link

@kaihere14 kaihere14 commented Nov 9, 2025

Fixes #661

Describe the changes you have made in this PR -

Added a new type declaration file (src/types/global.d.ts) defining common window properties used across the simulator (e.g., globalScope, lightMode, projectId, embed, etc.).
This improves TypeScript type safety and removes the need for repetitive @ts-ignore comments.

Screenshots of the changes (If any) -

Screenshot 2025-11-09 at 6 43 37 AM

Note: Please check Allow edits from maintainers.

Summary by CodeRabbit

  • Chores
    • Enhanced TypeScript type definitions for improved developer experience and type safety.

@netlify
Copy link

netlify bot commented Nov 9, 2025

Deploy Preview for circuitverse ready!

Name Link
🔨 Latest commit 26010e3
🔍 Latest deploy log https://app.netlify.com/projects/circuitverse/deploys/690feb6f2fe2450008d9c356
😎 Deploy Preview https://deploy-preview-675--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.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 9, 2025

Walkthrough

A new TypeScript global type declarations file is added that extends the Window interface with optional properties for DOM utilities, authentication state, project identifiers, UI settings, and display dimensions. No runtime code is introduced; only type definitions are added.

Changes

Cohort / File(s) Summary
Global type augmentations
src/types/global.d.ts
New file declaring Window interface extensions with optional properties for jQuery/DOM references ($, jQuery), authentication and project state (isUserLoggedIn, logixProjectId, projectId, id), UI configuration (lightMode, embed), rendering dimensions (width, height, DPR), and utility scope (restrictedElements, globalScope, loading).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

  • Verify that all Window property declarations are necessary and correctly aligned with usage patterns across the codebase
  • Confirm that optional property markers (?) are appropriate for each declaration

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding TypeScript type declarations for Window interface properties used in the simulator.
Linked Issues check ✅ Passed The PR adds TypeScript type declarations to improve type safety without changing logic, directly addressing the JavaScript-to-TypeScript conversion goals in issue #661.
Out of Scope Changes check ✅ Passed The PR is limited to a single new type declaration file with no logic changes, staying within the scope of improving TypeScript type safety as specified in issue #661.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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.

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

🧹 Nitpick comments (3)
src/types/global.d.ts (3)

7-8: Consider installing jQuery type definitions.

While any works for eliminating @ts-ignore comments, consider installing @types/jquery if jQuery is used extensively across the codebase. This would provide better type safety and autocompletion.

Apply this approach if jQuery is a major dependency:

npm install --save-dev @types/jquery

Then update the declarations:

-    $?: any
-    jQuery?: any
+    $?: JQueryStatic
+    jQuery?: JQueryStatic

10-10: Remove redundant | undefined from optional properties.

The ? operator already makes properties optional (meaning they can be undefined). Explicitly adding | undefined to the type union is redundant.

Apply this diff to remove the redundancy:

-    logixProjectId?: number | string | undefined
+    logixProjectId?: number | string
-    projectId?: number | string | undefined
-    id?: number | string | undefined
+    projectId?: number | string
+    id?: number | string
-    width?: number | undefined
-    height?: number | undefined
+    width?: number
+    height?: number

Also applies to: 14-15, 18-19


11-11: Consider more specific typing for restrictedElements.

The property uses any[] which provides minimal type safety. If the structure of restricted elements is known, consider defining a more specific type or interface.

For example:

interface RestrictedElement {
  // Define properties based on actual usage
  id: string
  type: string
  // ... other properties
}

// Then in the Window interface:
restrictedElements?: RestrictedElement[]
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 11b4883 and 26010e3.

📒 Files selected for processing (1)
  • src/types/global.d.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ThatDeparted2061
Repo: CircuitVerse/cv-frontend-vue PR: 442
File: src/simulator/src/wire.ts:0-0
Timestamp: 2025-01-27T17:29:33.929Z
Learning: In the CircuitVerse frontend Vue project, globalScope should be declared on the window object using TypeScript declaration files (.d.ts) rather than importing it as a module.
Learnt from: ThatDeparted2061
Repo: CircuitVerse/cv-frontend-vue PR: 442
File: src/simulator/src/wire.ts:0-0
Timestamp: 2025-01-27T17:29:33.929Z
Learning: In the CircuitVerse frontend Vue project, globalScope is a global variable that should be typed by extending the Window interface in a TypeScript declaration file (global.d.ts), as it's initialized on the window object.
📚 Learning: 2025-01-27T17:29:33.929Z
Learnt from: ThatDeparted2061
Repo: CircuitVerse/cv-frontend-vue PR: 442
File: src/simulator/src/wire.ts:0-0
Timestamp: 2025-01-27T17:29:33.929Z
Learning: In the CircuitVerse frontend Vue project, globalScope should be declared on the window object using TypeScript declaration files (.d.ts) rather than importing it as a module.

Applied to files:

  • src/types/global.d.ts
📚 Learning: 2025-01-27T17:29:33.929Z
Learnt from: ThatDeparted2061
Repo: CircuitVerse/cv-frontend-vue PR: 442
File: src/simulator/src/wire.ts:0-0
Timestamp: 2025-01-27T17:29:33.929Z
Learning: In the CircuitVerse frontend Vue project, globalScope is a global variable that should be typed by extending the Window interface in a TypeScript declaration file (global.d.ts), as it's initialized on the window object.

Applied to files:

  • src/types/global.d.ts
🔇 Additional comments (1)
src/types/global.d.ts (1)

1-5: LGTM! Correct structure for global augmentation.

The file structure is correct with export {} making this a module, which allows the declare global block to extend the Window interface. The comment clearly explains the purpose of type-only augmentation.

Based on learnings.

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.

Feature: Javascript to Typescript conversion in the src folder

1 participant