feat: integrated bun, fixed typescript errors, add bengali support in v0 #451
Conversation
|
Warning Rate limit exceeded@devsubid has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 22 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (2)
WalkthroughThe changes update various configuration and source files. The ESLint configuration now targets ES2020 and supports TypeScript. Project documentation in Changes
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
…t state management
…evsubid/cv-frontend-vue into devsubid-chore(bun,typescript)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (11)
types/window.d.ts (2)
1-2: Consider using @types/jquery instead of direct import.Instead of importing jQuery directly, consider using
@types/jquerypackage for type definitions.-import jQuery from 'jquery' +// Remove import and add @types/jquery to devDependencies
8-9: Replace 'any' types with more specific types.Using
any[]reduces type safety. Consider defining more specific types for these arrays.- restrictedElements: any[] + restrictedElements: string[] // or more specific type based on usage - elementHierarchy: any[] + elementHierarchy: Array<{ type: string; [key: string]: unknown }> // or more specific typeAlso applies to: 18-18
.eslintrc.js (1)
21-21: Consider adding common TypeScript ESLint rules.The empty rules object could benefit from some common TypeScript rules.
- rules: {}, + rules: { + '@typescript-eslint/no-explicit-any': 'warn', + '@typescript-eslint/explicit-function-return-type': 'warn', + '@typescript-eslint/no-unused-vars': ['error', { argsIgnorePattern: '^_' }], + },v0/src/globalVariables.ts (1)
14-15: Replace 'any' types with more specific types.Using
anytypes reduces TypeScript's effectiveness in catching potential issues.- projectId: any - id: any - logixProjectId: any + projectId: string | undefined + id: string | undefined + logixProjectId: string | string[] | undefinedAlso applies to: 22-22
vite.config.ts (1)
25-27: Consider documenting the purpose of each alias.While the aliases are well-structured, adding comments to explain their intended usage would improve maintainability.
alias: { + // Source directory '#': fileURLToPath(new URL('./v0/src', import.meta.url)), + // Components directory '@': fileURLToPath(new URL('./v0/src/components', import.meta.url)), + // Assets directory '~': fileURLToPath(new URL('./v0/src/assets', import.meta.url)), },v0/src/pages/simulatorHandler.vue (3)
14-17: Consider enhancing cookie handling with type safety and validation.The current cookie handling could be improved for better security and type safety.
-export function getToken(name: string) { +export function getToken(name: string): string | undefined { var match = document.cookie.match(new RegExp('(^| )' + name + '=([^;]+)')) - if (match) return match[2] + if (match && match[2]) { + return decodeURIComponent(match[2]) + } + return undefined }
34-62: Consider enhancing error handling with specific error types.The error handling could be more robust with specific error types and user feedback.
async function checkEditAccess() { + const handleError = (status: number) => { + switch (status) { + case 403: + case 404: + hasAccess.value = false + break + case 401: + window.location.href = '/users/sign_in' + break + default: + console.error(`Unexpected status: ${status}`) + } + isLoading.value = false + } + try { await fetch(`/api/v1/projects/${wd.logixProjectId}/check_edit_access`, { // ... existing config - }).then((res) => { - if (res.ok) { - res.json().then((data) => { - authStore.setUserInfo(data.data) - wd.isUserLoggedIn = true - isLoading.value = false - }) - } else if (res.status === 403) { - hasAccess.value = false - isLoading.value = false - } else if (res.status === 404) { - hasAccess.value = false - isLoading.value = false - } else if (res.status === 401) { - window.location.href = '/users/sign_in' - } }) + const res = await response + if (res.ok) { + const data = await res.json() + authStore.setUserInfo(data.data) + wd.isUserLoggedIn = true + isLoading.value = false + } else { + handleError(res.status) + } + } catch (error) { + console.error('Failed to check edit access:', error) + isLoading.value = false } }
65-84: Consider validating token before making API calls.Add token validation to prevent unnecessary API calls when token is missing.
async function getLoginData() { + const token = getToken('cvt') + if (!token) { + wd.isUserLoggedIn = false + return + } + try { const response = await fetch('/api/v1/me', { method: 'GET', headers: { Accept: 'application/json', - Authorization: `Token ${getToken('cvt')}`, + Authorization: `Token ${token}`, }, }) // ... rest of the function } }README.md (2)
21-23: Specify fenced code block languages.
Multiple fenced code blocks within the Installation and Cloud setup sections do not specify a language. For improved readability and syntax highlighting, consider adding an appropriate language identifier (e.g., ```bash for shell commands). For example:-``` +```bashApply this change to all code fences in the affected sections.
Also applies to: 27-29, 33-35, 39-41, 53-55
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
21-21: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
87-88: Refine section title formatting.
The "## To Dos -" header can be improved for clarity. Consider updating it to something like "## To-Dos" (using a hyphen) to conform with common style conventions.🧰 Tools
🪛 LanguageTool
[grammar] ~87-~87: It appears that a hyphen is missing in the plural noun “To-Dos”.
Context: ...d under the MIT License. ## To Dos - 1. **Creating the mobile version of ...(TO_DO_HYPHEN)
v0/src/locales/bn.json (1)
72-141: Extensive Panel Body Translation
The"panel_body"section is thorough, covering multiple areas such as circuit elements, timing diagrams, module information, layout properties, image rendering, context menus, bit conversion, custom shortcuts, and issue reporting.
- Nitpick: In the
"optional"field (line 136), there is a leading space (" [ঐচ্ছিক]"). Please double-check if this space is intentional or if it should be removed for UI consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
bun.lockbis excluded by!**/bun.lockbpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
.eslintrc.js(1 hunks)README.md(1 hunks)build.sh(1 hunks)netlify.toml(1 hunks)package.json(3 hunks)tsconfig.json(1 hunks)types/window.d.ts(1 hunks)v0/src/globalVariables.ts(1 hunks)v0/src/locales/bn.json(1 hunks)v0/src/locales/i18n.ts(1 hunks)v0/src/main.ts(0 hunks)v0/src/pages/simulatorHandler.vue(5 hunks)version/versionLoader.ts(1 hunks)vite.config.ts(3 hunks)vite.config.v0.ts(2 hunks)vite.config.v1.ts(2 hunks)
💤 Files with no reviewable changes (1)
- v0/src/main.ts
✅ Files skipped from review due to trivial changes (1)
- version/versionLoader.ts
🧰 Additional context used
🪛 Shellcheck (0.10.0)
build.sh
[warning] 3-3: Prefer mapfile or read -a to split command output (or quote to avoid splitting).
(SC2207)
🪛 LanguageTool
README.md
[grammar] ~87-~87: It appears that a hyphen is missing in the plural noun “To-Dos”.
Context: ...d under the MIT License. ## To Dos - 1. **Creating the mobile version of ...
(TO_DO_HYPHEN)
🪛 markdownlint-cli2 (0.17.2)
README.md
21-21: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
27-27: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
33-33: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
39-39: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
53-53: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🪛 Biome (1.9.4)
v0/src/globalVariables.ts
[error] 5-5: Do not shadow the global "Array" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (28)
v0/src/locales/i18n.ts (1)
4-4: LGTM! Bengali language support successfully integrated.The changes correctly add Bengali language support by importing the translations and including them in the i18n configuration.
Also applies to: 13-13
vite.config.ts (2)
9-11: LGTM! Good practice using constants for configuration values.Moving configuration values to constants improves maintainability and reusability across the codebase.
36-45: LGTM! Well-structured test configuration.The test configuration is properly set up with global settings, environment, and dependencies.
vite.config.v1.ts (3)
9-11: LGTM! Constants are consistent with other config files.Good practice maintaining consistency across configuration files.
25-29: LGTM! HTML injection is properly configured.The script injection is correctly set up for v1 module.
44-47: LGTM! Build input configuration is properly set up.The rollup configuration correctly specifies the entry point.
vite.config.v0.ts (2)
8-10: LGTM! Constants maintain consistency across configurations.Good practice keeping configuration values consistent across different versions.
35-35: LGTM! Asset alias improves resource management.The '~' alias for assets directory enhances maintainability.
v0/src/pages/simulatorHandler.vue (1)
24-27: LGTM! Good TypeScript implementation with Window interface.The Window interface implementation improves type safety for global properties.
build.sh (3)
8-8: Transition to Bun tooling confirmed.
The build command has been updated to usebunx vite build --config vite.config."$version".ts, aligning with the PR’s objective of integrating Bun. This change is clear and consistent with the other configuration updates.
16-16: Successful build confirmation message.
The final echo statement "All builds completed successfully" appropriately confirms the completion of all builds.
3-3: Improve array population from command output.
The current approach using the command substitution splits the output by whitespace. Consider using a more robust method (e.g., usingreadarrayormapfile) to avoid unintended word splitting, which can happen if any version strings contain spaces. For example:-versions=($(jq -r '.[].version' version.json | tr -d '\r')) +readarray -t versions < <(jq -r '.[].version' version.json | tr -d '\r')🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 3-3: Prefer mapfile or read -a to split command output (or quote to avoid splitting).
(SC2207)
netlify.toml (1)
2-2: Updated build command using Bun.
Changing the build command to
command = "bun run build && mkdir -p ./dist && cp -r ../public/* ./dist/"
correctly reflects the migration to Bun. This ensures consistency with the build script and overall project tooling updates.tsconfig.json (3)
15-15: Verify updated module resolution base.
The change"baseUrl": "."(from a previous"./") alters module lookup behavior. Please verify that all module paths resolve as intended with the new base directory.
17-19: Review custom path mappings.
The updated path mappings now include:
"@/*": ["./src/components/*"]"~/*": ["./."]
The new"~/*"mapping is quite broad because it maps to the project root. Ensure that this does not introduce unintended module resolution conflicts elsewhere in the codebase.
25-26: Enable useful TypeScript features.
The additions"declarationMap": trueand"allowSyntheticDefaultImports": truehelp improve type declaration discovery and import flexibility. These changes are appropriate given the project’s updated TypeScript support.package.json (2)
4-4: Confirm project visibility update.
Changing"private": trueto"private": falseindicates that the project is now intended to be publicly accessible (or publishable). Please ensure this aligns with your release and distribution strategy.
44-46: Update dependency versions as planned.
The version updates for@commitlint/cli,@commitlint/config-conventional,@intlify/vite-plugin-vue-i18n, andtypescriptare all in line with the PR objectives. These updates will improve tooling, linting, and TypeScript support.Also applies to: 59-59
README.md (1)
6-8: Enhanced project description and introduction.
The updated introductory section now clearly states the project goals including decoupling the simulator backend, removing jQueryUI, and integrating internationalization features. This provides valuable context for new contributors and users.v0/src/locales/bn.json (9)
1-11: Comprehensive JSON Header Setup
The top-level"simulator"object and its immediate keys (e.g.,"save_online","save_offline", etc.) are well-structured and properly localized. The JSON formatting and indentation are clear, ensuring maintainability.
12-20: Localized Navigation – Intro & User Dropdown
The"nav"section is introduced cleanly here with clear translations for general navigation entries and the nested"user_dropdown". The keys reflect proper intent, and the translations appear appropriate for the Bengali audience.
21-34: Localized Navigation – Project Section
The"project"sub-section within"nav"contains detailed keys (e.g.,"new_project","open_offline","save_online", etc.) which are accurately translated. Please verify that any repeated phrases (like"save_online") used in different contexts are indeed intentional for UI consistency.
35-40: Localization for Circuit Section
The"circuit"block includes keys such as"new_circuit","insert_subcircuit", and"new_verilog_module_html". Using a newline character in"new_verilog_module_html"is handled correctly. Ensure that the rendered UI supports multiline text as expected.
41-49: Tools Section Localization
The"tools"section provides translations for tool-related actions (e.g.,"combinational_analysis_html","hex_bin_dec_converter_html","download_image", etc.). The values are concise and appropriately localized, supporting a seamless experience.
50-56: Help Section Localization
The"help"block cleanly presents translations for assistance and tutorial-related resources. The clarity in keys like"tutorial_guide","user_manual", and"discussion_forum"contributes positively to user guidance.
58-71: Panel Header Setup
The"panel_header"object specifies header labels for different panels (e.g.,"circuit_elements","layout_elements","timing_diagram", etc.). All headers are translated accurately; the consistency in phrasing will aid user navigation.
142-160: Tooltip Localization
The"tooltip"section presents a rich set of translations for tooltips covering various interactive actions (e.g.,"delete_selected","download_as_image","fit_to_screen", etc.). These keys are concise and provide the necessary guidance to users in Bengali.
161-163: JSON Closure
The file correctly terminates the JSON structure, ensuring no syntactical errors. Overall, the new localization file integrates well with the project's internationalization setup and will enhance the experience for Bengali-speaking users.
v0/src/globalVariables.ts
Outdated
| declare global { | ||
| interface Window { | ||
| restrictedElements: any[] | ||
| jQuery: typeof jQuery | ||
| $: typeof jQuery | ||
| globalScope: any | ||
| lightMode: boolean | ||
| projectId: any | ||
| id: any | ||
| loading: boolean | ||
| embed: boolean | ||
| width?: number | ||
| height?: number | ||
| DPR: number | ||
| isUserLoggedIn: boolean | ||
| logixProjectId: any | ||
| elementHierarchy: any[] | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move Window interface to types/window.d.ts.
The Window interface is duplicated. Consider moving all Window type definitions to types/window.d.ts to maintain a single source of truth.
-declare global {
- interface Window {
- // ... interface contents
- }
-}
+// Import Window type from types/window.d.ts if needed📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| declare global { | |
| interface Window { | |
| restrictedElements: any[] | |
| jQuery: typeof jQuery | |
| $: typeof jQuery | |
| globalScope: any | |
| lightMode: boolean | |
| projectId: any | |
| id: any | |
| loading: boolean | |
| embed: boolean | |
| width?: number | |
| height?: number | |
| DPR: number | |
| isUserLoggedIn: boolean | |
| logixProjectId: any | |
| elementHierarchy: any[] | |
| } | |
| } | |
| // Import Window type from types/window.d.ts if needed |
v0/src/globalVariables.ts
Outdated
| import jQuery from 'jquery' | ||
| import Array from './simulator/src/arrayHelpers.js' |
There was a problem hiding this comment.
Rename imported Array to avoid shadowing global Array.
The static analysis tool correctly flags that importing as 'Array' shadows the global Array property.
-import Array from './simulator/src/arrayHelpers.js'
+import ArrayHelpers from './simulator/src/arrayHelpers.js'Update the usage accordingly:
-window.Array = Array
+window.Array = ArrayHelpers📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import Array from './simulator/src/arrayHelpers.js' | |
| import ArrayHelpers from './simulator/src/arrayHelpers.js' | |
| // ... other global variables and interface definitions ... | |
| window.Array = ArrayHelpers; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 5-5: Do not shadow the global "Array" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
build.sh (2)
3-3: Enhance Array Population Using a Robust ApproachThe current command uses command substitution to populate the array, which may risk incorrect splitting if any element unexpectedly contains spaces. Shellcheck recommends using
mapfile(orread -a) for such cases. Consider refactoring as follows for improved robustness:-versions=($(jq -r '.[].version' version.json | tr -d '\r')) +mapfile -t versions < <(jq -r '.[].version' version.json | tr -d '\r')This change better handles newline-separated output without risking unwanted word splitting.
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 3-3: Prefer mapfile or read -a to split command output (or quote to avoid splitting).
(SC2207)
8-8: Clarify Variable Interpolation in the Build CommandThe build command now appropriately uses
bunxfor building with a version-specific configuration file. For better readability and to ensure robust concatenation, it’s recommended to use curly braces for the variable interpolation. For example:-bunx --bun vite build --config vite.config."$version".ts +bunx --bun vite build --config vite.config."${version}".tsThis small change clarifies the intended token boundaries and is a common best practice in shell scripting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
build.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
build.sh
[warning] 3-3: Prefer mapfile or read -a to split command output (or quote to avoid splitting).
(SC2207)
🔇 Additional comments (1)
build.sh (1)
16-16: Final Success Message is Clear and InformativeThe final echo statement confirms the completion of all builds successfully, which is clear and concise. No changes are needed here.
…rt Array from arrayHelpers
Fixes #
Summary by CodeRabbit
Summary by CodeRabbit
Documentation
New Features
Windowinterface for enhanced type safety and clarity in global property access.Configuration