Skip to content

Make simulator API base configurable#1016

Open
mundele2004 wants to merge 1 commit into
CircuitVerse:mainfrom
mundele2004:fix-configurable-api-base
Open

Make simulator API base configurable#1016
mundele2004 wants to merge 1 commit into
CircuitVerse:mainfrom
mundele2004:fix-configurable-api-base

Conversation

@mundele2004

@mundele2004 mundele2004 commented Mar 9, 2026

Copy link
Copy Markdown

Fixes #1015

Describe the changes you have made in this PR -

  • Introduced a small API base helper module for both simulator versions:
    • src/simulator/src/apiBase.js
    • v1/src/simulator/src/apiBase.js
  • The helper determines the API base URL in this order:
    • window.CV_API_BASE (runtime override, no trailing slash)
    • import.meta.env.VITE_API_BASE (build-time env, no trailing slash)
    • Fallback to /api (current CircuitVerse Rails default)
  • Updated fetchProjectData in:
    • src/simulator/src/setup.js
    • v1/src/simulator/src/setup.js
      to use buildApiUrl('/v1/projects/${projectId}/circuit_data') instead of a hard-coded /api/v1/... path.
  • This keeps the default behavior unchanged on circuitverse.org, but allows the Vue simulator to be mounted on custom paths or separate origins while pointing to any API base path.

Screenshots of the UI changes (If any) -

N/A – no UI changes, only configuration / data-loading behavior.


Code Understanding and AI Usage

Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?

  • No, I wrote all the code myself
  • Yes, I used AI assistance (continue below)

If you used AI assistance:

  • I have reviewed every single line of the AI-generated code
  • I can explain the purpose and logic of each function/component I added
  • I have tested edge cases and understand how the code handles them
  • I have modified the AI output to follow this project's coding standards and conventions

Explain your implementation approach:

  • The problem: the Vue simulator fetched project circuit data from a hard-coded absolute path (/api/v1/projects/:id/circuit_data), which breaks when the simulator is hosted under a custom base path or on a different origin where the API is not available at /api.
  • I introduced an API base resolver (getApiBase + buildApiUrl) that first checks a runtime global (window.CV_API_BASE), then a build-time env variable (VITE_API_BASE), and finally falls back to /api. This keeps current production behavior while enabling route-agnostic and decoupled deployments.
  • I chose this helper-based approach instead of inlining env checks in setup.js because:
    • It centralizes the logic and makes future API calls easy to update.
    • It keeps setup.js focused on simulator setup and loading, not environment detection.
  • Key functions:
    • getApiBase() returns the normalized API base URL (no trailing /), using the preference order above.
    • buildApiUrl(path) joins the base with a given path fragment (with or without leading /), guaranteeing exactly one slash between them.
    • fetchProjectData(projectId) in both v0 and v1 now uses buildApiUrl('/v1/projects/${projectId}/circuit_data') so that all project-loading requests respect the configurable base.
  • If I were a watermelon, I would still make sure the API base URL is refreshingly configurable. 🍉

Checklist before requesting a review

  • I have added proper PR title and linked to the issue
  • I have performed a self-review of my code
  • I can explain the purpose of every function, class, and logic block I added
  • I understand why my changes work and have tested them thoroughly
  • I have considered potential edge cases and how my code handles them
  • If it is a core feature, I have added thorough tests
  • My code follows the project's style guidelines and conventions

I’d appreciate feedback on whether this approach aligns with the project's preferred configuration pattern.

Summary by CodeRabbit

  • Refactor
    • Enhanced API endpoint configuration infrastructure to support flexible URL resolution through environment variables and custom configurations, improving deployment flexibility and maintainability.

@netlify

netlify Bot commented Mar 9, 2026

Copy link
Copy Markdown

Deploy Preview for circuitverse ready!

Name Link
🔨 Latest commit 0d2315a
🔍 Latest deploy log https://app.netlify.com/projects/circuitverse/deploys/69af107c94eba70008449c44
😎 Deploy Preview https://deploy-preview-1016--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: 47 (🔴 down 1 from production)
Accessibility: 66 (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

coderabbitai Bot commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

This change introduces API base URL utilities in both the v0 and v1 simulator directories. Two new modules (apiBase.js) are created that export getApiBase() and buildApiUrl(path) functions. The getApiBase() function resolves the API base URL using a priority order: window.CV_API_BASE, import.meta.env.VITE_API_BASE, or a default /api. The buildApiUrl(path) function constructs complete API URLs by combining the resolved base with a provided path. The existing setup.js files in both directories are updated to use buildApiUrl() instead of hard-coded API endpoint strings.

Suggested labels

potential-ai-slop

🚥 Pre-merge checks | ✅ 5
✅ 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 'Make simulator API base configurable' directly and clearly summarizes the main change: introducing configurable API base functionality via helper utilities and replacing hard-coded paths.
Linked Issues check ✅ Passed The PR fully addresses issue #1015 by eliminating hard-coded absolute API paths, introducing configurable API base resolution (window.CV_API_BASE, VITE_API_BASE, default /api), and applying fixes to both v0 and v1 simulator code.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the linked issue: new apiBase.js utilities and setup.js modifications in both simulator versions to make API base configurable.
Docstring Coverage ✅ Passed Docstring coverage is 100.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 unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
v1/src/simulator/src/setup.js (1)

100-120: ⚠️ Potential issue | 🟠 Major

Keep simulator-version redirects configurable too.

Line 117 and Line 120 still redirect to hard-coded absolute routes. That means a project load that switches simulator versions will bypass the new API-base configurability and still break when this app is mounted under a custom base path or paired with a different origin.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 471a3b3e-a02c-4c5f-976d-bfa3812bc016

📥 Commits

Reviewing files that changed from the base of the PR and between a302cbf and 0d2315a.

📒 Files selected for processing (4)
  • src/simulator/src/apiBase.js
  • src/simulator/src/setup.js
  • v1/src/simulator/src/apiBase.js
  • v1/src/simulator/src/setup.js

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.

🐞 Bug: Route-agnostic mounting breaks API requests (hard-coded /api/...)

1 participant