Skip to content

Fix quotes and add debounced text field#40

Open
heavyrubberslave wants to merge 2 commits into
mainfrom
chore/fix-quotes
Open

Fix quotes and add debounced text field#40
heavyrubberslave wants to merge 2 commits into
mainfrom
chore/fix-quotes

Conversation

@heavyrubberslave

@heavyrubberslave heavyrubberslave commented Mar 16, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • New Features

    • Added debounced text input component for improved form handling
    • Added new backend URL setup page accessible at startup
    • Introduced enhanced type safety for device attribute validation
  • Improvements

    • Optimized health monitoring intervals for better performance
    • Improved store initialization and state management
    • Enhanced device control component with better attribute type checking
  • Chores

    • Removed Prettier dependency and simplified ESLint configuration
    • Code style standardization across the project
    • Cleaned up unused utility code

@coderabbitai

coderabbitai Bot commented Mar 16, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This pull request enforces single-quote string literals across the entire codebase via direct ESLint configuration, removes prettier and related dependencies, adds a new DebouncedTextField component for debounced input handling, introduces FloatDeviceAttribute type and device attribute type guards, removes TimeoutHelper class, and adjusts health store polling intervals.

Changes

Cohort / File(s) Summary
ESLint & Prettier Configuration
eslint.config.ts, package.json
Removes prettier and @vue/eslint-config-prettier dependencies; replaces skip-formatting import with explicit single-quote enforcement in ESLint config; changes lint script from auto-fix to report-only mode.
Quote Normalization - Core Files
src/main.ts, src/App.vue, src/router/index.ts, src/layouts/LayoutWithMenu.vue, src/plugins/*, src/stores/*, src/utils/apiFetch.ts, vite.config.ts, vue.config.js
Standardized string literals from double to single quotes across imports, configuration values, and runtime strings; no functional logic changes.
Quote Normalization - Components
src/components/**/*.vue, src/components/__tests__/HelloWorld.spec.ts, cypress/integration/example.spec.ts, cypress/support/index.ts
Updated import statements and string literals to use single quotes; no behavioral changes to component logic or test assertions.
Quote Normalization - Models & Utilities
src/model/**/*.ts, src/helper/ChartHelper.ts, src/shims-*.d.ts
Unified quote style in import paths and type annotations; minor formatting updates to type parameter literals (e.g., ChartData<"line">ChartData<'line'>).
DebouncedTextField Component
src/components/device/DebouncedTextField.vue
New Vue 3 single-file component wrapping v-text-field with debounced input emission; supports configurable debounce interval, focus tracking, and type variants (text/number).
Device Attribute Type System
src/model/devices/Device.ts, src/utils/utils.ts, src/components/device/control/GenericDeviceControl.vue
Added FloatDeviceAttribute interface with optional uom property; introduced type-guard functions (isStringDeviceAttribute, isIntDeviceAttribute, isFloatDeviceAttribute, isBoolDeviceAttribute); refactored GenericDeviceControl to use new predicates and DebouncedTextField component.
Store Enhancements
src/stores/devices.ts, src/stores/health.ts, src/stores/app.ts, src/stores/automation.ts, src/stores/settings.ts, src/stores/backend.ts
Added clear() method to devices store for clearing all device entries; adjusted health store polling intervals (data fetch 500ms→3000ms, chart update 425ms→1000ms); quote normalization in store initialization and state values.
Router & Backend Initialization
src/router/index.ts, src/main.ts
Added new /start route with BackendUrl component; integrated Pinia store and vueSocketIOClient with dynamic backend URL connection string.
TimeoutHelper Removal & DeviceCommunicator Update
src/helper/TimeoutHelper.ts, src/helper/DeviceCommunicator.ts
Removed TimeoutHelper class entirely; removed timeout-based device.receiveUpdates toggling from DeviceCommunicator emission logic; streamlined event emission.
Device Control Refinements
src/components/device/DeviceControl.vue, src/components/device/control/slvctrlplus/*
Added runtime validation guard for dynamic component imports; quote normalization across control components; minor layout/logic consistency updates without behavioral changes.
View & Layout Updates
src/views/*.vue
Standardized quote usage and introduced store references (automationStore, appStore, backendStore) in view components; minor initialization and message string updates.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • Add server url prompt on startup #25: Adds and wires backend store, BackendUrl route/component, and refactors app/socket initialization to use backend.backendUrl—directly overlaps with main PR's router and main.ts changes.
  • Unite attributes values & enable strict mode #35: Modifies device attribute/type system (src/model/devices/Device.ts) and utility helpers (src/utils/utils.ts)—shares code-level changes in attribute handling and type guards.
  • ZC95 support: Add support for new attribute properties #34: Modifies device control components, introduces debounced input components (DebouncedSlider/DebouncedTextField), and refactors device model/store types—overlaps with GenericDeviceControl and new DebouncedTextField component additions.

Suggested labels

chore, refactor

Poem

🐰 A rabbit's ode to single quotes

Single quotes now reign supreme,
The linter's code-style dream!
Prettier packed away with care,
TimeoutHelper's no longer there—
DebouncedText hops into place,
Making the codebase neat and embrace! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main changes: systematic quote style normalization to single quotes and the addition of a new DebouncedTextField component.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/fix-quotes
📝 Coding Plan
  • Generate coding plan for human review comments

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.

Tip

CodeRabbit can suggest fixes for GitHub Check annotations.

Configure the reviews.tools.github-checks setting to adjust the time to wait for GitHub Checks to complete.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 12

Caution

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

⚠️ Outside diff range comments (6)
src/components/automation/MonacoEditor.vue (1)

31-50: ⚠️ Potential issue | 🟠 Major

Leak risk: resize listener is never removed.

Line 31 registers a global listener, but there's no teardown on unmount. If this component remounts, listeners accumulate and can keep stale closures alive.

Proposed fix
+ import {defineAsyncComponent, watch, onMounted, onBeforeUnmount} from 'vue';
+
- window.addEventListener('resize', () => {
+ const onResize = () => {
   if (editorInstance === null) {
     return;
   }
   // make editor as small as possible
   editorInstance.layout({ width: 0, height: 0 });

   // wait for next frame to ensure last layout finished
   window.requestAnimationFrame(() => {
     const monacoElement = document.getElementById('monaco-wrapper');

     if (null === editorInstance || null === monacoElement) {
       return;
     }

     // get the parent dimensions and re-layout the editor
     const rect = monacoElement.getBoundingClientRect();
     editorInstance.layout({ width: rect.width, height: rect.height });
   });
- });
+ };
+
+ onMounted(() => window.addEventListener('resize', onResize));
+ onBeforeUnmount(() => window.removeEventListener('resize', onResize));
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/automation/MonacoEditor.vue` around lines 31 - 50, The resize
listener added with window.addEventListener(...) that references editorInstance
and the 'monaco-wrapper' element can leak because it is never removed; capture
the handler in a stable variable when you register it (rather than an inline
arrow) and remove it in the component teardown (beforeUnmount or unmounted) via
window.removeEventListener using the same function reference so the listener is
cleaned up and stale closures are not retained.
src/components/device/DeviceCard.vue (1)

23-46: ⚠️ Potential issue | 🟠 Major

Treat a closed popup handle as closed.

Line 24 only checks popup !== null, and the close listener is only attached after Line 41 fires. If the user closes the child window before load, isPopupOpen stays true and later clicks just focus a dead handle instead of reopening it.

Suggested fix
 const openInNewWindow = (device: Device): void => {
-  if (isPopupOpen.value === true && popup !== null) {
+  if (isPopupOpen.value === true && popup !== null && !popup.closed) {
     console.log(`popup is already open. Bring it into focus`);
     popup.focus();
     return;
   }
@@
   if (!popup) {
     return;
   }
 
+  popup.addEventListener('beforeunload', onPopupClose);
   popup.addEventListener('load', () => {
-    if (!popup) {
+    if (!popup || popup.closed) {
       return;
     }
     popup.document.title = `${document.title}: ${props.device.deviceName}`;
-    popup.addEventListener('beforeunload', () => onPopupClose());
   });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/device/DeviceCard.vue` around lines 23 - 46, openInNewWindow
currently treats a popup as open if popup !== null and isPopupOpen is true,
which fails if the user closed the child window before its load handler
attached; update openInNewWindow to treat a closed window as closed by checking
popup?.closed (use popup && popup.closed) before focusing and reopening, and
attach a close handler earlier (or poll/check) so onPopupClose is invoked when
popup.closed is true; adjust logic around the popup variable, isPopupOpen flag
and the beforeunload listener in openInNewWindow (and ensure onPopupClose clears
popup and sets isPopupOpen = false) so dead handles are detected and a new
window is opened.
src/components/automation/CreateForm.vue (1)

41-51: ⚠️ Potential issue | 🟠 Major

Return the refresh calls if they are async.

Lines 43–46 call fetchScripts() and fetchScript() without returning them. Since both are async functions, the next .then() executes before the store is refreshed, and any errors in those calls bypass the .catch().

Return or await both calls before emitting 'save':

Suggested fix
   automationStore
     .saveScript()
-    .then(() => {
-      automationStore.fetchScripts();
-      automationStore.fetchScript(scriptNameWithExt);
-    })
+    .then(() =>
+      Promise.all([
+        automationStore.fetchScripts(),
+        automationStore.fetchScript(scriptNameWithExt),
+      ])
+    )
     .then(() => {
       emit('save');
       newScriptName.value = '';
     })
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/automation/CreateForm.vue` around lines 41 - 51, The promise
chain calls automationStore.fetchScripts() and
automationStore.fetchScript(scriptNameWithExt) without returning them, so the
subsequent .then() runs before those async refreshes complete and their errors
bypass .catch(); modify the save flow in CreateForm.vue so that
automationStore.saveScript().then(() => { return
Promise.all([automationStore.fetchScripts(),
automationStore.fetchScript(scriptNameWithExt)]); }) (or use async/await) before
the .then() that calls emit('save') and clears newScriptName.value, ensuring
both fetchScripts and fetchScript are awaited and their errors propagate to the
existing .catch that calls appStore.displaySnackbar.
src/main.ts (1)

18-25: ⚠️ Potential issue | 🟠 Major

Socket initialization should be guarded when no backend URL is configured.

The socket plugin is initialized unconditionally, even when backend.backendUrl is empty or null. This causes a connection attempt to an invalid URL before the router redirects to /start. While App.vue guards event handlers with if (backendStore.backendUrl), the socket connection itself is still attempted.

Consider conditionally initializing the socket plugin:

🛡️ Proposed fix
 const app = createApp(App).use(createPinia());
 const backend = useBackendStore();

-app
-  .use(vueSocketIOClient, {
-    connection: `${backend.backendUrl}`,
-  })
-  .use(router)
+if (backend.backendUrl) {
+  app.use(vueSocketIOClient, {
+    connection: `${backend.backendUrl}`,
+  });
+}
+
+app
+  .use(router)
   .use(vuetify)
   .use(VueFullscreen)
   .mount('#app');
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main.ts` around lines 18 - 25, The socket plugin is initialized
unconditionally which attempts a connection when backend.backendUrl is empty;
wrap the vueSocketIOClient registration in a conditional that checks the
backend.backendUrl (e.g., if (backend.backendUrl) { app.use(vueSocketIOClient, {
connection: `${backend.backendUrl}` }) }) and leave the remaining
.use(router).use(vuetify).use(VueFullscreen).mount('#app') calls intact so the
socket is only registered/connected when a valid backend URL exists.
src/stores/health.ts (2)

97-137: ⚠️ Potential issue | 🟠 Major

Prevent duplicate timers when init() is called more than once.

init() creates two new intervals every call and never guards against re-entry. This can duplicate polling and data pushes over time.

Suggested guard for one-time initialization
+  let fetchIntervalId: number | undefined;
+  let chartIntervalId: number | undefined;
+
   function init(): void {
+    if (fetchIntervalId !== undefined || chartIntervalId !== undefined) {
+      return;
+    }
     const backendStore = useBackendStore();
 
-    setInterval(async () => {
+    fetchIntervalId = window.setInterval(async () => {
       if (!backendStore.isServerOnline) {
         return;
       }
@@
-    setInterval(() => {
+    chartIntervalId = window.setInterval(() => {
       if (!backendStore.isServerOnline || !chartData || !state.value) {
         return;
       }
@@
-    }, 1000);
+    }, 1000);
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/stores/health.ts` around lines 97 - 137, The init() function creates two
setInterval timers on every call, causing duplicated polling and chart updates;
add a re-entry guard and manage the timers: introduce a module-scoped boolean
like isHealthInitialized (or store it on useBackendStore) and return immediately
if true, or store the interval IDs from the two setInterval calls (e.g.,
healthPollIntervalId and chartUpdateIntervalId) and clear them before creating
new timers; ensure init() sets isHealthInitialized = true after creating timers
and provide a cleanup path (clearInterval and reset the flag) if needed.

100-107: ⚠️ Potential issue | 🟠 Major

Guard the poll loop against network/HTTP failures.

Line 105 and Line 106 run in an interval without try/catch and without response.ok checks. A failing /health call can produce repeated unhandled async errors.

Suggested hardening
-    setInterval(async () => {
+    setInterval(async () => {
       if (!backendStore.isServerOnline) {
         return;
       }
-
-      const response = await apiFetch(`/health`);
-      state.value = await response.json();
+      try {
+        const response = await apiFetch('/health');
+        if (!response.ok) {
+          return;
+        }
+        state.value = await response.json();
+      } catch {
+        // optional: keep previous value, or set state.value = undefined
+      }
     }, 3000);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/stores/health.ts` around lines 100 - 107, The polling interval started
with setInterval lacks error handling and response validation, which can cause
unhandled promise rejections when apiFetch(`/health`) fails or returns non-OK
responses; update the anonymous async callback used by setInterval (the block
referencing backendStore.isServerOnline, apiFetch, and state.value) to wrap the
fetch and JSON parsing in a try/catch, check response.ok before calling
response.json(), and handle/log non-OK responses or thrown errors (e.g., skip
updating state.value and log the error) so failures do not crash the interval
loop.
🧹 Nitpick comments (5)
src/components/automation/MonacoEditor.vue (1)

127-129: Prefer typed emits for payload safety.

Line 127/Line 129 use untyped emits, so payload type checks are weak. Typing defineEmits will catch invalid update:code payloads at compile time.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/automation/MonacoEditor.vue` around lines 127 - 129, The emits
are untyped which weakens payload checking; update the defineEmits call to
provide a typed signature for the 'update:code' event (e.g., declare that
'update:code' expects a string) and keep updateValue calling emit('update:code',
event) — this ensures compile-time validation of the payload for the defineEmits
symbol and the updateValue function.
src/model/devices/Device.ts (1)

22-28: Make FloatDeviceAttribute a real discriminated subtype.

Both IntDeviceAttribute and FloatDeviceAttribute inherit the same type: DeviceAttributeType union from DeviceAttribute, making them structurally identical. While type guards exist and check .type at runtime, TypeScript cannot narrow these interfaces properly as discriminated unions without literal type fields. Add type: 'float' to FloatDeviceAttribute and type: 'int' to IntDeviceAttribute to enable proper discriminated-union narrowing downstream.

Possible fix
 export interface IntDeviceAttribute<V extends number|undefined = number|undefined> extends DeviceAttribute<V> {
+   type: 'int';
    uom?: string;
 }
 
 export interface FloatDeviceAttribute<V extends number|undefined = number|undefined> extends DeviceAttribute<V> {
+   type: 'float';
    uom?: string;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/model/devices/Device.ts` around lines 22 - 28, IntDeviceAttribute and
FloatDeviceAttribute are currently structurally identical to DeviceAttribute
because they inherit a non-literal DeviceAttributeType; modify the interfaces so
they each declare a literal discriminant field (e.g. add type: 'int' to
IntDeviceAttribute and type: 'float' to FloatDeviceAttribute) while preserving
the generic V and uom?: string, so downstream code can narrow via the .type
property; ensure DeviceAttributeType (if used elsewhere) still accepts those
literals or update its union accordingly.
src/components/device/control/virtual/DeviceVirtualTtsControl.vue (1)

13-13: Don't cast away useSocketIO()'s nullability.

useSocketIO() is defined to return Socket | null, so Line 13 can still pass null into DeviceCommunicator if the provider is missing in tests or a future mount path. Guard here, or make the composable throw and return a non-null socket.

♻️ Suggested guard
-const io = useSocketIO() as Socket;
+const io = useSocketIO();
+if (!io) {
+  throw new Error('Socket.IO client is not available');
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/device/control/virtual/DeviceVirtualTtsControl.vue` at line
13, The code currently forces a non-null Socket by casting useSocketIO() to
Socket (const io = useSocketIO() as Socket), which can pass null into
DeviceCommunicator; instead remove the unsafe cast and explicitly guard the
result of useSocketIO() (or make useSocketIO() throw) — e.g., check if
useSocketIO() returns null and either throw a descriptive error or handle the
absence (early return, stub/fallback socket, or skip DeviceCommunicator
initialization) so DeviceCommunicator never receives a null socket; update
references to io and DeviceCommunicator initialization to use the
guarded/non-null value.
src/components/device/control/GenericDeviceControl.vue (1)

52-61: Redundant condition: isStringDeviceAttribute already covers float and int.

Looking at src/utils/utils.ts lines 31-34, isStringDeviceAttribute returns true for types 'str', 'float', and 'int'. The additional isFloatDeviceAttribute(attr) || isIntDeviceAttribute(attr) checks are redundant.

♻️ Simplify condition
         <DebouncedTextField
-          v-if="isStringDeviceAttribute(attr) || isFloatDeviceAttribute(attr) || isIntDeviceAttribute(attr)"
+          v-if="isStringDeviceAttribute(attr)"
           :model-value="attr.value"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/device/control/GenericDeviceControl.vue` around lines 52 - 61,
The v-if on the DebouncedTextField is redundant: replace the compound condition
with only isStringDeviceAttribute(attr) since isStringDeviceAttribute already
returns true for 'str', 'float', and 'int'; update the component usage
(DebouncedTextField) to use v-if="isStringDeviceAttribute(attr)" and keep the
existing bindings (model-value, type mapping, `@update`:modelValue calling
deviceComm.setAttribute, disabled, debounce-ms) unchanged.
src/stores/health.ts (1)

136-136: Align data push cadence with chart refresh cadence.

Line 136 now pushes points every 1000ms, while src/views/SystemHealthView.vue still configures stream refresh at 425ms (Lines 32-45 there). That causes repeated redraws of identical data and avoidable rendering overhead.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/stores/health.ts` at line 136, The health store's periodic data push uses
setInterval(..., 1000) which is out of sync with SystemHealthView.vue's stream
refresh at 425ms, causing redundant redraws; update the interval in the
setInterval call in src/stores/health.ts from 1000 to 425 to match the view (or
better, extract a shared constant like REFRESH_INTERVAL and use it both in the
health store's setInterval and in SystemHealthView.vue's stream refresh
configuration) so the push cadence and chart refresh cadence are identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/components/automation/CreateForm.vue`:
- Around line 24-25: The validation message returned by the script name
max-length check (the branch where value.length >= 64 in CreateForm.vue) has a
typo; update the string returned from "The script name needs to no more than 64
characters long." to "The script name needs to be no more than 64 characters
long." so the message reads correctly when the validation fails.

In `@src/components/device/control/DeviceEstim2bControl.vue`:
- Line 7: The import line in DeviceEstim2bControl.vue is inconsistent: change
"import {isIntRangeDeviceAttribute, typedEntries} from '@/utils/utils';" to
include spaces inside the braces so it matches the file's import style; locate
the import that references isIntRangeDeviceAttribute and typedEntries and update
it to use spaced braces (e.g., "import { isIntRangeDeviceAttribute, typedEntries
} from '@/utils/utils';") to restore consistent formatting.

In `@src/components/device/DebouncedTextField.vue`:
- Around line 23-24: The component uses a module-level debounceTimer and doesn't
clear pending timers, which lets a delayed blur flip isActive and desync
localValue after refocus; change to an instance-scoped timer (e.g., a ref or
property named debounceTimer) and always clearTimeout/debounceTimer before
scheduling a new timer and in onFocus/onBlur handlers to cancel stale callbacks,
and also clear it in the component's beforeUnmount/unmounted hook so no
post-unmount emits occur; update all places referencing debounceTimer (the blur
scheduling code and any handlers that set isActive/localValue) to use the
instance-scoped timer and centralize timer-clear logic.
- Around line 45-53: Normalize and validate props.debounceMs before scheduling:
convert to a Number, treat non-finite or NaN as 0 and clamp negative values to 0
(e.g. const delay = Number(props.debounceMs); const safeDelay =
Number.isFinite(delay) ? Math.max(0, delay) : 0), then use safeDelay in the
conditional and in window.setTimeout (call emitValue immediately when safeDelay
=== 0, otherwise schedule with safeDelay). Reference symbols: props.debounceMs,
emitValue, debounceTimer.

In `@src/helper/DeviceCommunicator.ts`:
- Around line 25-28: Reintroduce temporary suppression of self-echo processing
when emitting outbound updates: in DeviceCommunicator (the code path that calls
this.io.emit('deviceUpdate', ...), likely setAttribute), set
this.device.receiveUpdates = false immediately before calling this.io.emit and
restore it to true in a finally block after the emit so the flag is always
re-enabled even on errors; use the existing this.device.receiveUpdates flag to
match src/stores/devices.ts gating logic and ensure suppression scope is limited
to this.device to prevent processing the echoed update by the same device.

In `@src/plugins/vuetify.ts`:
- Around line 12-17: The theme object named "dark" incorrectly uses the property
"dark2: true" which prevents Vuetify from treating it as a dark theme; change
the property name from "dark2" to "dark" (i.e., set dark: true) in the "dark"
theme object so Vuetify's theme API recognizes it (look for the "dark" object
and the "dark2" property to update).

In `@src/stores/devices.ts`:
- Around line 50-54: The clear() function currently deletes entries from the
devices map but doesn't update the devicesLoaded flag, leaving it true and
causing consumers to think devices are present; update clear() to also set
devicesLoaded = false so the state accurately reflects that devices are cleared
(ensure you modify the same clear() function in src/stores/devices.ts and
coordinate with init()/load logic that sets devicesLoaded = true when devices
are refetched).

In `@src/utils/utils.ts`:
- Around line 31-33: The type guard isStringDeviceAttribute currently returns
true for 'float' and 'int' due to hasType checks, which contradicts its declared
narrowing to StrDeviceAttribute; change the predicate in isStringDeviceAttribute
to only test hasType(obj, 'str') so the runtime check matches the type guard and
only string attributes are accepted (update the function isStringDeviceAttribute
and its use of hasType accordingly).

In `@src/views/SettingsView.vue`:
- Around line 73-75: Fix the typo in the error snackbar message produced in the
save settings promise chain: in the catch handler attached to
settingsStore.saveServerSettings(), update the string passed to
appStore.displaySnackbar from "Sever settings could not be saved:
${err.message}" to "Server settings could not be saved: ${err.message}" so the
user-facing message displays "Server" correctly; verify the change in the catch
block where appStore.displaySnackbar is called.
- Line 2: The resize listener added in storeEditorInstance() is registered as an
anonymous handler and never removed, causing stale listeners to accumulate and
call editor.layout() on old instances; fix by assigning the handler to a scoped
variable (e.g., const onResize = () => editor.layout()) when calling
window.addEventListener('resize', onResize) inside storeEditorInstance(), and
then remove it in onBeforeUnmount (or before re-registering) via
window.removeEventListener('resize', onResize); ensure the same handler
reference is used and tie removal to the component lifecycle so layout() is not
invoked for unmounted editor instances.

In `@src/vueSocketIOClient.d.ts`:
- Around line 1-5: Change the $socket declaration in src/vueSocketIOClient.d.ts
to use the Socket instance type (not the constructor) and import it only as a
type: replace "import { Socket } from 'socket.io-client';" with a type-only
import and change ComponentCustomProperties.$socket from "typeof Socket" to
"Socket" so it matches the instance assigned by io(...) and the declaration in
src/shims-vue.d.ts; ensure the module augmentation uses the type import (e.g.,
import type { Socket } from 'socket.io-client') to avoid importing a runtime
value.

In `@vue.config.js`:
- Line 1: The file mixes ESM import (defineConfig) with a CommonJS export;
replace the module.exports usage with an ESM default export so the file is pure
ESM: remove any "module.exports = ..." and instead export the configuration via
"export default" using the existing defineConfig(...) call (refer to
defineConfig and the current export site in vue.config.js) so the module shape
matches the package's "type": "module".

---

Outside diff comments:
In `@src/components/automation/CreateForm.vue`:
- Around line 41-51: The promise chain calls automationStore.fetchScripts() and
automationStore.fetchScript(scriptNameWithExt) without returning them, so the
subsequent .then() runs before those async refreshes complete and their errors
bypass .catch(); modify the save flow in CreateForm.vue so that
automationStore.saveScript().then(() => { return
Promise.all([automationStore.fetchScripts(),
automationStore.fetchScript(scriptNameWithExt)]); }) (or use async/await) before
the .then() that calls emit('save') and clears newScriptName.value, ensuring
both fetchScripts and fetchScript are awaited and their errors propagate to the
existing .catch that calls appStore.displaySnackbar.

In `@src/components/automation/MonacoEditor.vue`:
- Around line 31-50: The resize listener added with window.addEventListener(...)
that references editorInstance and the 'monaco-wrapper' element can leak because
it is never removed; capture the handler in a stable variable when you register
it (rather than an inline arrow) and remove it in the component teardown
(beforeUnmount or unmounted) via window.removeEventListener using the same
function reference so the listener is cleaned up and stale closures are not
retained.

In `@src/components/device/DeviceCard.vue`:
- Around line 23-46: openInNewWindow currently treats a popup as open if popup
!== null and isPopupOpen is true, which fails if the user closed the child
window before its load handler attached; update openInNewWindow to treat a
closed window as closed by checking popup?.closed (use popup && popup.closed)
before focusing and reopening, and attach a close handler earlier (or
poll/check) so onPopupClose is invoked when popup.closed is true; adjust logic
around the popup variable, isPopupOpen flag and the beforeunload listener in
openInNewWindow (and ensure onPopupClose clears popup and sets isPopupOpen =
false) so dead handles are detected and a new window is opened.

In `@src/main.ts`:
- Around line 18-25: The socket plugin is initialized unconditionally which
attempts a connection when backend.backendUrl is empty; wrap the
vueSocketIOClient registration in a conditional that checks the
backend.backendUrl (e.g., if (backend.backendUrl) { app.use(vueSocketIOClient, {
connection: `${backend.backendUrl}` }) }) and leave the remaining
.use(router).use(vuetify).use(VueFullscreen).mount('#app') calls intact so the
socket is only registered/connected when a valid backend URL exists.

In `@src/stores/health.ts`:
- Around line 97-137: The init() function creates two setInterval timers on
every call, causing duplicated polling and chart updates; add a re-entry guard
and manage the timers: introduce a module-scoped boolean like
isHealthInitialized (or store it on useBackendStore) and return immediately if
true, or store the interval IDs from the two setInterval calls (e.g.,
healthPollIntervalId and chartUpdateIntervalId) and clear them before creating
new timers; ensure init() sets isHealthInitialized = true after creating timers
and provide a cleanup path (clearInterval and reset the flag) if needed.
- Around line 100-107: The polling interval started with setInterval lacks error
handling and response validation, which can cause unhandled promise rejections
when apiFetch(`/health`) fails or returns non-OK responses; update the anonymous
async callback used by setInterval (the block referencing
backendStore.isServerOnline, apiFetch, and state.value) to wrap the fetch and
JSON parsing in a try/catch, check response.ok before calling response.json(),
and handle/log non-OK responses or thrown errors (e.g., skip updating
state.value and log the error) so failures do not crash the interval loop.

---

Nitpick comments:
In `@src/components/automation/MonacoEditor.vue`:
- Around line 127-129: The emits are untyped which weakens payload checking;
update the defineEmits call to provide a typed signature for the 'update:code'
event (e.g., declare that 'update:code' expects a string) and keep updateValue
calling emit('update:code', event) — this ensures compile-time validation of the
payload for the defineEmits symbol and the updateValue function.

In `@src/components/device/control/GenericDeviceControl.vue`:
- Around line 52-61: The v-if on the DebouncedTextField is redundant: replace
the compound condition with only isStringDeviceAttribute(attr) since
isStringDeviceAttribute already returns true for 'str', 'float', and 'int';
update the component usage (DebouncedTextField) to use
v-if="isStringDeviceAttribute(attr)" and keep the existing bindings
(model-value, type mapping, `@update`:modelValue calling deviceComm.setAttribute,
disabled, debounce-ms) unchanged.

In `@src/components/device/control/virtual/DeviceVirtualTtsControl.vue`:
- Line 13: The code currently forces a non-null Socket by casting useSocketIO()
to Socket (const io = useSocketIO() as Socket), which can pass null into
DeviceCommunicator; instead remove the unsafe cast and explicitly guard the
result of useSocketIO() (or make useSocketIO() throw) — e.g., check if
useSocketIO() returns null and either throw a descriptive error or handle the
absence (early return, stub/fallback socket, or skip DeviceCommunicator
initialization) so DeviceCommunicator never receives a null socket; update
references to io and DeviceCommunicator initialization to use the
guarded/non-null value.

In `@src/model/devices/Device.ts`:
- Around line 22-28: IntDeviceAttribute and FloatDeviceAttribute are currently
structurally identical to DeviceAttribute because they inherit a non-literal
DeviceAttributeType; modify the interfaces so they each declare a literal
discriminant field (e.g. add type: 'int' to IntDeviceAttribute and type: 'float'
to FloatDeviceAttribute) while preserving the generic V and uom?: string, so
downstream code can narrow via the .type property; ensure DeviceAttributeType
(if used elsewhere) still accepts those literals or update its union
accordingly.

In `@src/stores/health.ts`:
- Line 136: The health store's periodic data push uses setInterval(..., 1000)
which is out of sync with SystemHealthView.vue's stream refresh at 425ms,
causing redundant redraws; update the interval in the setInterval call in
src/stores/health.ts from 1000 to 425 to match the view (or better, extract a
shared constant like REFRESH_INTERVAL and use it both in the health store's
setInterval and in SystemHealthView.vue's stream refresh configuration) so the
push cadence and chart refresh cadence are identical.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 335aba3d-49ac-4540-9d03-13e577726a08

📥 Commits

Reviewing files that changed from the base of the PR and between 65f97a0 and 61d405e.

📒 Files selected for processing (73)
  • cypress/integration/example.spec.ts
  • cypress/support/index.ts
  • eslint.config.ts
  • package.json
  • src/App.vue
  • src/components/DeviceInfo.vue
  • src/components/ServerStatusOverlay.vue
  • src/components/__tests__/HelloWorld.spec.ts
  • src/components/automation/CreateForm.vue
  • src/components/automation/LogViewer.vue
  • src/components/automation/MonacoEditor.vue
  • src/components/chart/StreamLineChart.vue
  • src/components/device/DebouncedSlider.vue
  • src/components/device/DebouncedTextField.vue
  • src/components/device/DeviceCard.vue
  • src/components/device/DeviceControl.vue
  • src/components/device/control/DeviceEstim2bControl.vue
  • src/components/device/control/DeviceZc95Control.vue
  • src/components/device/control/GenericDeviceControl.vue
  • src/components/device/control/slvctrlplus/DeviceAirValveControl.vue
  • src/components/device/control/slvctrlplus/DeviceDisplayControl.vue
  • src/components/device/control/slvctrlplus/DeviceDistanceControl.vue
  • src/components/device/control/slvctrlplus/DeviceEt312Control.vue
  • src/components/device/control/slvctrlplus/DeviceNogasmControl.vue
  • src/components/device/control/slvctrlplus/DeviceStrikerMk2Control.vue
  • src/components/device/control/virtual/DeviceVirtualDisplayControl.vue
  • src/components/device/control/virtual/DeviceVirtualPiperTtsControl.vue
  • src/components/device/control/virtual/DeviceVirtualRandomGeneratorControl.vue
  • src/components/device/control/virtual/DeviceVirtualTtsControl.vue
  • src/components/icons/DeviceIcon.vue
  • src/helper/ChartHelper.ts
  • src/helper/DeviceCommunicator.ts
  • src/helper/TimeoutHelper.ts
  • src/layouts/LayoutWithMenu.vue
  • src/main.ts
  • src/model/MapRule.ts
  • src/model/devices/Device.ts
  • src/model/devices/estim2b/DeviceEstim2b.ts
  • src/model/devices/slvctrl/DeviceAirValve.ts
  • src/model/devices/slvctrl/DeviceDisplay.ts
  • src/model/devices/slvctrl/DeviceDistance.ts
  • src/model/devices/slvctrl/DeviceEt312.ts
  • src/model/devices/slvctrl/DeviceNogasm.ts
  • src/model/devices/slvctrl/DeviceStrikerMk2.ts
  • src/model/devices/virtual/VirtualDeviceDisplay.ts
  • src/model/devices/virtual/VirtualDevicePiperTts.ts
  • src/model/devices/virtual/VirtualDeviceTts.ts
  • src/model/devices/virtual/VirtualRandomGenerator.ts
  • src/model/devices/zc95/DeviceZc95.ts
  • src/plugins/vueSocketIOClient.ts
  • src/plugins/vuetify.ts
  • src/plugins/webfontloader.ts
  • src/router/index.ts
  • src/shims-vue.d.ts
  • src/shims-vuetify.d.ts
  • src/stores/app.ts
  • src/stores/automation.ts
  • src/stores/backend.ts
  • src/stores/devices.ts
  • src/stores/health.ts
  • src/stores/settings.ts
  • src/utils/apiFetch.ts
  • src/utils/utils.ts
  • src/views/AutomationView.vue
  • src/views/BackendUrl.vue
  • src/views/DevicesView.vue
  • src/views/MissionControlSingleView.vue
  • src/views/MissionControlView.vue
  • src/views/SettingsView.vue
  • src/views/SystemHealthView.vue
  • src/vueSocketIOClient.d.ts
  • vite.config.ts
  • vue.config.js
💤 Files with no reviewable changes (1)
  • src/helper/TimeoutHelper.ts

Comment on lines 24 to +25
if (value.length < 64) return true;
return "The script name needs to no more than 64 characters long.";
return 'The script name needs to no more than 64 characters long.';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix the max-length validation copy.

Line 25 says "needs to no more than 64 characters long." That should read "needs to be no more than 64 characters long."

Suggested fix
-    return 'The script name needs to no more than 64 characters long.';
+    return 'The script name needs to be no more than 64 characters long.';
📝 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.

Suggested change
if (value.length < 64) return true;
return "The script name needs to no more than 64 characters long.";
return 'The script name needs to no more than 64 characters long.';
if (value.length < 64) return true;
return 'The script name needs to be no more than 64 characters long.';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/automation/CreateForm.vue` around lines 24 - 25, The
validation message returned by the script name max-length check (the branch
where value.length >= 64 in CreateForm.vue) has a typo; update the string
returned from "The script name needs to no more than 64 characters long." to
"The script name needs to be no more than 64 characters long." so the message
reads correctly when the validation fails.

import type { Socket } from 'socket.io-client';
import DeviceCommunicator from '@/helper/DeviceCommunicator';
import DebouncedSlider from '@/components/device/DebouncedSlider.vue';
import {isIntRangeDeviceAttribute, typedEntries} from '@/utils/utils';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix spacing inconsistency in import statement.

The import statement is missing spaces inside the braces, which is inconsistent with other imports in this file (lines 2, 4). Since Prettier has been removed from the project, maintaining consistent formatting manually is important.

✨ Proposed fix for consistent spacing
-import {isIntRangeDeviceAttribute, typedEntries} from '@/utils/utils';
+import { isIntRangeDeviceAttribute, typedEntries } from '@/utils/utils';
📝 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.

Suggested change
import {isIntRangeDeviceAttribute, typedEntries} from '@/utils/utils';
import { isIntRangeDeviceAttribute, typedEntries } from '@/utils/utils';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/device/control/DeviceEstim2bControl.vue` at line 7, The import
line in DeviceEstim2bControl.vue is inconsistent: change "import
{isIntRangeDeviceAttribute, typedEntries} from '@/utils/utils';" to include
spaces inside the braces so it matches the file's import style; locate the
import that references isIntRangeDeviceAttribute and typedEntries and update it
to use spaced braces (e.g., "import { isIntRangeDeviceAttribute, typedEntries }
from '@/utils/utils';") to restore consistent formatting.

Comment on lines +23 to +24
let debounceTimer: number | undefined;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Track and clear all timers to avoid stale state and post-unmount emits.

There is no cleanup for pending timers, and the delayed blur callback can flip isActive to false after refocus. That can desync localValue while editing.

Suggested timer lifecycle fix
- import { ref, watch } from 'vue';
+ import { onBeforeUnmount, ref, watch } from 'vue';
@@
-let debounceTimer: number | undefined;
+let debounceTimer: ReturnType<typeof window.setTimeout> | undefined;
+let blurTimer: ReturnType<typeof window.setTimeout> | undefined;
@@
 const handleValueChange = (value: string | number): void => {
   localValue.value = value;
 
-  if (debounceTimer) {
+  if (debounceTimer !== undefined) {
     clearTimeout(debounceTimer);
+    debounceTimer = undefined;
   }
@@
 const handleStart = (): void => {
+  if (blurTimer !== undefined) {
+    clearTimeout(blurTimer);
+    blurTimer = undefined;
+  }
   isActive.value = true;
 };
 
 const handleEnd = (): void => {
-  setTimeout(() => {
+  blurTimer = window.setTimeout(() => {
     isActive.value = false;
+    blurTimer = undefined;
   }, 100);
 };
+
+onBeforeUnmount(() => {
+  if (debounceTimer !== undefined) clearTimeout(debounceTimer);
+  if (blurTimer !== undefined) clearTimeout(blurTimer);
+});

Also applies to: 41-43, 50-53, 56-64

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/device/DebouncedTextField.vue` around lines 23 - 24, The
component uses a module-level debounceTimer and doesn't clear pending timers,
which lets a delayed blur flip isActive and desync localValue after refocus;
change to an instance-scoped timer (e.g., a ref or property named debounceTimer)
and always clearTimeout/debounceTimer before scheduling a new timer and in
onFocus/onBlur handlers to cancel stale callbacks, and also clear it in the
component's beforeUnmount/unmounted hook so no post-unmount emits occur; update
all places referencing debounceTimer (the blur scheduling code and any handlers
that set isActive/localValue) to use the instance-scoped timer and centralize
timer-clear logic.

Comment on lines +45 to +53
if (props.debounceMs === 0) {
emitValue(value);
return;
}

debounceTimer = window.setTimeout(() => {
emitValue(value);
debounceTimer = undefined;
}, props.debounceMs);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Validate debounce delay before scheduling.

debounceMs accepts any number; negative or non-finite values should be normalized to prevent surprising behavior.

Suggested normalization
-  if (props.debounceMs === 0) {
+  const delay = Number.isFinite(props.debounceMs) ? Math.max(0, props.debounceMs) : 0;
+  if (delay === 0) {
     emitValue(value);
     return;
   }
 
   debounceTimer = window.setTimeout(() => {
     emitValue(value);
     debounceTimer = undefined;
-  }, props.debounceMs);
+  }, delay);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/device/DebouncedTextField.vue` around lines 45 - 53, Normalize
and validate props.debounceMs before scheduling: convert to a Number, treat
non-finite or NaN as 0 and clamp negative values to 0 (e.g. const delay =
Number(props.debounceMs); const safeDelay = Number.isFinite(delay) ? Math.max(0,
delay) : 0), then use safeDelay in the conditional and in window.setTimeout
(call emitValue immediately when safeDelay === 0, otherwise schedule with
safeDelay). Reference symbols: props.debounceMs, emitValue, debounceTimer.

Comment on lines +25 to 28
this.io.emit('deviceUpdate', {
deviceId: this.device.deviceId,
data: { [attrName]: newValue },
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Reintroduce outbound-update suppression to avoid self-echo processing.

setAttribute now emits without temporarily disabling device.receiveUpdates. Given src/stores/devices.ts (Line 37-41) gates updates on that flag, removing suppression here can let a device process its own outbound update when echoed back, causing duplicate updates/loops.

Suggested fix
 export default class DeviceCommunicator<T extends Device> {
   private readonly io: Socket;
 
   private readonly device: T;
+  private receiveUpdatesResetTimer?: ReturnType<typeof setTimeout>;
@@
   public setAttribute<K extends keyof T['attributes']>(attrName: K, newValue: DeviceData<T>[K] | null): void {
@@
-    this.io.emit('deviceUpdate', {
+    this.device.receiveUpdates = false;
+    this.io.emit('deviceUpdate', {
       deviceId: this.device.deviceId,
       data: { [attrName]: newValue },
     });
+    if (this.receiveUpdatesResetTimer) {
+      clearTimeout(this.receiveUpdatesResetTimer);
+    }
+    this.receiveUpdatesResetTimer = setTimeout(() => {
+      this.device.receiveUpdates = true;
+    }, 200);
   }
 }
📝 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.

Suggested change
this.io.emit('deviceUpdate', {
deviceId: this.device.deviceId,
data: { [attrName]: newValue },
});
export default class DeviceCommunicator<T extends Device> {
private readonly io: Socket;
private readonly device: T;
private receiveUpdatesResetTimer?: ReturnType<typeof setTimeout>;
public setAttribute<K extends keyof T['attributes']>(attrName: K, newValue: DeviceData<T>[K] | null): void {
this.device.receiveUpdates = false;
this.io.emit('deviceUpdate', {
deviceId: this.device.deviceId,
data: { [attrName]: newValue },
});
if (this.receiveUpdatesResetTimer) {
clearTimeout(this.receiveUpdatesResetTimer);
}
this.receiveUpdatesResetTimer = setTimeout(() => {
this.device.receiveUpdates = true;
}, 200);
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/helper/DeviceCommunicator.ts` around lines 25 - 28, Reintroduce temporary
suppression of self-echo processing when emitting outbound updates: in
DeviceCommunicator (the code path that calls this.io.emit('deviceUpdate', ...),
likely setAttribute), set this.device.receiveUpdates = false immediately before
calling this.io.emit and restore it to true in a finally block after the emit so
the flag is always re-enabled even on errors; use the existing
this.device.receiveUpdates flag to match src/stores/devices.ts gating logic and
ensure suppression scope is limited to this.device to prevent processing the
echoed update by the same device.

Comment thread src/utils/utils.ts
Comment on lines +31 to +33
export const isStringDeviceAttribute = (obj: object): obj is StrDeviceAttribute => {
return hasType(obj, 'str') || hasType(obj, 'float') || hasType(obj, 'int');
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n src/utils/utils.ts | head -50

Repository: SlvCtrlPlus/slvctrlplus-frontend

Length of output: 2402


🏁 Script executed:

rg "type StrDeviceAttribute|interface StrDeviceAttribute" -A 3

Repository: SlvCtrlPlus/slvctrlplus-frontend

Length of output: 392


🏁 Script executed:

rg "isStringDeviceAttribute" -B 2 -A 2

Repository: SlvCtrlPlus/slvctrlplus-frontend

Length of output: 1464


🏁 Script executed:

cat -n src/components/device/control/GenericDeviceControl.vue | sed -n '50,65p'

Repository: SlvCtrlPlus/slvctrlplus-frontend

Length of output: 772


isStringDeviceAttribute() type guard does not match its implementation.

The function returns true for 'float' and 'int' attributes (line 32), but its type guard signature claims to narrow to StrDeviceAttribute, which is constrained to string values only. This violates the type-guard contract and causes misleading type narrowing. The fix is to check only for 'str':

🔧 Fix
 export const isStringDeviceAttribute = (obj: object): obj is StrDeviceAttribute => {
-    return hasType(obj, 'str') || hasType(obj, 'float') || hasType(obj, 'int');
+    return hasType(obj, 'str');
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/utils.ts` around lines 31 - 33, The type guard
isStringDeviceAttribute currently returns true for 'float' and 'int' due to
hasType checks, which contradicts its declared narrowing to StrDeviceAttribute;
change the predicate in isStringDeviceAttribute to only test hasType(obj, 'str')
so the runtime check matches the type guard and only string attributes are
accepted (update the function isStringDeviceAttribute and its use of hasType
accordingly).

import { storeToRefs } from "pinia";
import type * as monaco from "monaco-editor/esm/vs/editor/editor.api";
import {useBackendStore} from "@/stores/backend";
import {defineAsyncComponent, nextTick, ref, watch} from 'vue';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cd src/views && wc -l SettingsView.vue

Repository: SlvCtrlPlus/slvctrlplus-frontend

Length of output: 96


🏁 Script executed:

cat -n src/views/SettingsView.vue | head -80

Repository: SlvCtrlPlus/slvctrlplus-frontend

Length of output: 3166


🏁 Script executed:

cat -n src/views/SettingsView.vue | tail -110

Repository: SlvCtrlPlus/slvctrlplus-frontend

Length of output: 4755


🏁 Script executed:

# Check if onBeforeUnmount is used elsewhere in the codebase for reference
rg "onBeforeUnmount" src/ --type ts --type vue -B 2 -A 5

Repository: SlvCtrlPlus/slvctrlplus-frontend

Length of output: 103


🏁 Script executed:

rg "onBeforeUnmount" src/ -B 2 -A 5

Repository: SlvCtrlPlus/slvctrlplus-frontend

Length of output: 981


Remove the resize event listener during component unmount.

The anonymous resize handler added at line 52 is never unregistered. When navigating away from the tab containing this editor and returning, storeEditorInstance() is called again on remount, adding another listener without removing the previous one. Stale listeners accumulate and continue calling layout() on old editor instances on every window resize event, causing memory leaks and unnecessary layout computations.

Store the handler in a variable and remove it using onBeforeUnmount:

🔧 Suggested fix
-import {defineAsyncComponent, nextTick, ref, watch} from 'vue';
+import {defineAsyncComponent, nextTick, onBeforeUnmount, ref, watch} from 'vue';
@@
 let editorInstance: monaco.editor.IStandaloneCodeEditor | null = null;
+let resizeHandler: (() => void) | null = null;
@@
 function storeEditorInstance(
   editor: monaco.editor.IStandaloneCodeEditor
 ): void {
   const container = document.getElementById('monaco-wrapper');
 
   if (container) {
     const rect = container.getBoundingClientRect();
     editor.layout({ width: rect.width, height: rect.height });
   }
 
-  window.addEventListener('resize', () => {
+  if (resizeHandler) {
+    window.removeEventListener('resize', resizeHandler);
+  }
+
+  resizeHandler = () => {
     // make editor as small as possible
     editor.layout({ width: 0, height: 0 });
 
     // wait for next frame to ensure last layout finished
     window.requestAnimationFrame(() => {
       const monacoElement = document.getElementById('monaco-wrapper');
 
       if (null === editor || null === monacoElement) {
         return;
       }
 
       // get the parent dimensions and re-layout the editor
       const rect = monacoElement.getBoundingClientRect();
       editor.layout({ width: rect.width, height: rect.height });
     });
-  });
+  };
+
+  window.addEventListener('resize', resizeHandler);
   editorInstance = editor;
 }
+
+onBeforeUnmount(() => {
+  if (resizeHandler) {
+    window.removeEventListener('resize', resizeHandler);
+  }
+});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/views/SettingsView.vue` at line 2, The resize listener added in
storeEditorInstance() is registered as an anonymous handler and never removed,
causing stale listeners to accumulate and call editor.layout() on old instances;
fix by assigning the handler to a scoped variable (e.g., const onResize = () =>
editor.layout()) when calling window.addEventListener('resize', onResize) inside
storeEditorInstance(), and then remove it in onBeforeUnmount (or before
re-registering) via window.removeEventListener('resize', onResize); ensure the
same handler reference is used and tie removal to the component lifecycle so
layout() is not invoked for unmounted editor instances.

Comment on lines 73 to +75
settingsStore.saveServerSettings()
.then(() => appStore.displaySnackbar(`Server settings saved`))
.catch((err: Error) => appStore.displaySnackbar(`Sever settings could not be saved: ${err.message}`, "red"));
.catch((err: Error) => appStore.displaySnackbar(`Sever settings could not be saved: ${err.message}`, 'red'));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix the user-facing typo in the error snackbar.

Line 75 currently says Sever settings could not be saved.

✏️ Minimal fix
-      .catch((err: Error) => appStore.displaySnackbar(`Sever settings could not be saved: ${err.message}`, 'red'));
+      .catch((err: Error) => appStore.displaySnackbar(`Server settings could not be saved: ${err.message}`, 'red'));
📝 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.

Suggested change
settingsStore.saveServerSettings()
.then(() => appStore.displaySnackbar(`Server settings saved`))
.catch((err: Error) => appStore.displaySnackbar(`Sever settings could not be saved: ${err.message}`, "red"));
.catch((err: Error) => appStore.displaySnackbar(`Sever settings could not be saved: ${err.message}`, 'red'));
settingsStore.saveServerSettings()
.then(() => appStore.displaySnackbar(`Server settings saved`))
.catch((err: Error) => appStore.displaySnackbar(`Server settings could not be saved: ${err.message}`, 'red'));
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/views/SettingsView.vue` around lines 73 - 75, Fix the typo in the error
snackbar message produced in the save settings promise chain: in the catch
handler attached to settingsStore.saveServerSettings(), update the string passed
to appStore.displaySnackbar from "Sever settings could not be saved:
${err.message}" to "Server settings could not be saved: ${err.message}" so the
user-facing message displays "Server" correctly; verify the change in the catch
block where appStore.displaySnackbar is called.

Comment on lines +1 to 5
import { Socket } from 'socket.io-client';

declare module "@vue/runtime-core" {
declare module '@vue/runtime-core' {
interface ComponentCustomProperties {
$socket: typeof Socket;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== ComponentCustomProperties socket typings =="
rg -n -C2 "\$socket\s*:" src/shims-vue.d.ts src/vueSocketIOClient.d.ts

echo
echo "== Runtime assignment sites for \$socket (if any) =="
rg -n -C2 "globalProperties\.\$socket|\$socket\s*=|provide\(\s*['\"]socket['\"]" src/plugins/vueSocketIOClient.ts src/helper/DeviceCommunicator.ts

Repository: SlvCtrlPlus/slvctrlplus-frontend

Length of output: 122


🏁 Script executed:

git ls-files | grep -E '\.(d\.ts|ts)$' | head -30

Repository: SlvCtrlPlus/slvctrlplus-frontend

Length of output: 1073


🏁 Script executed:

find src -name "*.d.ts" -o -name "*shims*" -o -name "*vueSocketIO*" 2>/dev/null

Repository: SlvCtrlPlus/slvctrlplus-frontend

Length of output: 192


🏁 Script executed:

ls -la src/ 2>/dev/null | head -20

Repository: SlvCtrlPlus/slvctrlplus-frontend

Length of output: 1146


🏁 Script executed:

cat -n src/vueSocketIOClient.d.ts

Repository: SlvCtrlPlus/slvctrlplus-frontend

Length of output: 279


🏁 Script executed:

cat -n src/shims-vue.d.ts

Repository: SlvCtrlPlus/slvctrlplus-frontend

Length of output: 345


🏁 Script executed:

cat -n src/plugins/vueSocketIOClient.ts

Repository: SlvCtrlPlus/slvctrlplus-frontend

Length of output: 1083


Fix type conflict in $socket declaration—use instance type instead of constructor type.

The $socket property is declared with conflicting types across two files. In src/vueSocketIOClient.d.ts line 5, it's typed as typeof Socket (the constructor), but src/shims-vue.d.ts line 7 declares it as Socket (the instance). At runtime (line 26 of src/plugins/vueSocketIOClient.ts), $socket is assigned the result of io(...), which is a Socket instance, not the constructor. This type mismatch breaks module augmentation and causes TypeScript declaration conflicts.

Additionally, src/vueSocketIOClient.d.ts imports Socket as a value when it's only used as a type.

Proposed fix
-import { Socket } from 'socket.io-client';
+import type { Socket } from 'socket.io-client';

 declare module '@vue/runtime-core' {
   interface ComponentCustomProperties {
-    $socket: typeof Socket;
+    $socket: Socket;
   }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/vueSocketIOClient.d.ts` around lines 1 - 5, Change the $socket
declaration in src/vueSocketIOClient.d.ts to use the Socket instance type (not
the constructor) and import it only as a type: replace "import { Socket } from
'socket.io-client';" with a type-only import and change
ComponentCustomProperties.$socket from "typeof Socket" to "Socket" so it matches
the instance assigned by io(...) and the declaration in src/shims-vue.d.ts;
ensure the module augmentation uses the type import (e.g., import type { Socket
} from 'socket.io-client') to avoid importing a runtime value.

Comment thread vue.config.js
@@ -1,4 +1,4 @@
import { defineConfig } from "@vue/cli-service";
import { defineConfig } from '@vue/cli-service';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

fd '^vue\.config\.js$' -x sh -c '
  echo "--- $1 ---"
  sed -n "1,20p" "$1"
' sh {}

echo
fd '^package\.json$' -x sh -c '
  echo ">>> $1"
  rg -n "\"type\"\\s*:\\s*\"module\"" "$1" || true
' sh {}

Repository: SlvCtrlPlus/slvctrlplus-frontend

Length of output: 359


Convert vue.config.js to pure ESM since the package declares "type": "module".

Line 1 uses ESM import syntax, but the file exports via module.exports on line 2. Since package.json declares "type": "module", the entire package is ESM-configured. Mixing ESM and CommonJS in the same file violates Node.js module rules and will prevent Vue CLI from loading the config. Convert the export to export default to match the ESM context.

Fix
 import { defineConfig } from '@vue/cli-service';
-module.exports = defineConfig({
+export default defineConfig({
   pluginOptions: {
     vuetify: {
       // https://github.com/vuetifyjs/vuetify-loader/tree/next/packages/vuetify-loader
     },
   },
-});
+});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@vue.config.js` at line 1, The file mixes ESM import (defineConfig) with a
CommonJS export; replace the module.exports usage with an ESM default export so
the file is pure ESM: remove any "module.exports = ..." and instead export the
configuration via "export default" using the existing defineConfig(...) call
(refer to defineConfig and the current export site in vue.config.js) so the
module shape matches the package's "type": "module".

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.

1 participant