Combining Vue Simulator Versioning PRs #337
Conversation
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
ESLint found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
| // const { activeCircuit } = toRefs(simulatorStore) | ||
| name = name || 'Untitled' | ||
| name = stripTags(name) | ||
| scopeList[id].name = name |
Check warning
Code scanning / CodeQL
Prototype-polluting assignment Medium
| scope.backups[scope.backups.length - 1] !== backup | ||
| ) { | ||
| scope.backups.push(backup) | ||
| scope.history = [] |
Check warning
Code scanning / CodeQL
Prototype-polluting assignment Medium
| ) { | ||
| scope.backups.push(backup) | ||
| scope.history = [] | ||
| scope.timeStamp = new Date().getTime() |
Check warning
Code scanning / CodeQL
Prototype-polluting assignment Medium
| */ | ||
| function loadModule(data, scope) { | ||
| // Create circuit element | ||
| var obj = new modules[rectifyObjectType(data.objectType)]( |
Check failure
Code scanning / CodeQL
Unvalidated dynamic method call High
… openProjectOffline
WalkthroughThe changes introduce a new Vue component, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OpenOffline
participant SimulatorState
User->>OpenOffline: Opens offline project dialog
OpenOffline->>SimulatorState: Fetches project list
SimulatorState-->>OpenOffline: Returns project list
User->>OpenOffline: Selects a project
OpenOffline->>OpenOffline: Checks project version
alt Version mismatch
OpenOffline-->>User: Displays version mismatch dialog
User->>OpenOffline: Chooses to continue or cancel
else No mismatch
OpenOffline-->>User: Loads selected project
end
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (3)
v0/src/components/DialogBox/OpenOffline.vue (3)
33-35: Prevent click event on delete icon from selecting the radio buttonClicking the delete icon inside the label may inadvertently select the radio button due to event propagation. To prevent this, use the
.stopmodifier on the@clickevent.Apply this change:
Line 35~:
- @click="deleteOfflineProject(projectId)" + @click.stop="deleteOfflineProject(projectId)"This ensures that clicking the delete icon does not affect the radio button selection.
45-60: Use a computed property to check ifprojectListis emptyComparing
JSON.stringify(projectList)to'{}'is inefficient and may lead to unexpected results. Instead, use a computed property to check ifprojectListis empty.In your script, add the computed property:
+ import { computed } from '@vue/runtime-core' + const isProjectListEmpty = computed(() => { + return Object.keys(projectList.value).length === 0 + })Update the template to use this computed property:
Lines 37~:
- <p v-if="JSON.stringify(projectList) == '{}'"> + <p v-if="isProjectListEmpty">Lines 45~ to 60~:
<v-btn - v-if="JSON.stringify(projectList) != '{}'" + v-if="!isProjectListEmpty" class="messageBtn" block @click="openProjectOffline()" > open project </v-btn> <v-btn - v-else + v-if="isProjectListEmpty" class="messageBtn" block @click.stop="OpenImportProjectDialog" > open CV file </v-btn>This approach is cleaner and enhances readability.
101-101: Useconstinstead ofvarfor variable declarationUsing
constfor variable declarations that are not reassigned promotes better coding practices and prevents accidental reassignment.Line 101~:
- var data = localStorage.getItem('projectList') + const data = localStorage.getItem('projectList')
| function openProjectOffline() { | ||
| SimulatorState.dialogBox.open_project_dialog = false | ||
| let ele = $('input[name=projectId]:checked') | ||
| if (!ele.val()) return | ||
| const projectData = JSON.parse(localStorage.getItem(ele.val())) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid using jQuery in Vue components; utilize Vue's reactivity system instead
Using jQuery inside Vue components is not recommended as it can lead to unpredictable state management and makes the code harder to maintain. Instead, leverage Vue's reactivity to handle user inputs.
You can bind the selected project ID to a reactive variable using v-model.
First, declare a reactive variable for the selected project ID:
+ const selectedProjectId = ref('')Update the radio input to use v-model:
Lines 26~ to 30~:
< input
type="radio"
name="projectId"
:value="projectId"
+ v-model="selectedProjectId"
/>Modify the openProjectOffline function to use the reactive variable:
Lines 115~ to 117~:
- let ele = $('input[name=projectId]:checked')
- if (!ele.val()) return
- const projectData = JSON.parse(localStorage.getItem(ele.val()))
+ if (!selectedProjectId.value) return
+ const projectData = JSON.parse(localStorage.getItem(selectedProjectId.value))Committable suggestion was skipped due to low confidence.
| onUpdated(() => { | ||
| var data = localStorage.getItem('projectList') | ||
| projectList.value = JSON.parse(localStorage.getItem('projectList')) || {} | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid using onUpdated for initialization; use onMounted instead
Using onUpdated to initialize projectList can cause unnecessary executions every time the component updates, impacting performance. Since you need to load projectList when the component mounts, it's better to use onMounted.
Move the initialization code to onMounted:
Lines 96~ to 98~:
onMounted(() => {
SimulatorState.dialogBox.open_project_dialog = false
+ projectList.value = JSON.parse(localStorage.getItem('projectList')) || {}
})Remove the onUpdated hook:
Lines 100~ to 103~:
- onUpdated(() => {
- var data = localStorage.getItem('projectList')
- projectList.value = JSON.parse(localStorage.getItem('projectList')) || {}
- })📝 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.
| onUpdated(() => { | |
| var data = localStorage.getItem('projectList') | |
| projectList.value = JSON.parse(localStorage.getItem('projectList')) || {} | |
| }) | |
| onMounted(() => { | |
| SimulatorState.dialogBox.open_project_dialog = false | |
| projectList.value = JSON.parse(localStorage.getItem('projectList')) || {} | |
| }) |
| function confirmOpenProject() { | ||
| SimulatorState.dialogBox.version_mismatch_dialog = false | ||
| // Redirect to the appropriate version after confirmation | ||
| if(targetVersion.value == "Legacy"){ | ||
| window.location.href = `/simulator/edit/${projectName}` | ||
| }else{ | ||
| window.location.href = `/simulatorvue/edit/${projectName}?simver=${targetVersion.value}` | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Validate targetVersion.value before using it in the redirect URL
Using variables from local storage directly in URLs can pose security risks if manipulated maliciously. Ensure that targetVersion.value contains only expected values before redirecting.
You can validate targetVersion.value against a whitelist of allowed versions:
function confirmOpenProject() {
SimulatorState.dialogBox.version_mismatch_dialog = false
+ const allowedVersions = ['v0', 'v1', 'Legacy']; // Add all allowed versions
+ if (!allowedVersions.includes(targetVersion.value)) {
+ alert('Invalid simulator version.');
+ return;
+ }
// Redirect to the appropriate version after confirmation
if(targetVersion.value == "Legacy"){
window.location.href = `/simulator/edit/${projectName}`
}else{
window.location.href = `/simulatorvue/edit/${projectName}?simver=${targetVersion.value}`
}
}This ensures that only valid versions are used in the redirect URL.
📝 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.
| function confirmOpenProject() { | |
| SimulatorState.dialogBox.version_mismatch_dialog = false | |
| // Redirect to the appropriate version after confirmation | |
| if(targetVersion.value == "Legacy"){ | |
| window.location.href = `/simulator/edit/${projectName}` | |
| }else{ | |
| window.location.href = `/simulatorvue/edit/${projectName}?simver=${targetVersion.value}` | |
| } | |
| } | |
| function confirmOpenProject() { | |
| SimulatorState.dialogBox.version_mismatch_dialog = false | |
| const allowedVersions = ['v0', 'v1', 'Legacy']; // Add all allowed versions | |
| if (!allowedVersions.includes(targetVersion.value)) { | |
| alert('Invalid simulator version.'); | |
| return; | |
| } | |
| // Redirect to the appropriate version after confirmation | |
| if(targetVersion.value == "Legacy"){ | |
| window.location.href = `/simulator/edit/${projectName}` | |
| }else{ | |
| window.location.href = `/simulatorvue/edit/${projectName}?simver=${targetVersion.value}` | |
| } | |
| } |
Fixes #372
Describe the changes you have made in this PR -
Combining all the PRs related to Vue simulator versioning
Screenshots of the changes (If any) -
#332 - Enable the Main simulator to dynamically load the Vue simulator.
#327 - build: script modification for different builds.
#327 - chore: adding versioned directory(v0 and v1).
Key Achievements:
Versioning Directories:
v0,v1, etc.) to manage multiple versions of the Vue simulator, allowing for independent development and maintenance of each version.Modifying the Build Script:
vite.config.v0.ts,vite.config.v1.ts) to ensure proper building and output management for each simulator version.Bash Script for Multi-Version Builds:
Hot-Swapping Vue Simulator Versions:
simverquery parameter, allowing users to seamlessly switch between different versions.Dynamic Script Injection:
createHtmlPluginin Vite, ensuring that the correct version-specific script tags are dynamically inserted into theindex.htmlduring the build process.Storing Simulator Version in Circuit Data:
circuit_datato store the simulator version, enabling circuits to be loaded with the appropriate simulator version based on their creation version.Redirecting to Correct Simulator Version:
simulatorVersionincircuit_data, ensuring compatibility and functionality.Creation of
index-cv.html:index-cv.htmlto serve as the entry point for the main repository, while the defaultindex.htmlcontinues to serve thecv-frontend-vuerepository, maintaining stability during development.Setting
v0as Default Simulator:v0as the default simulator version, ensuring consistency during the development process.Fixing Circuit Preview Image Issue:
Version-Specific Links for Launching Circuits:
Future Work:
/simulatorvueNote: Please check Allow edits from maintainers. if you would like us to assist in the PR.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style