refactor: rework the gui/gcodehistory store #2424
Open
meteyou wants to merge 78 commits intomainsail-crew:developfrom
Open
refactor: rework the gui/gcodehistory store #2424meteyou wants to merge 78 commits intomainsail-crew:developfrom
meteyou wants to merge 78 commits intomainsail-crew:developfrom
Conversation
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
…/init-server-process # Conflicts: # src/plugins/webSocketClient.ts
Signed-off-by: Stefan Dej <meteyou@gmail.com>
…nraker-types-emitAndWait
…/init-server-process # Conflicts: # src/components/dialogs/SpoolmanChangeSpoolDialog.vue # src/components/panels/SpoolmanPanel.vue
…he moonraker docs Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
…/init-server-process # Conflicts: # src/types/MoonrakerRPCInterface.ts
…files Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
…/init-server-process
…sponses Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
…nraker-types-emitAndWait
…/init-server-process
Replace complex dual-timer polling system with unified async approach: - Consolidate 7 actions into 4 by extracting shared checkAndUpdateKlippyState() - Replace callback-based emit() with async emitAndWait() - Merge klippy_connected_timer and klippy_state_timer into single klippy_polling_timer - Load printer/initGcodes only when gcode.commands is empty - Add printer.info RPC type definition Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
…cessing Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
…rver-process # Conflicts: # src/plugins/webSocketClient.ts # src/types/moonraker/PrinterRPC.ts # src/types/moonraker/ServerRPC.ts # src/types/moonraker/index.ts
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
… conversion Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
…ity toggle logic Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
…tion Signed-off-by: Stefan Dej <meteyou@gmail.com>
…ssion Signed-off-by: Stefan Dej <meteyou@gmail.com>
…ity handling Signed-off-by: Stefan Dej <meteyou@gmail.com>
…ate state management Signed-off-by: Stefan Dej <meteyou@gmail.com>
…lity handling Signed-off-by: Stefan Dej <meteyou@gmail.com>
…setter Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
…ations Signed-off-by: Stefan Dej <meteyou@gmail.com>
…nd streamline layout handling Signed-off-by: Stefan Dej <meteyou@gmail.com>
…mutations Signed-off-by: Stefan Dej <meteyou@gmail.com>
…operations Signed-off-by: Stefan Dej <meteyou@gmail.com>
…operations Signed-off-by: Stefan Dej <meteyou@gmail.com>
…ult values Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
📝 WalkthroughWalkthroughThis pull request refactors server initialization from HTTP-based to WebSocket-driven async patterns, introducing progress tracking and staged initialization steps. It adds comprehensive Moonraker RPC type definitions, centralizes config access through getters, and enhances error handling with new initialization state management throughout the application. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/UI
participant Socket as WebSocket
participant Server as Server
participant DB as Database
participant Comp as Components
Client->>Socket: emit init
activate Socket
Socket->>Server: identifyClient
activate Server
Server-->>Socket: connection_id
deactivate Server
Socket->>Client: setInitializationStep("Identifying Client")
Socket->>Server: initServerInfo
activate Server
Server-->>Socket: server info
deactivate Server
Socket->>Client: setInitializationProgress(20)
Socket->>Server: initServerConfig
activate Server
Server-->>Socket: config
deactivate Server
Socket->>Client: setInitializationProgress(40)
Socket->>Server: initDatabases
activate Server
Server->>DB: list namespaces
DB-->>Server: namespaces
Server-->>Socket: database list
deactivate Server
Socket->>Client: setInitializationProgress(60)
Socket->>Server: initServerComponents
activate Server
loop for each component
Server->>Comp: initialize
Comp-->>Server: ready
Server->>Socket: setInitializationStepComponent(component)
end
Server-->>Socket: components ready
deactivate Server
Socket->>Client: setInitializationProgress(100)
Socket->>Server: initKlippyConnection
activate Server
Server-->>Socket: klippy ready
deactivate Server
Socket->>Client: resetInitialization
deactivate Socket
Client->>Client: guiIsReady = true
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@src/components/panels/Timelapse/TimelapseStatusPanel.vue`:
- Around line 135-141: The getter existsSnapshoturlInMoonrakerConfig currently
does "'snapshoturl' in (this.$store.state.server.config_orig?.timelapse ?? {})"
which can throw if timelapse is a non-object primitive; update it to first read
the timelapse value (e.g. assign to a local like timelapse =
this.$store.state.server.config_orig?.timelapse) and only perform the "in" check
when typeof timelapse === 'object' && timelapse !== null (or use
Object.prototype.hasOwnProperty.call) so the "in" operator is guarded and no
TypeError occurs.
In `@src/locales/de.json`:
- Line 174: The "Sensor" translation value uses inconsistent capitalization;
update the value for the "Sensor" key from "Moonraker sensoren" to title case
"Moonraker Sensoren" so it matches the casing style of other entries (e.g.,
"Prozess-Statistiken", "System Informationen") in the locales file.
In `@src/locales/en.json`:
- Line 173: The InitComponents translation entry currently maps "Power" to
"Power-Devices" which uses a hyphen and is inconsistent with other compound
labels and the TopCornerMenu.PowerDevices entry; update the value for the
InitComponents "Power" key to "Power Devices" (matching
TopCornerMenu.PowerDevices and the spacing convention) so the displayed label is
consistent across the UI.
In `@src/store/gui/actions.ts`:
- Around line 24-68: The async restore step is not awaited and can race with the
subsequent exclude-key deletion loop; change the call to
dispatch('restoreValues', values) inside the init action (function init) to
await dispatch('restoreValues', values) so the restore completes before running
the excludeKeys loop and deleteSetting calls.
In `@src/store/gui/gcodehistory/actions.ts`:
- Around line 11-15: In addToHistory replace the unsupported Array.prototype.at
usage by computing the last element via indexing: check state.entries.length > 0
and compare state.entries[state.entries.length - 1] to gcode (handle empty
array) before pushing; then build newHistory the same way and dispatch
'gui/saveSetting' as before (symbols: addToHistory, state.entries,
maxGcodeHistory, dispatch 'gui/saveSetting').
In `@src/store/server/actions.ts`:
- Around line 313-343: Guard against null/non-object payloads before using "in"
and ensure event.type is always a string: in the addEvent action, replace checks
like "typeof payload === 'object' && 'type' in payload" with a fast guard
(payload !== null && typeof payload === 'object') before any "'prop' in payload"
tests, and similarly guard the other `'message'`, `'result'`, and `'error'`
checks; when building the event via formatGcodeEvent({ message, type }) ensure
you coerce or default type to a string (e.g., derive type only when payload is a
non-null object and fallback to 'response' otherwise) so later uses like
event.type and the isErrorMessage check cannot throw.
🧹 Nitpick comments (5)
src/store/gui/gcodehistory/actions.ts (1)
11-12: Check Array.prototype.at browser supportArray.prototype.at is supported in Chrome 92+, Firefox 90+, Safari 15.4+, Edge 92+ and Node 16.6+ but not in older Safari/WebViews; if you need legacy support, replace with
state.entries[state.entries.length - 1]or include a polyfill.🔧 Fallback example
- if (state.entries.at(-1) === gcode) return + if (state.entries[state.entries.length - 1] === gcode) returnsrc/components/panels/Gcodefiles/GcodefilesPanelHeaderSettings.vue (1)
76-84: Variable shadowing in findIndex callback.The
valueparameter in thefindIndexcallback shadows the outervaluevariable, which could cause confusion. Consider renaming the callback parameter.♻️ Suggested fix
changeMetadataVisible(name: string) { const value = [...(this.$store.state.gui.view.gcodefiles.hideMetadataColumns ?? [])] - const index = value.findIndex((value: string) => value === name) + const index = value.findIndex((item: string) => item === name) if (index !== -1) value.splice(index, 1) else value.push(name) this.$store.dispatch('gui/saveSetting', { name: 'view.gcodefiles.hideMetadataColumns', value }) }src/store/socket/actions.ts (1)
158-171: Consider tracking deprecation with an issue.The TODO comments indicate these actions should be removed after converting to async init server components. Consider creating a tracking issue to ensure this cleanup happens.
Would you like me to help create an issue to track the removal of these deprecated actions?
src/components/panels/HistoryListPanel.vue (1)
523-533: Minor inconsistency in state access patterns.
changeColumnVisibleusesthis.hideColums(a getter), whilechangeStatusVisibleaccesses state directly viathis.$store.state.gui.view.history.hidePrintStatus. For consistency, consider adding a getter forhidePrintStatussimilar tohideColums.♻️ Suggested getter for consistency
get hidePrintStatus() { return this.$store.state.gui.view.history.hidePrintStatus ?? [] }Then update the method:
changeStatusVisible(status: string) { - const value = toggleArrayItem(this.$store.state.gui.view.history.hidePrintStatus, status) + const value = toggleArrayItem(this.hidePrintStatus, status) this.$store.dispatch('gui/saveSetting', { name: 'view.history.hidePrintStatus', value }) }src/store/server/mutations.ts (1)
101-105: Consider clearingthrottled_statewhen payload is null to avoid stale UI.If null indicates “not available,” keeping prior values can leave stale flags after reconnect or hardware changes.
♻️ Optional clear-on-null update
setThrottledState(state, payload: ServerState['throttled_state'] | null) { - if (payload === null) return + if (payload === null) { + Vue.set(state.throttled_state, 'bits', 0) + Vue.set(state.throttled_state, 'flags', []) + return + }
Comment on lines
135
to
141
| get existsSnapshoturlInMoonrakerConfig() { | ||
| return 'snapshoturl' in this.$store.state.server.config.orig.timelapse | ||
| return 'snapshoturl' in (this.$store.state.server.config_orig?.timelapse ?? {}) | ||
| } | ||
|
|
||
| get moonrakerTimelapseConfig() { | ||
| return this.$store.state.server.config.config.timelapse ?? {} | ||
| return this.$store.getters['server/getConfigSection']('timelapse', {}) | ||
| } |
There was a problem hiding this comment.
Guard the in check to avoid runtime TypeError.
'snapshoturl' in ... throws if timelapse is a non-object primitive. Add a type guard before using in.
💡 Suggested fix
get existsSnapshoturlInMoonrakerConfig() {
- return 'snapshoturl' in (this.$store.state.server.config_orig?.timelapse ?? {})
+ const timelapseConfig = this.$store.state.server.config_orig?.timelapse
+ return !!timelapseConfig && typeof timelapseConfig === 'object' && 'snapshoturl' in timelapseConfig
}📝 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
| get existsSnapshoturlInMoonrakerConfig() { | |
| return 'snapshoturl' in this.$store.state.server.config.orig.timelapse | |
| return 'snapshoturl' in (this.$store.state.server.config_orig?.timelapse ?? {}) | |
| } | |
| get moonrakerTimelapseConfig() { | |
| return this.$store.state.server.config.config.timelapse ?? {} | |
| return this.$store.getters['server/getConfigSection']('timelapse', {}) | |
| } | |
| get existsSnapshoturlInMoonrakerConfig() { | |
| const timelapseConfig = this.$store.state.server.config_orig?.timelapse | |
| return !!timelapseConfig && typeof timelapseConfig === 'object' && 'snapshoturl' in timelapseConfig | |
| } | |
| get moonrakerTimelapseConfig() { | |
| return this.$store.getters['server/getConfigSection']('timelapse', {}) | |
| } |
🤖 Prompt for AI Agents
In `@src/components/panels/Timelapse/TimelapseStatusPanel.vue` around lines 135 -
141, The getter existsSnapshoturlInMoonrakerConfig currently does "'snapshoturl'
in (this.$store.state.server.config_orig?.timelapse ?? {})" which can throw if
timelapse is a non-object primitive; update it to first read the timelapse value
(e.g. assign to a local like timelapse =
this.$store.state.server.config_orig?.timelapse) and only perform the "in" check
when typeof timelapse === 'object' && timelapse !== null (or use
Object.prototype.hasOwnProperty.call) so the "in" operator is guarded and no
TypeError occurs.
| "JobQueue": "Job-Warteschlange", | ||
| "Power": "Stromgeräte", | ||
| "ProcStats": "Prozess-Statistiken", | ||
| "Sensor": "Moonraker sensoren", |
There was a problem hiding this comment.
Minor inconsistency in capitalization.
The value "Moonraker sensoren" uses lowercase 's' while other entries use title case (e.g., "Prozess-Statistiken", "System Informationen", "Server Informationen"). Consider changing to "Moonraker Sensoren" for consistency.
🤖 Prompt for AI Agents
In `@src/locales/de.json` at line 174, The "Sensor" translation value uses
inconsistent capitalization; update the value for the "Sensor" key from
"Moonraker sensoren" to title case "Moonraker Sensoren" so it matches the casing
style of other entries (e.g., "Prozess-Statistiken", "System Informationen") in
the locales file.
| "GcodeStore": "G-Code Store", | ||
| "History": "History", | ||
| "JobQueue": "Job Queue", | ||
| "Power": "Power-Devices", |
There was a problem hiding this comment.
Minor inconsistency: "Power-Devices" uses a hyphen.
Other compound terms in InitComponents use spaces (e.g., "Job Queue", "Process Statistics", "System Info"). Additionally, the existing key TopCornerMenu.PowerDevices displays as "Power Devices" (line 118). Consider changing to "Power Devices" for consistency.
🤖 Prompt for AI Agents
In `@src/locales/en.json` at line 173, The InitComponents translation entry
currently maps "Power" to "Power-Devices" which uses a hyphen and is
inconsistent with other compound labels and the TopCornerMenu.PowerDevices
entry; update the value for the InitComponents "Power" key to "Power Devices"
(matching TopCornerMenu.PowerDevices and the spacing convention) so the
displayed label is consistent across the UI.
Comment on lines
+24
to
68
| async init({ commit, dispatch, rootGetters, rootState }) { | ||
| logDebug('init') | ||
|
|
||
| async initStore({ commit, dispatch, rootGetters, rootState }, payload) { | ||
| const baseUrl = rootGetters['socket/getUrl'] + '/server/database/item' | ||
| const mainsailUrl = baseUrl + '?namespace=mainsail' | ||
| let values: GuiStateInitPayload = {} | ||
| try { | ||
| const payload = await Vue.$socket.emitAndWait('server.database.get_item', { namespace: 'mainsail' }) | ||
| values = payload.value as GuiStateInitPayload | ||
| } catch (error) { | ||
| logDebug('create Mainsail namespace') | ||
|
|
||
| if ('remoteprinters' in payload.value) { | ||
| if (rootState.instancesDB === 'moonraker') | ||
| dispatch('remoteprinters/initStore', payload.value.remoteprinters.printers) | ||
| delete payload.value.remoteprinters | ||
| } | ||
| values = await this.dispatch('gui/getDefaults') | ||
| values.initVersion = rootGetters['getVersion'] | ||
|
|
||
| // delete currentPath if exists | ||
| if (payload.value?.view?.gcodefiles?.currentPath) { | ||
| window.console.debug('remove currentPath from gui namespace') | ||
| await fetch(mainsailUrl + '&key=view.gcodefiles.currentPath', { method: 'DELETE' }) | ||
| dispatch('restoreValues', values) | ||
| } | ||
|
|
||
| // delete currentPath if exists | ||
| if (payload.value?.view?.configfiles?.currentPath) { | ||
| window.console.debug('remove currentPath from gui namespace') | ||
| await fetch(mainsailUrl + '&key=view.configfiles.currentPath', { method: 'DELETE' }) | ||
| if ('remoteprinters' in values) { | ||
| if (rootState.instancesDB === 'moonraker') { | ||
| dispatch('remoteprinters/initStore', values.remoteprinters?.printers ?? {}) | ||
| } | ||
| delete values.remoteprinters | ||
| } | ||
|
|
||
| //update cooldownGcode from V2.0.1 to V2.1.0 | ||
| if ('cooldownGcode' in payload.value) { | ||
| window.console.debug('update cooldownGcode to new namespace') | ||
| dispatch('saveSetting', { name: 'presets.cooldownGcode', value: payload.value.cooldownGcode }) | ||
| for (const key of excludeKeys) { | ||
| const parts = key.split('.') | ||
| let value: any = values | ||
|
|
||
| await fetch(mainsailUrl + '&key=cooldownGcode', { method: 'DELETE' }) | ||
| delete payload.value.cooldownGcode | ||
| } | ||
|
|
||
| //update presets from V2.0.1 to V2.1.0 | ||
| if ('presets' in payload.value && Array.isArray(payload.value.presets)) { | ||
| window.console.debug('update presets to new namespace') | ||
| for (const part of parts) { | ||
| if (value === undefined || value === null) break | ||
| value = value[part] | ||
| } | ||
|
|
||
| payload.value.presets.forEach((preset: any) => { | ||
| dispatch('presets/store', { values: preset }) | ||
| }) | ||
| if (value === undefined || value === null) continue | ||
|
|
||
| delete payload.value.presets | ||
| logDebug(`remove ${key} from gui namespace`) | ||
| await dispatch('deleteSetting', key) | ||
| deletePath(values, key) | ||
| } | ||
|
|
||
| //update nonExpandPanels from V2.1.x to V2.2.0 | ||
| if ( | ||
| 'dashboard' in payload.value && | ||
| 'nonExpandPanels' in payload.value.dashboard && | ||
| Array.isArray(payload.value.dashboard.nonExpandPanels) | ||
| ) { | ||
| await fetch(mainsailUrl + '&key=dashboard.nonExpandPanels', { method: 'DELETE' }) | ||
| dispatch('saveSetting', { | ||
| name: 'dashboard.nonExpandPanels.widescreen', | ||
| value: payload.value.dashboard.nonExpandPanels, | ||
| }) | ||
| delete payload.value.dashboard.nonExpandPanels | ||
| } | ||
| commit('setData', values) | ||
|
|
||
| //update tools to temperatures panel from V2.1.x to V2.2.0 | ||
| if ('dashboard' in payload.value) { | ||
| const dashboard = payload.value.dashboard | ||
| const layouts = [ | ||
| 'mobileLayout', | ||
| 'tabletLayout1', | ||
| 'tabletLayout2', | ||
| 'desktopLayout1', | ||
| 'desktopLayout2', | ||
| 'widescreenLayout1', | ||
| 'widescreenLayout2', | ||
| 'widescreenLayout3', | ||
| ] | ||
|
|
||
| layouts.forEach((layout) => { | ||
| if (layout in dashboard) { | ||
| const index = dashboard[layout].findIndex((entry: GuiStateLayoutoption) => entry.name === 'tools') | ||
|
|
||
| if (index !== -1) { | ||
| dashboard[layout][index].name = 'temperature' | ||
|
|
||
| dispatch('saveSetting', { | ||
| name: 'dashboard.' + layout, | ||
| value: dashboard[layout], | ||
| }) | ||
| } | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| await commit('setData', payload.value) | ||
| await dispatch('socket/removeInitModule', 'gui/init', { root: true }) | ||
| // TODO: convert to async module initialization | ||
| dispatch('socket/addInitModule', 'gui/webcam/init', { root: true }) | ||
| dispatch('gui/webcams/init', null, { root: true }) | ||
| }, |
There was a problem hiding this comment.
Await restoreValues to avoid races with deleteSetting.
Without awaiting, the async restore can re‑post keys after the exclude‑key deletion loop, leaving unwanted keys in the DB.
🛠️ Proposed fix
- dispatch('restoreValues', values)
+ await dispatch('restoreValues', values)📝 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
| async init({ commit, dispatch, rootGetters, rootState }) { | |
| logDebug('init') | |
| async initStore({ commit, dispatch, rootGetters, rootState }, payload) { | |
| const baseUrl = rootGetters['socket/getUrl'] + '/server/database/item' | |
| const mainsailUrl = baseUrl + '?namespace=mainsail' | |
| let values: GuiStateInitPayload = {} | |
| try { | |
| const payload = await Vue.$socket.emitAndWait('server.database.get_item', { namespace: 'mainsail' }) | |
| values = payload.value as GuiStateInitPayload | |
| } catch (error) { | |
| logDebug('create Mainsail namespace') | |
| if ('remoteprinters' in payload.value) { | |
| if (rootState.instancesDB === 'moonraker') | |
| dispatch('remoteprinters/initStore', payload.value.remoteprinters.printers) | |
| delete payload.value.remoteprinters | |
| } | |
| values = await this.dispatch('gui/getDefaults') | |
| values.initVersion = rootGetters['getVersion'] | |
| // delete currentPath if exists | |
| if (payload.value?.view?.gcodefiles?.currentPath) { | |
| window.console.debug('remove currentPath from gui namespace') | |
| await fetch(mainsailUrl + '&key=view.gcodefiles.currentPath', { method: 'DELETE' }) | |
| dispatch('restoreValues', values) | |
| } | |
| // delete currentPath if exists | |
| if (payload.value?.view?.configfiles?.currentPath) { | |
| window.console.debug('remove currentPath from gui namespace') | |
| await fetch(mainsailUrl + '&key=view.configfiles.currentPath', { method: 'DELETE' }) | |
| if ('remoteprinters' in values) { | |
| if (rootState.instancesDB === 'moonraker') { | |
| dispatch('remoteprinters/initStore', values.remoteprinters?.printers ?? {}) | |
| } | |
| delete values.remoteprinters | |
| } | |
| //update cooldownGcode from V2.0.1 to V2.1.0 | |
| if ('cooldownGcode' in payload.value) { | |
| window.console.debug('update cooldownGcode to new namespace') | |
| dispatch('saveSetting', { name: 'presets.cooldownGcode', value: payload.value.cooldownGcode }) | |
| for (const key of excludeKeys) { | |
| const parts = key.split('.') | |
| let value: any = values | |
| await fetch(mainsailUrl + '&key=cooldownGcode', { method: 'DELETE' }) | |
| delete payload.value.cooldownGcode | |
| } | |
| //update presets from V2.0.1 to V2.1.0 | |
| if ('presets' in payload.value && Array.isArray(payload.value.presets)) { | |
| window.console.debug('update presets to new namespace') | |
| for (const part of parts) { | |
| if (value === undefined || value === null) break | |
| value = value[part] | |
| } | |
| payload.value.presets.forEach((preset: any) => { | |
| dispatch('presets/store', { values: preset }) | |
| }) | |
| if (value === undefined || value === null) continue | |
| delete payload.value.presets | |
| logDebug(`remove ${key} from gui namespace`) | |
| await dispatch('deleteSetting', key) | |
| deletePath(values, key) | |
| } | |
| //update nonExpandPanels from V2.1.x to V2.2.0 | |
| if ( | |
| 'dashboard' in payload.value && | |
| 'nonExpandPanels' in payload.value.dashboard && | |
| Array.isArray(payload.value.dashboard.nonExpandPanels) | |
| ) { | |
| await fetch(mainsailUrl + '&key=dashboard.nonExpandPanels', { method: 'DELETE' }) | |
| dispatch('saveSetting', { | |
| name: 'dashboard.nonExpandPanels.widescreen', | |
| value: payload.value.dashboard.nonExpandPanels, | |
| }) | |
| delete payload.value.dashboard.nonExpandPanels | |
| } | |
| commit('setData', values) | |
| //update tools to temperatures panel from V2.1.x to V2.2.0 | |
| if ('dashboard' in payload.value) { | |
| const dashboard = payload.value.dashboard | |
| const layouts = [ | |
| 'mobileLayout', | |
| 'tabletLayout1', | |
| 'tabletLayout2', | |
| 'desktopLayout1', | |
| 'desktopLayout2', | |
| 'widescreenLayout1', | |
| 'widescreenLayout2', | |
| 'widescreenLayout3', | |
| ] | |
| layouts.forEach((layout) => { | |
| if (layout in dashboard) { | |
| const index = dashboard[layout].findIndex((entry: GuiStateLayoutoption) => entry.name === 'tools') | |
| if (index !== -1) { | |
| dashboard[layout][index].name = 'temperature' | |
| dispatch('saveSetting', { | |
| name: 'dashboard.' + layout, | |
| value: dashboard[layout], | |
| }) | |
| } | |
| } | |
| }) | |
| } | |
| await commit('setData', payload.value) | |
| await dispatch('socket/removeInitModule', 'gui/init', { root: true }) | |
| // TODO: convert to async module initialization | |
| dispatch('socket/addInitModule', 'gui/webcam/init', { root: true }) | |
| dispatch('gui/webcams/init', null, { root: true }) | |
| }, | |
| async init({ commit, dispatch, rootGetters, rootState }) { | |
| logDebug('init') | |
| let values: GuiStateInitPayload = {} | |
| try { | |
| const payload = await Vue.$socket.emitAndWait('server.database.get_item', { namespace: 'mainsail' }) | |
| values = payload.value as GuiStateInitPayload | |
| } catch (error) { | |
| logDebug('create Mainsail namespace') | |
| values = await this.dispatch('gui/getDefaults') | |
| values.initVersion = rootGetters['getVersion'] | |
| await dispatch('restoreValues', values) | |
| } | |
| if ('remoteprinters' in values) { | |
| if (rootState.instancesDB === 'moonraker') { | |
| dispatch('remoteprinters/initStore', values.remoteprinters?.printers ?? {}) | |
| } | |
| delete values.remoteprinters | |
| } | |
| for (const key of excludeKeys) { | |
| const parts = key.split('.') | |
| let value: any = values | |
| for (const part of parts) { | |
| if (value === undefined || value === null) break | |
| value = value[part] | |
| } | |
| if (value === undefined || value === null) continue | |
| logDebug(`remove ${key} from gui namespace`) | |
| await dispatch('deleteSetting', key) | |
| deletePath(values, key) | |
| } | |
| commit('setData', values) | |
| // TODO: convert to async module initialization | |
| dispatch('socket/addInitModule', 'gui/webcam/init', { root: true }) | |
| dispatch('gui/webcams/init', null, { root: true }) | |
| }, |
🤖 Prompt for AI Agents
In `@src/store/gui/actions.ts` around lines 24 - 68, The async restore step is not
awaited and can race with the subsequent exclude-key deletion loop; change the
call to dispatch('restoreValues', values) inside the init action (function init)
to await dispatch('restoreValues', values) so the restore completes before
running the excludeKeys loop and deleteSetting calls.
Comment on lines
+11
to
+15
| async addToHistory({ dispatch, state }, gcode: string) { | ||
| if (state.entries.at(-1) === gcode) return | ||
|
|
||
| await commit('updateHistory', newHistory) | ||
| await dispatch('upload') | ||
| const newHistory = [...state.entries, gcode].slice(-maxGcodeHistory) | ||
| await dispatch('gui/saveSetting', { name: `gcodehistory.entries`, value: newHistory }, { root: true }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the full file to understand the module structure
cat -n src/store/gui/gcodehistory/actions.tsRepository: mainsail-crew/mainsail
Length of output: 819
🏁 Script executed:
# Check if setEntries mutation exists in this module
fd -t f "gcodehistory" --exec cat {} \; | rg -n "setEntries|mutations" -A 3 -B 3Repository: mainsail-crew/mainsail
Length of output: 48
🏁 Script executed:
# Search for gui/saveSetting implementation to understand what it does
rg -n "saveSetting" --type=ts -A 10 | head -100Repository: mainsail-crew/mainsail
Length of output: 6488
🏁 Script executed:
# Check for browser target configuration (package.json, .browserslistrc, tsconfig.json, etc.)
fd -t f "package.json|.browserslistrc|tsconfig.json" -exec cat {} \; | head -200Repository: mainsail-crew/mainsail
Length of output: 253
🏁 Script executed:
# Search for Array.prototype.at usage and any polyfills in the codebase
rg -n "\.at\(" --type=ts | head -30Repository: mainsail-crew/mainsail
Length of output: 157
🏁 Script executed:
cat -n package.json | grep -A 20 "browserslist"Repository: mainsail-crew/mainsail
Length of output: 499
🏁 Script executed:
# Also check tsconfig for target
rg -n "target.*ES" --type=json src/tsconfig.json tsconfig.jsonRepository: mainsail-crew/mainsail
Length of output: 123
🏁 Script executed:
# Check if there are any polyfills or compatibility notes
rg -n "polyfill|at\(" --type=ts -C 2 | head -50Repository: mainsail-crew/mainsail
Length of output: 3804
Replace Array.prototype.at() with a compatible alternative for Safari 12 support.
This action uses Array.prototype.at(), which is an ES2022 feature not supported in Safari 12 (currently in your browserslist). Use array indexing instead:
- async addToHistory({ dispatch, state }, gcode: string) {
+ async addToHistory({ dispatch, state }, gcode: string) {
- if (state.entries.at(-1) === gcode) return
+ if (state.entries[state.entries.length - 1] === gcode) return📝 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
| async addToHistory({ dispatch, state }, gcode: string) { | |
| if (state.entries.at(-1) === gcode) return | |
| await commit('updateHistory', newHistory) | |
| await dispatch('upload') | |
| const newHistory = [...state.entries, gcode].slice(-maxGcodeHistory) | |
| await dispatch('gui/saveSetting', { name: `gcodehistory.entries`, value: newHistory }, { root: true }) | |
| async addToHistory({ dispatch, state }, gcode: string) { | |
| if (state.entries[state.entries.length - 1] === gcode) return | |
| const newHistory = [...state.entries, gcode].slice(-maxGcodeHistory) | |
| await dispatch('gui/saveSetting', { name: `gcodehistory.entries`, value: newHistory }, { root: true }) |
🤖 Prompt for AI Agents
In `@src/store/gui/gcodehistory/actions.ts` around lines 11 - 15, In addToHistory
replace the unsupported Array.prototype.at usage by computing the last element
via indexing: check state.entries.length > 0 and compare
state.entries[state.entries.length - 1] to gcode (handle empty array) before
pushing; then build newHistory the same way and dispatch 'gui/saveSetting' as
before (symbols: addToHistory, state.entries, maxGcodeHistory, dispatch
'gui/saveSetting').
Comment on lines
313
to
343
| addEvent({ commit, rootGetters }, payload) { | ||
| let message = payload | ||
| let type = 'response' | ||
|
|
||
| if (typeof payload === 'object' && 'type' in payload) type = payload.type | ||
|
|
||
| if ('message' in payload) message = payload.message | ||
| else if ('result' in payload) message = payload.result | ||
| else if ('error' in payload) message = message.error.message | ||
|
|
||
| let formatMessage = formatConsoleMessage(message) | ||
| if (type === 'response') { | ||
| if (message.startsWith('// action:')) type = 'action' | ||
| else if (message.startsWith('// debug:')) type = 'debug' | ||
| } | ||
|
|
||
| const filters = rootGetters['gui/console/getConsolefilterRules'] | ||
| let boolImport = true | ||
| filters.every((filter: string) => { | ||
| const type = typeof payload === 'object' && 'type' in payload ? payload.type : 'response' | ||
| let message: string | ||
| if (typeof payload === 'string') { | ||
| message = payload | ||
| } else if ('message' in payload) { | ||
| message = payload.message | ||
| } else if ('result' in payload) { | ||
| message = payload.result | ||
| } else if ('error' in payload) { | ||
| message = payload.error.message | ||
| } else return | ||
|
|
||
| const filters = rootGetters['gui/console/getConsolefilterRules'] as string[] | ||
| const isFiltered = filters.some((filter) => { | ||
| try { | ||
| const regex = new RegExp(filter) | ||
| if (regex.test(formatMessage)) boolImport = false | ||
| return new RegExp(filter).test(message) | ||
| } catch { | ||
| window.console.error("Custom console filter '" + filter + "' doesn't work!") | ||
| logError('invalid console filter:', filter) | ||
| return false | ||
| } | ||
|
|
||
| return boolImport | ||
| }) | ||
| if (isFiltered) return | ||
|
|
||
| if (boolImport) { | ||
| if (payload.type === 'command') formatMessage = '<a class="command text--blue">' + formatMessage + '</a>' | ||
|
|
||
| commit('addEvent', { | ||
| date: new Date(), | ||
| message: message, | ||
| formatMessage: formatMessage, | ||
| type: type, | ||
| }) | ||
| const event = formatGcodeEvent({ message, type }) | ||
| commit('addEvent', event) | ||
|
|
||
| if ( | ||
| ['error', 'response'].includes(type) && | ||
| !['/', '/console'].includes(router.currentRoute.path) && | ||
| message.startsWith('!! ') | ||
| ) { | ||
| Vue.$toast.error(formatMessage) | ||
| } | ||
| } | ||
| const isErrorMessage = ['error', 'response'].includes(event.type) && message.startsWith('!! ') | ||
| const isOnConsolePage = ['/', '/console'].includes(router.currentRoute.path) | ||
| if (isErrorMessage && !isOnConsolePage) Vue.$toast.error(event.formatMessage) | ||
| }, |
There was a problem hiding this comment.
Guard against null/non-object payloads before using in.
'type' in payload and similar checks will throw on null/primitive payloads. Add a fast guard and tighten the type check to keep event.type a string.
🛠️ Proposed fix
addEvent({ commit, rootGetters }, payload) {
- const type = typeof payload === 'object' && 'type' in payload ? payload.type : 'response'
+ if (payload == null) return
+ const type =
+ typeof payload === 'object' &&
+ 'type' in payload &&
+ typeof (payload as { type?: unknown }).type === 'string'
+ ? (payload as { type: string }).type
+ : 'response'
let message: string
if (typeof payload === 'string') {
message = payload
- } else if ('message' in payload) {
+ } else if (typeof payload === 'object' && 'message' in payload) {
message = payload.message
- } else if ('result' in payload) {
+ } else if (typeof payload === 'object' && 'result' in payload) {
message = payload.result
- } else if ('error' in payload) {
+ } else if (typeof payload === 'object' && 'error' in payload) {
message = payload.error.message
} else return📝 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
| addEvent({ commit, rootGetters }, payload) { | |
| let message = payload | |
| let type = 'response' | |
| if (typeof payload === 'object' && 'type' in payload) type = payload.type | |
| if ('message' in payload) message = payload.message | |
| else if ('result' in payload) message = payload.result | |
| else if ('error' in payload) message = message.error.message | |
| let formatMessage = formatConsoleMessage(message) | |
| if (type === 'response') { | |
| if (message.startsWith('// action:')) type = 'action' | |
| else if (message.startsWith('// debug:')) type = 'debug' | |
| } | |
| const filters = rootGetters['gui/console/getConsolefilterRules'] | |
| let boolImport = true | |
| filters.every((filter: string) => { | |
| const type = typeof payload === 'object' && 'type' in payload ? payload.type : 'response' | |
| let message: string | |
| if (typeof payload === 'string') { | |
| message = payload | |
| } else if ('message' in payload) { | |
| message = payload.message | |
| } else if ('result' in payload) { | |
| message = payload.result | |
| } else if ('error' in payload) { | |
| message = payload.error.message | |
| } else return | |
| const filters = rootGetters['gui/console/getConsolefilterRules'] as string[] | |
| const isFiltered = filters.some((filter) => { | |
| try { | |
| const regex = new RegExp(filter) | |
| if (regex.test(formatMessage)) boolImport = false | |
| return new RegExp(filter).test(message) | |
| } catch { | |
| window.console.error("Custom console filter '" + filter + "' doesn't work!") | |
| logError('invalid console filter:', filter) | |
| return false | |
| } | |
| return boolImport | |
| }) | |
| if (isFiltered) return | |
| if (boolImport) { | |
| if (payload.type === 'command') formatMessage = '<a class="command text--blue">' + formatMessage + '</a>' | |
| commit('addEvent', { | |
| date: new Date(), | |
| message: message, | |
| formatMessage: formatMessage, | |
| type: type, | |
| }) | |
| const event = formatGcodeEvent({ message, type }) | |
| commit('addEvent', event) | |
| if ( | |
| ['error', 'response'].includes(type) && | |
| !['/', '/console'].includes(router.currentRoute.path) && | |
| message.startsWith('!! ') | |
| ) { | |
| Vue.$toast.error(formatMessage) | |
| } | |
| } | |
| const isErrorMessage = ['error', 'response'].includes(event.type) && message.startsWith('!! ') | |
| const isOnConsolePage = ['/', '/console'].includes(router.currentRoute.path) | |
| if (isErrorMessage && !isOnConsolePage) Vue.$toast.error(event.formatMessage) | |
| }, | |
| addEvent({ commit, rootGetters }, payload) { | |
| if (payload == null) return | |
| const type = | |
| typeof payload === 'object' && | |
| 'type' in payload && | |
| typeof (payload as { type?: unknown }).type === 'string' | |
| ? (payload as { type: string }).type | |
| : 'response' | |
| let message: string | |
| if (typeof payload === 'string') { | |
| message = payload | |
| } else if (typeof payload === 'object' && 'message' in payload) { | |
| message = payload.message | |
| } else if (typeof payload === 'object' && 'result' in payload) { | |
| message = payload.result | |
| } else if (typeof payload === 'object' && 'error' in payload) { | |
| message = payload.error.message | |
| } else return | |
| const filters = rootGetters['gui/console/getConsolefilterRules'] as string[] | |
| const isFiltered = filters.some((filter) => { | |
| try { | |
| return new RegExp(filter).test(message) | |
| } catch { | |
| logError('invalid console filter:', filter) | |
| return false | |
| } | |
| }) | |
| if (isFiltered) return | |
| const event = formatGcodeEvent({ message, type }) | |
| commit('addEvent', event) | |
| const isErrorMessage = ['error', 'response'].includes(event.type) && message.startsWith('!! ') | |
| const isOnConsolePage = ['/', '/console'].includes(router.currentRoute.path) | |
| if (isErrorMessage && !isOnConsolePage) Vue.$toast.error(event.formatMessage) | |
| }, |
🧰 Tools
🪛 ast-grep (0.40.5)
[warning] 328-328: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(filter)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🤖 Prompt for AI Agents
In `@src/store/server/actions.ts` around lines 313 - 343, Guard against
null/non-object payloads before using "in" and ensure event.type is always a
string: in the addEvent action, replace checks like "typeof payload === 'object'
&& 'type' in payload" with a fast guard (payload !== null && typeof payload ===
'object') before any "'prop' in payload" tests, and similarly guard the other
`'message'`, `'result'`, and `'error'` checks; when building the event via
formatGcodeEvent({ message, type }) ensure you coerce or default type to a
string (e.g., derive type only when payload is a non-null object and fallback to
'response' otherwise) so later uses like event.type and the isErrorMessage check
cannot throw.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR refactor the complete gui/gcodehistory store and is based on #2421 .
Related Tickets & Documents
none
Mobile & Desktop Screenshots/Recordings
none
[optional] Are there any post-deployment tasks we need to perform?
none