From 0a1b7f969971024d675ea9c58f2a3a4df6d25d0c Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Wed, 1 Jul 2026 22:05:48 +0200 Subject: [PATCH] fix: decouple settings package loading --- .changeset/responsive-package-settings.md | 5 ++ src/client/src/components/SettingsDialog.ts | 72 +++++++++++-------- .../settings/SettingsPackagesPanel.ts | 27 ++++--- .../settings/settingsDataLoading.test.ts | 55 ++++++++++++++ .../settings/settingsDataLoading.ts | 45 ++++++++++++ 5 files changed, 166 insertions(+), 38 deletions(-) create mode 100644 .changeset/responsive-package-settings.md create mode 100644 src/client/src/components/settings/settingsDataLoading.test.ts create mode 100644 src/client/src/components/settings/settingsDataLoading.ts diff --git a/.changeset/responsive-package-settings.md b/.changeset/responsive-package-settings.md new file mode 100644 index 0000000..141c148 --- /dev/null +++ b/.changeset/responsive-package-settings.md @@ -0,0 +1,5 @@ +--- +"@jmfederico/pi-web": patch +--- + +Keep gateway Settings panels responsive while selected-machine Pi packages load or fail separately. diff --git a/src/client/src/components/SettingsDialog.ts b/src/client/src/components/SettingsDialog.ts index 4883d04..85278ce 100644 --- a/src/client/src/components/SettingsDialog.ts +++ b/src/client/src/components/SettingsDialog.ts @@ -9,6 +9,7 @@ import "./settings/SettingsPackagesPanel"; import "./settings/SettingsPluginsPanel"; import "./settings/SettingsShortcutsPanel"; import { friendlyPiPackageErrorMessage, piPackageMutationFollowUpMessage, piPackageTargetContext, piPackageTargetLabel, shouldRefreshGatewayPluginsAfterPiPackageMutation, type PiPackageOperationState, type PiPackageTargetContext } from "./settings/piPackageSettings"; +import { loadGatewaySettingsData, loadPiPackagesData } from "./settings/settingsDataLoading"; @customElement("settings-dialog") export class SettingsDialog extends LitElement { @@ -22,6 +23,7 @@ export class SettingsDialog extends LitElement { @state() private pluginsResponse: PiWebPluginsResponse | undefined; @state() private packagesResponse: PiPackagesResponse | undefined; @state() private loading = true; + @state() private packageLoading = true; @state() private saving = false; @state() private packageOperation: PiPackageOperationState | undefined; @state() private error = ""; @@ -30,12 +32,13 @@ export class SettingsDialog extends LitElement { @state() private packageMessage = ""; private savedMessageTimer: number | undefined; private loadRequestSeq = 0; + private packageLoadRequestSeq = 0; private packageMutationSeq = 0; - private lastRequestedPackageTargetId: string | undefined; override connectedCallback(): void { super.connectedCallback(); void this.loadConfig(); + void this.loadPackagesForTarget(); } override disconnectedCallback(): void { @@ -50,7 +53,7 @@ export class SettingsDialog extends LitElement { const currentTarget = this.packageTarget(); if (previousTarget.id === currentTarget.id) return; this.resetPackageStateForTargetChange(); - if (this.isConnected && this.lastRequestedPackageTargetId !== currentTarget.id) void this.loadConfig(); + if (this.isConnected) void this.loadPackagesForTarget(currentTarget); } override render(): TemplateResult { @@ -115,11 +118,11 @@ export class SettingsDialog extends LitElement { this.loadConfig()} + .onReload=${() => this.loadPackagesForTarget()} .onInstallPackage=${(source: string) => this.installPiPackage(source)} .onRemovePackage=${(source: string, scope: PiPackageScope) => this.removePiPackage(source, scope)} .onUpdatePackage=${(source?: string) => this.updatePiPackage(source)} @@ -184,32 +187,37 @@ export class SettingsDialog extends LitElement { } private async loadConfig(): Promise { - const target = this.packageTarget(); const requestSeq = ++this.loadRequestSeq; - this.lastRequestedPackageTargetId = target.id; this.loading = true; this.error = ""; - this.packageError = ""; try { - const [config, plugins, packages] = await Promise.allSettled([configApi.config(), pluginsApi.plugins(), piPackagesApi.packages(target.id)]); - if (!this.isCurrentLoad(requestSeq, target)) return; + const result = await loadGatewaySettingsData({ + loadConfig: () => configApi.config(), + loadPlugins: () => pluginsApi.plugins(), + }); + if (!this.isCurrentLoad(requestSeq)) return; - const errors: string[] = []; - if (config.status === "fulfilled") this.configResponse = config.value; - else errors.push(`config: ${errorMessage(config.reason)}`); - - if (plugins.status === "fulfilled") this.pluginsResponse = plugins.value; - else errors.push(`PI WEB plugins: ${errorMessage(plugins.reason)}`); - - if (packages.status === "fulfilled") this.packagesResponse = packages.value; - else { - this.packagesResponse = undefined; - this.packageError = `Failed to load Pi packages from ${piPackageTargetLabel(target)}: ${friendlyPiPackageErrorMessage(errorMessage(packages.reason), target)}`; - } - - if (errors.length > 0) this.error = `Failed to load settings: ${errors.join("; ")}`; + if (result.config !== undefined) this.configResponse = result.config; + if (result.plugins !== undefined) this.pluginsResponse = result.plugins; + this.error = result.error; } finally { - if (this.isCurrentLoad(requestSeq, target)) this.loading = false; + if (this.isCurrentLoad(requestSeq)) this.loading = false; + } + } + + private async loadPackagesForTarget(target = this.packageTarget()): Promise { + const requestSeq = ++this.packageLoadRequestSeq; + this.packageLoading = true; + this.packageError = ""; + this.packageMessage = ""; + try { + const result = await loadPiPackagesData(target, (targetId) => piPackagesApi.packages(targetId)); + if (!this.isCurrentPackageLoad(requestSeq, target)) return; + + this.packagesResponse = result.packagesResponse; + this.packageError = result.error; + } finally { + if (this.isCurrentPackageLoad(requestSeq, target)) this.packageLoading = false; } } @@ -233,8 +241,6 @@ export class SettingsDialog extends LitElement { this.saving = true; this.error = ""; this.savedMessage = ""; - this.packageMessage = ""; - this.packageError = ""; try { const response = await configApi.saveConfig(config); this.configResponse = response; @@ -265,11 +271,11 @@ export class SettingsDialog extends LitElement { private async runPiPackageMutation(operation: PiPackageOperationState, label: string, target: PiPackageTargetContext, mutate: () => Promise): Promise { if (this.saving) throw new Error("A settings operation is already running."); const requestSeq = ++this.packageMutationSeq; + this.packageLoadRequestSeq += 1; + this.packageLoading = false; this.saving = true; this.packageOperation = operation; - this.error = ""; this.packageError = ""; - this.savedMessage = ""; this.packageMessage = ""; try { const response = await mutate(); @@ -303,8 +309,12 @@ export class SettingsDialog extends LitElement { return piPackageTargetContext(this.machine); } - private isCurrentLoad(requestSeq: number, target: PiPackageTargetContext): boolean { - return requestSeq === this.loadRequestSeq && this.isCurrentPackageTarget(target); + private isCurrentLoad(requestSeq: number): boolean { + return requestSeq === this.loadRequestSeq; + } + + private isCurrentPackageLoad(requestSeq: number, target: PiPackageTargetContext): boolean { + return requestSeq === this.packageLoadRequestSeq && this.isCurrentPackageTarget(target); } private isCurrentPackageMutation(requestSeq: number, target: PiPackageTargetContext): boolean { @@ -317,7 +327,9 @@ export class SettingsDialog extends LitElement { private resetPackageStateForTargetChange(): void { const hadPackageOperation = this.packageOperation !== undefined; + this.packageLoadRequestSeq += 1; this.packageMutationSeq += 1; + this.packageLoading = false; this.packageOperation = undefined; this.packageMessage = ""; this.packageError = ""; diff --git a/src/client/src/components/settings/SettingsPackagesPanel.ts b/src/client/src/components/settings/SettingsPackagesPanel.ts index d8d9969..d133efe 100644 --- a/src/client/src/components/settings/SettingsPackagesPanel.ts +++ b/src/client/src/components/settings/SettingsPackagesPanel.ts @@ -52,8 +52,11 @@ export class SettingsPackagesPanel extends LitElement { } private renderPackageList(packages: PiPackageInfo[], target: PiPackageTargetContext): TemplateResult { - const updateAllReason = updateAllPiPackagesDisabledReason(packages); const targetLabel = piPackageTargetLabel(target); + const packageListUnavailable = this.error !== "" && packages.length === 0; + const updateAllReason = updateAllPiPackagesDisabledReason(packages); + const showUpdateAllReason = updateAllReason !== undefined && (packages.length > 0 || (!this.loading && !packageListUnavailable)); + const updateAllTitle = packageListUnavailable ? `Pi package list unavailable for ${targetLabel}` : updateAllReason ?? "Update all user-scope Pi packages"; return html`
@@ -61,20 +64,28 @@ export class SettingsPackagesPanel extends LitElement {

Configured Pi packages

This list comes from Pi's package manager settings on ${targetLabel}.

- - ${updateAllReason === undefined ? null : html`
${updateAllReason}
`} - ${this.loading && packages.length === 0 ? html`
Loading Pi packages from ${targetLabel}…
` : packages.length === 0 ? html`
No Pi packages configured in Pi settings on ${targetLabel} yet.
` : html` -
- ${packages.map((packageInfo) => this.renderPackage(packageInfo))} -
- `} + ${showUpdateAllReason ? html`
${updateAllReason}
` : null} + ${this.loading && packages.length > 0 ? html`
Refreshing Pi packages from ${targetLabel}…
` : null} + ${this.renderPackageListContent(packages, targetLabel)}
`; } + private renderPackageListContent(packages: PiPackageInfo[], targetLabel: string): TemplateResult { + if (this.loading && packages.length === 0) return html`
Loading Pi packages from ${targetLabel}…
`; + if (this.error !== "" && packages.length === 0) return html`
Pi package list unavailable for ${targetLabel}. Use Reload to try again.
`; + if (packages.length === 0) return html`
No Pi packages configured in Pi settings on ${targetLabel} yet.
`; + return html` +
+ ${packages.map((packageInfo) => this.renderPackage(packageInfo))} +
+ `; + } + private renderPackage(packageInfo: PiPackageInfo): TemplateResult { const updateReason = piPackageUpdateDisabledReason(packageInfo); const updating = isPiPackageOperationPending(this.operation, "update", packageInfo.source); diff --git a/src/client/src/components/settings/settingsDataLoading.test.ts b/src/client/src/components/settings/settingsDataLoading.test.ts new file mode 100644 index 0000000..0124992 --- /dev/null +++ b/src/client/src/components/settings/settingsDataLoading.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from "vitest"; +import type { PiPackagesResponse, PiWebConfigResponse, PiWebPluginsResponse } from "../../api"; +import { loadGatewaySettingsData, loadPiPackagesData } from "./settingsDataLoading"; + +const configResponse: PiWebConfigResponse = { + path: "/home/test/.config/pi-web/config.json", + exists: true, + config: { host: "127.0.0.1" }, + effectiveConfig: { host: "127.0.0.1" }, + envOverrides: { host: false, port: false, allowedHosts: false, spawnSessions: false, subsessions: false }, +}; + +const pluginsResponse: PiWebPluginsResponse = { plugins: [] }; +const packagesResponse: PiPackagesResponse = { packages: [{ source: "npm:@acme/tools", scope: "user", filtered: false }] }; + +const remoteTarget = { id: "remote-a", name: "Lab Mac", kind: "remote" } as const; + +describe("settings data loading helpers", () => { + it("loads gateway settings without depending on Pi package data", async () => { + const result = await loadGatewaySettingsData({ + loadConfig: () => Promise.resolve(configResponse), + loadPlugins: () => Promise.resolve(pluginsResponse), + }); + + expect(result).toEqual({ config: configResponse, plugins: pluginsResponse, error: "" }); + }); + + it("keeps gateway settings errors scoped to gateway config and plugins", async () => { + const result = await loadGatewaySettingsData({ + loadConfig: () => Promise.resolve(configResponse), + loadPlugins: () => Promise.reject(new Error("plugin scan failed")), + }); + + expect(result.config).toBe(configResponse); + expect(result.plugins).toBeUndefined(); + expect(result.error).toBe("Failed to load settings: PI WEB plugins: plugin scan failed"); + }); + + it("loads Pi packages for the selected target with package-scoped errors", async () => { + const requestedTargets: string[] = []; + const success = await loadPiPackagesData(remoteTarget, (targetId) => { + requestedTargets.push(targetId); + return Promise.resolve(packagesResponse); + }); + const failure = await loadPiPackagesData(remoteTarget, (targetId) => { + requestedTargets.push(targetId); + return Promise.reject(new Error("Remote machine unavailable")); + }); + + expect(requestedTargets).toEqual(["remote-a", "remote-a"]); + expect(success).toEqual({ packagesResponse, error: "" }); + expect(failure.packagesResponse).toBeUndefined(); + expect(failure.error).toBe("Failed to load Pi packages from Lab Mac (remote machine): Could not reach Lab Mac for Pi package management. Check the machine connection and try again."); + }); +}); diff --git a/src/client/src/components/settings/settingsDataLoading.ts b/src/client/src/components/settings/settingsDataLoading.ts new file mode 100644 index 0000000..65dd032 --- /dev/null +++ b/src/client/src/components/settings/settingsDataLoading.ts @@ -0,0 +1,45 @@ +import type { PiPackagesResponse, PiWebConfigResponse, PiWebPluginsResponse } from "../../api"; +import { friendlyPiPackageErrorMessage, piPackageTargetLabel, type PiPackageTargetContext } from "./piPackageSettings"; + +export interface GatewaySettingsLoaders { + loadConfig: () => Promise; + loadPlugins: () => Promise; +} + +export interface GatewaySettingsLoadResult { + config?: PiWebConfigResponse; + plugins?: PiWebPluginsResponse; + error: string; +} + +export interface PiPackagesLoadResult { + packagesResponse?: PiPackagesResponse; + error: string; +} + +export async function loadGatewaySettingsData(loaders: GatewaySettingsLoaders): Promise { + const [config, plugins] = await Promise.allSettled([loaders.loadConfig(), loaders.loadPlugins()]); + const result: GatewaySettingsLoadResult = { error: "" }; + const errors: string[] = []; + + if (config.status === "fulfilled") result.config = config.value; + else errors.push(`config: ${errorMessage(config.reason)}`); + + if (plugins.status === "fulfilled") result.plugins = plugins.value; + else errors.push(`PI WEB plugins: ${errorMessage(plugins.reason)}`); + + if (errors.length > 0) result.error = `Failed to load settings: ${errors.join("; ")}`; + return result; +} + +export async function loadPiPackagesData(target: PiPackageTargetContext, loadPackages: (targetId: string) => Promise): Promise { + try { + return { packagesResponse: await loadPackages(target.id), error: "" }; + } catch (error) { + return { error: `Failed to load Pi packages from ${piPackageTargetLabel(target)}: ${friendlyPiPackageErrorMessage(errorMessage(error), target)}` }; + } +} + +function errorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +}