fix: Resolve Verilog compilation stuck issue in Tauri desktop app #919
Conversation
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThis pull request introduces TAURI support for Verilog compilation. The changes include: adding a CircuitVerse API endpoint to the TAURI HTTP allowlist, converting the Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ 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: 1
🤖 Fix all issues with AI agents
In `@src-tauri/capabilities/default.json`:
- Around line 20-23: The allow-fetch list in src-tauri/capabilities/default.json
only lists the production Verilog endpoint but Verilog2CV.js builds the endpoint
from VITE_API_URL (see VITE_API_URL usage in Verilog2CV.js around lines
269–273), so Tauri can block non-prod hosts; fix by either adding all expected
dev/staging base URLs (and their /api/v1/simulator/verilogcv paths) to the
allow-fetch array in default.json, or modify Verilog2CV.js to use a hard-coded
production URL when running inside Tauri (detectable via TAURI env or process)
so the Tauri allow-list and the runtime endpoint always match.
🧹 Nitpick comments (1)
src/simulator/src/Verilog2CV.js (1)
269-274: Prefer robust URL joining to avoid double‑slash edge cases.If
VITE_API_URLends with/, concatenation can yield//api/.... It usually works, but usingURLkeeps it clean and avoids subtle path issues.Suggested refactor
- let apiUrl = '/api/v1/simulator/verilogcv' + const apiPath = '/api/v1/simulator/verilogcv' + let apiUrl = apiPath if (isTauri()) { const baseUrl = import.meta.env.VITE_API_URL || 'https://circuitverse.org' - apiUrl = `${baseUrl}${apiUrl}` + apiUrl = new URL(apiPath, baseUrl).toString() }
Nakshatra480
force-pushed
the
fix/verilog-compilation-tauri
branch
3 times, most recently
from
31e899f to
990551c
Compare
Nakshatra480
force-pushed
the
fix/verilog-compilation-tauri
branch
3 times, most recently
from
195ae76 to
0f69be9
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@package.json`:
- Line 59: The package.json currently pins "cross-env": "^10.1.0" which is
incompatible with Node 18; update the dependency entry for "cross-env" to a
Node-18-compatible version such as "^7.0.3" (change the value in package.json),
then reinstall/update lockfiles (npm install or yarn install) so
package-lock.json / yarn.lock reflect the change; ensure the project build
scripts that reference cross-env continue to work after the version change.
In `@src/simulator/src/Verilog2CV.js`:
- Around line 261-264: The async function generateVerilogCircuit currently
starts an fetch chain with .then() but does not return that promise, so callers
cannot await completion; update generateVerilogCircuit to either return the
fetch promise chain or (preferred) refactor to use await for clarity: await the
fetch call, await response.json()/text as appropriate, and await any subsequent
processing (look at the similar .then() chain around lines referenced 284-290)
and return the final result (or throw on error) so callers can await
generateVerilogCircuit; ensure references to generateVerilogCircuit and the
fetch/response handling are updated accordingly.
🧹 Nitpick comments (2)
src/simulator/src/Verilog2CV.js (2)
322-322: Use strict equality for status comparison.Prefer
===over==for type-safe comparison.🔧 Suggested fix
- if (error.status == 500) { + if (error.status === 500) {
62-66: Consider makingsaveVerilogCodeasync and awaitinggenerateVerilogCircuit.Since
generateVerilogCircuitis now async, calling it withoutawaitmeanssaveVerilogCodereturns before compilation completes. If callers need to know when compilation finishes (e.g., to sequence operations), this could cause subtle issues.If fire-and-forget is intentional, this is acceptable since the UI feedback is handled within
generateVerilogCircuit. Otherwise, consider:♻️ Proposed change
-export function saveVerilogCode() { +export async function saveVerilogCode() { var code = editor.getValue() globalScope.verilogMetadata.code = code - generateVerilogCircuit(code) + await generateVerilogCircuit(code) }
Nakshatra480
force-pushed
the
fix/verilog-compilation-tauri
branch
4 times, most recently
from
5979357 to
eb8dc4d
Compare
Nakshatra480
force-pushed
the
fix/verilog-compilation-tauri
branch
from
eb8dc4d to
c66a86f
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/simulator/src/Verilog2CV.js (1)
1-5:⚠️ Potential issue | 🔴 CriticalMissing import:
scopeListis not defined.
scopeListis used on line 291 but not imported. This will cause aReferenceErrorat runtime when attempting to delete sub-circuit scopes, breaking the Verilog compilation flow.🐛 Proposed fix: Add scopeList to imports
import { createNewCircuitScope, switchCircuit, changeCircuitName, + scopeList, } from './circuit'Also applies to: 290-291
🤖 Fix all issues with AI agents
In `@src/simulator/src/Verilog2CV.js`:
- Around line 271-274: The empty catch in the isTauri() branch swallows plugin
import errors; update the block that assigns fetchFn (the code that does try {
fetchFn = (await import('@tauri-apps/plugin-http')).fetch } catch (e) {}) to log
the caught error (including e.message/stack) via console.error or the project's
logger and optionally include a clear message that the code will fall back to
window.fetch; ensure the symbol references are the existing isTauri() check and
the dynamic import('@tauri-apps/plugin-http').fetch so future failures are
visible for debugging.
🧹 Nitpick comments (1)
src/simulator/src/Verilog2CV.js (1)
311-313: Empty catch block obscures JSON parsing failures.While
msghas a fallback value, the empty catch block makes the intent unclear. Consider logging for debugging or adding a comment to indicate the intentional fallback behavior.♻️ Proposed improvement
else if (typeof error.json === 'function') { - try { msg = (await error.json()).message || msg } catch (e) {} + try { + msg = (await error.json()).message || msg + } catch { + // Response body not JSON-parseable, use existing msg + } }
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/simulator/src/Verilog2CV.js`:
- Around line 294-297: The loop that removes sub-circuit entries uses "for (var
id in scope.verilogMetadata.subCircuitScopeIds)" which iterates indices rather
than actual ID values; change it to iterate values (e.g., "for (const id of
scope.verilogMetadata.subCircuitScopeIds)" or use .forEach) so delete
scopeList[id] uses the real scope IDs; keep the surrounding calls
(scope.initialize() and resetting scope.verilogMetadata.subCircuitScopeIds = [])
as-is and only replace the loop iteration method in Verilog2CV.js.
725c0a1 to
4f43452
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
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/modules/ConstantVal.js (1)
46-50:⚠️ Potential issue | 🟡 MinorDuplicate
generateVerilog()method definitions.The class defines
generateVerilog()twice (lines 46-50 and lines 170-172). In JavaScript, the second definition silently shadows the first. This appears unintentional—the first generates alocalparamdeclaration while the second generates anassignstatement with different syntax.🔧 Proposed fix - remove the first definition
this.label = '' } - generateVerilog() { - return `localparam [${this.bitWidth - 1}:0] ${this.verilogLabel}=${ - this.bitWidth - }b'${this.state};` - } - /** * `@memberof` ConstantValAlso applies to: 170-172
src/simulator/src/modules/ConstantVal.js (1)
46-50:⚠️ Potential issue | 🟡 MinorDuplicate
generateVerilog()method definitions.Same issue as in the v1 version—the class has two
generateVerilog()definitions. The second silently shadows the first.🔧 Proposed fix - remove the first definition
this.label = '' } - generateVerilog() { - return `localparam [${this.bitWidth - 1}:0] ${this.verilogLabel}=${ - this.bitWidth - }b'${this.state};` - } - /** * `@memberof` ConstantValAlso applies to: 170-172
🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 16-23: The dependency entry for `@tauri-apps/plugin-http` is set to
^2.5.7 which doesn't exist on npm and is incompatible with the project's Tauri
2.3.1; either change the `@tauri-apps/plugin-http` version in package.json to a
compatible release such as ^2.4.0 or upgrade the Tauri dependency to >=2.10.0 so
the plugin's peer requirements are met—update the "@tauri-apps/plugin-http"
version or the "tauri" dependency accordingly and run a fresh install to verify
resolution.
In `@v1/src/simulator/src/ux.js`:
- Around line 407-410: The forEach callback in exitFullView uses an
expression-bodied arrow (exitViewBtns.forEach(btn => btn.remove())) which
triggers the Biome lint rule; change it to a block-bodied arrow so there is no
implicit return, e.g. replace the callback with btn => { btn.remove(); } inside
the exitFullView function when iterating over elements selected by
'#exitViewBtn'.
🧹 Nitpick comments (9)
src/simulator/src/modules/Flag.js (1)
200-210: Pre-existing issue: duplicatehelplinkassignment.
Flag.prototype.helplinkis assigned on lines 200–201 (#flag) and then immediately overwritten on lines 209–210 (#tunnel). The second assignment wins, so the help link likely points to the wrong documentation anchor. This isn't introduced by your PR, but worth fixing if you're already touching this file.🔧 Suggested fix
Remove the duplicate assignment and keep only the correct one:
Flag.prototype.tooltipText = 'Flag ToolTip: Use this for debugging and plotting.' Flag.prototype.helplink = 'https://docs.circuitverse.org/chapter4/chapter4-misc#flag' -/** - * `@memberof` Flag - * Help URL - * `@type` {string} - * `@category` modules - */ -Flag.prototype.helplink = - 'https://docs.circuitverse.org/chapter4/chapter4-misc#tunnel'src/simulator/src/modules/Input.js (1)
3-3: Optional: consolidate imports from the same module.♻️ Suggested cleanup
-import Node, { findNode } from '../node' +import Node, { findNode, dec2bin, bin2dec } from '../node' @@ -import { dec2bin, bin2dec } from '../node'Also applies to: 21-21
src/simulator/src/modules/Output.js (1)
2-2: Optional: consolidate imports from the same module.♻️ Suggested cleanup
-import Node, { findNode } from '../node' +import Node, { findNode, dec2bin } from '../node' @@ -import { dec2bin } from '../node'Also applies to: 8-8
src/simulator/src/testbench/testbenchOutput.js (1)
4-5: Optional: consolidate imports from the same module.♻️ Suggested cleanup
-import Node, { findNode } from '../node' -import { dec2bin } from '../node' +import Node, { findNode, dec2bin } from '../node'v1/src/simulator/src/modules/ConstantVal.js (1)
6-6: Unused import:dec2binis imported but never used.Only
bin2decis used in this file (in theresolve()method). Thedec2binimport should be removed.🧹 Proposed fix
-import { dec2bin, bin2dec } from '../node' +import { bin2dec } from '../node'build-desktop.js (2)
24-24: Movefsimport to the top of the file with other imports.The
fsmodule is imported mid-file. For consistency and readability, group all imports at the top.🧹 Proposed fix
const os = require('os'); const { execSync } = require('child_process'); +const fs = require('fs'); function runCommand(command) {And remove line 24:
runCommand('npm run build') -const fs = require('fs') - if (!fs.existsSync('dist/index-cv.html')) {
1-1: Remove leading empty line.Minor nit: the file starts with an unnecessary blank line.
src/simulator/src/modules/ConstantVal.js (1)
6-6: Unused import:dec2binis imported but never used.Same issue as in the v1 version—only
bin2decis used in this file.🧹 Proposed fix
-import { dec2bin, bin2dec } from '../node' +import { bin2dec } from '../node'src/simulator/src/ux.js (1)
409-410: Static analysis warning:forEachcallback implicitly returns a value.While harmless (the return value is ignored), using block syntax clarifies intent and silences the linter.
🧹 Proposed fix
const exitViewBtns = document.querySelectorAll('#exitViewBtn') - exitViewBtns.forEach(btn => btn.remove()) + exitViewBtns.forEach(btn => { btn.remove() })
Nakshatra480
force-pushed
the
fix/verilog-compilation-tauri
branch
from
4f43452 to
1a71ca6
Compare
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 (1)
v1/src/simulator/src/ux.js (1)
255-258:⚠️ Potential issue | 🟡 MinorTypo in JSDoc comment.
The comment says "checkss" but should say "checks" (extra 's'). The
src/simulator/src/ux.jsversion has this corrected.📝 Proposed fix
/** - * checkss the input is safe or not + * checks the input is safe or not * `@param` {HTML} unsafe - the html which we wants to escape * `@category` ux */
🧹 Nitpick comments (1)
src/simulator/src/Verilog2CV.js (1)
123-140: Redundant DOM queries after null checks.After retrieving elements into variables, the code queries the DOM again instead of using the cached reference. This pattern repeats throughout the function.
♻️ Proposed fix (example for one block)
const code_window = document.getElementById('code-window') - if (code_window) - document.getElementById('code-window').style.display = 'block' + if (code_window) code_window.style.display = 'block' const elementPanel = document.querySelector('.elementPanel') - if (elementPanel) - document.querySelector('.elementPanel').style.display = 'none' + if (elementPanel) elementPanel.style.display = 'none'Apply the same pattern to
timingDiagramPanel,quickBtn,verilogEditorPanel, and their counterparts in theelseblock.
Nakshatra480
force-pushed
the
fix/verilog-compilation-tauri
branch
from
1a71ca6 to
d582544
Compare
- Use absolute API URL (circuitverse.org) for Tauri to bypass relative URL issues - Integrate @tauri-apps/plugin-http (v2.4.0) to bypass CORS in Tauri webview - Add conditional logic with isTauri() for environment detection - Improve error handling and user feedback Fixes CircuitVerse#785
Nakshatra480
force-pushed
the
fix/verilog-compilation-tauri
branch
from
d582544 to
35b1403
Compare
- Preserved @tauri-apps/plugin-http v2.4.0 from PR - Accepted cross-env v10.1.0 upgrade from upstream
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/simulator/src/Verilog2CV.js`:
- Around line 311-313: The empty catch block around parsing error.json should
not be left blank; update the handler in Verilog2CV.js where you read error.json
(the block using "else if (typeof error.json === 'function')") to catch the
parsing exception into a variable (e) and at minimum log or debug that exception
(e.g., console.debug/console.warn or your logger) so the linter is satisfied and
parsing failures are surfaced; keep the existing behavior of falling back to the
existing msg when parsing fails.
- Around line 289-292: The code references scopeList in the block that calls
scope.initialize() and iterates scope.verilogMetadata.subCircuitScopeIds, but
scopeList is not imported causing a runtime ReferenceError; fix by adding an
import for scopeList at the top of this module (the same module that defines
Verilog2CV.js) so the loop "for (var id in
scope.verilogMetadata.subCircuitScopeIds) delete scopeList[id]" can access the
symbol, ensure the import uses the correct exported name (scopeList) from its
source module and matches surrounding import style.
41f5abd to
0fd938e
Compare
|
@Nihal4777 pls review these changes, issue is solved properly. |
1 participant
Fix Verilog Compilation Stuck Issue in Tauri Desktop App
Description
This PR resolves the issue where Verilog compilation would get stuck in the Tauri desktop application. The problem was caused by the use of relative URLs for the API endpoint, which are not resolved correctly in the Tauri environment, and CORS restrictions.
Changes
https://circuitverse.org/api/v1/simulator/verilogcv) for Verilog compilation requests.@tauri-apps/plugin-httpto bypass CORS restrictions inherent to the webview environment in Tauri.@tauri-apps/plugin-httpand updatedcross-envto a Node-18 compatible version to fix build issues.src-tauri/capabilities/default.jsonto allow fetch requests to the CircuitVerse API.Verilog2CV.jsto use cleaner, more conciseasync/awaitlogic and improved error handling.Fixes
Checklist
Summary by CodeRabbit
Release Notes
New Features
UI/UX Improvements