From 6fd60e59c48d6e67bc1a3a7f2a6f68e95bb9200b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Fri, 7 Aug 2026 10:38:52 +0000 Subject: [PATCH] one way to mint a panel id, and a registry that cannot silently lose an app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two second implementations, both removed rather than fixed. DashboardPreview minted template panel ids with its own module-level counter, tpl-1, tpl-2, no entropy, reset every page load. Two dashboards built from templates in the same page load held panels with identical ids — and a panel id is not decorative any more: agent_panels addresses an agent by (dashboardId, panelId), and terminal-conn- and file-viewer: key persisted state by it. The templates now call the core uid(), which is exported from the Workspace barrel for the first time so there is one minter. metasToRegistry is Object.fromEntries, so two apps sharing a key means one app stops existing and every panel holding its appType renders the other. The todo asked for a throw in dev; a throw takes down every dashboard at runtime for a mistake made at edit time, so this is a test over the real meta list plus a console.error. All 44 keys are unique, and the test now says so rather than the doc. Getting the real list into a test needed test-setup.ts to provide localStorage: MusicPlayer/useLyricsOpen.ts reads it at import time, so the whole app graph was unimportable from a test. That unblocks testing anything that pulls in a panel app. Also deletes officerdev/src/useAppRegistry.ts — a stub returning {} with a different shape from the real hook, imported by nothing. --- .../src/AppRegistry/AppRegistry.test.ts | 39 ++++++++++++++ .../src/AppRegistry/AppRegistry.tsx | 2 +- .../src/AppRegistry/useAppRegistry/index.ts | 8 ++- .../useAppRegistry/useAppRegistry.ts | 19 ++++++- .../src/apps/Dashboards/DashboardPreview.tsx | 53 +++++++++---------- .../src/components/Workspace/index.ts | 1 + .../src/components/Workspace/layout-utils.ts | 3 +- .../officerdev/src/useAppRegistry.ts | 5 -- test-setup.ts | 3 ++ 9 files changed, 95 insertions(+), 38 deletions(-) create mode 100644 src/workspaces/officerdev/src/AppRegistry/AppRegistry.test.ts delete mode 100644 src/workspaces/officerdev/src/useAppRegistry.ts diff --git a/src/workspaces/officerdev/src/AppRegistry/AppRegistry.test.ts b/src/workspaces/officerdev/src/AppRegistry/AppRegistry.test.ts new file mode 100644 index 00000000..1b440a79 --- /dev/null +++ b/src/workspaces/officerdev/src/AppRegistry/AppRegistry.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, test } from 'bun:test'; +import { apps } from './AppRegistry'; +import { findDuplicateKeys, metasToRegistry } from './useAppRegistry'; + +/** + * `metasToRegistry` is `Object.fromEntries`, so two metas sharing a key means one app silently stops + * existing — and every panel already storing that `appType` renders the survivor instead. Nothing at + * runtime can recover from that, so it is caught here, over the real list the app actually mounts. + */ +describe('the real app registry', () => { + test('every app key is unique', () => { + expect(findDuplicateKeys(apps)).toEqual([]); + }); + + test('no app is lost between the meta list and the registry', () => { + expect(Object.keys(metasToRegistry(apps)).length).toBe(apps.length); + }); + + test('every app has a key, a name and a component', () => { + const incomplete = apps.filter((a) => !a.key || !a.name || !a.component).map((a) => a.key ?? '(no key)'); + expect(incomplete).toEqual([]); + }); +}); + +describe('findDuplicateKeys', () => { + const meta = (key: string) => ({ key }) as (typeof apps)[number]; + + test('reports nothing for a clean list', () => { + expect(findDuplicateKeys([meta('a'), meta('b')])).toEqual([]); + }); + + test('reports a repeated key once, however many times it repeats', () => { + expect(findDuplicateKeys([meta('a'), meta('a'), meta('a')])).toEqual(['a']); + }); + + test('reports each distinct collision', () => { + expect(findDuplicateKeys([meta('a'), meta('b'), meta('a'), meta('b')])).toEqual(['a', 'b']); + }); +}); diff --git a/src/workspaces/officerdev/src/AppRegistry/AppRegistry.tsx b/src/workspaces/officerdev/src/AppRegistry/AppRegistry.tsx index 6623059e..613704c2 100644 --- a/src/workspaces/officerdev/src/AppRegistry/AppRegistry.tsx +++ b/src/workspaces/officerdev/src/AppRegistry/AppRegistry.tsx @@ -21,7 +21,7 @@ import { appRegistryMetas as qrTransferMetas } from '../apps/QrTransfer'; import { appRegistryMetas as davMetas } from '../apps/Dav'; import { useAppRegistry } from './useAppRegistry'; -const apps = [ +export const apps = [ ...fileBrowserMetas, ...terminalMetas, ...codeEditorMetas, diff --git a/src/workspaces/officerdev/src/AppRegistry/useAppRegistry/index.ts b/src/workspaces/officerdev/src/AppRegistry/useAppRegistry/index.ts index f5cbe731..0ca19d08 100644 --- a/src/workspaces/officerdev/src/AppRegistry/useAppRegistry/index.ts +++ b/src/workspaces/officerdev/src/AppRegistry/useAppRegistry/index.ts @@ -1 +1,7 @@ -export { useAppRegistry, type UseAppRegistryType, type AppRegistryMeta } from './useAppRegistry'; +export { + useAppRegistry, + metasToRegistry, + findDuplicateKeys, + type UseAppRegistryType, + type AppRegistryMeta, +} from './useAppRegistry'; diff --git a/src/workspaces/officerdev/src/AppRegistry/useAppRegistry/useAppRegistry.ts b/src/workspaces/officerdev/src/AppRegistry/useAppRegistry/useAppRegistry.ts index 070e7888..eae13efa 100644 --- a/src/workspaces/officerdev/src/AppRegistry/useAppRegistry/useAppRegistry.ts +++ b/src/workspaces/officerdev/src/AppRegistry/useAppRegistry/useAppRegistry.ts @@ -15,5 +15,20 @@ export function useAppRegistry(initialApps: AppRegistryMeta[] = []) { export type UseAppRegistryType = ReturnType; -const metasToRegistry = (metas: AppRegistryMeta[]): AppRegistryMap => - Object.fromEntries(metas.map(({ key, ...entry }) => [key, entry])); +export const findDuplicateKeys = (metas: AppRegistryMeta[]): string[] => [ + ...new Set(metas.map((m) => m.key).filter((key, i, keys) => keys.indexOf(key) !== i)), +]; + +/** + * A collision here last-wins silently: one app stops existing, and every panel already holding its + * `appType` renders the other app's component instead. It is an authoring mistake, so it is caught by a + * test over the real meta list (`AppRegistry.test.ts`) rather than by throwing — a duplicate key is not a + * reason to take the owner's dashboard down at runtime. + */ +export const metasToRegistry = (metas: AppRegistryMeta[]): AppRegistryMap => { + const duplicates = findDuplicateKeys(metas); + if (duplicates.length > 0) + console.error(`AppRegistry: duplicate app key(s) ${duplicates.join(', ')} — one app is unreachable`); + + return Object.fromEntries(metas.map(({ key, ...entry }) => [key, entry])); +}; diff --git a/src/workspaces/officerdev/src/apps/Dashboards/DashboardPreview.tsx b/src/workspaces/officerdev/src/apps/Dashboards/DashboardPreview.tsx index 7e4d9d96..af639137 100644 --- a/src/workspaces/officerdev/src/apps/Dashboards/DashboardPreview.tsx +++ b/src/workspaces/officerdev/src/apps/Dashboards/DashboardPreview.tsx @@ -6,7 +6,7 @@ import { useGlobal } from 'hooks/useGlobal'; import { useClient } from 'hooks/useClient'; import { useUserState } from 'state/useUserState'; import { useDashboardState, persistDashboardState } from 'state/useDashboardState'; -import { WorkspaceLayout, WorkspaceView, createDefaultLayout } from '../../components/Workspace'; +import { WorkspaceLayout, WorkspaceView, createDefaultLayout, uid } from '../../components/Workspace'; import type { LayoutNode, DashboardDefinition } from '../../components/Workspace'; import { Button } from '@/components/ui/button'; import { generateSlug, slugify } from 'helpers/slug'; @@ -22,9 +22,6 @@ import { // --- Layout Templates --- -let tplCounter = 0; -const tplUid = () => `tpl-${++tplCounter}`; - type LayoutTemplate = { name: string; layout: () => LayoutNode; @@ -33,17 +30,17 @@ type LayoutTemplate = { const templates: LayoutTemplate[] = [ { name: 'Single', - layout: () => ({ type: 'panel', id: tplUid(), appType: null }), + layout: () => ({ type: 'panel', id: uid(), appType: null }), }, { name: '2 Columns', layout: () => ({ type: 'group', - id: tplUid(), + id: uid(), direction: 'horizontal', children: [ - { node: { type: 'panel', id: tplUid(), appType: null }, size: 50 }, - { node: { type: 'panel', id: tplUid(), appType: null }, size: 50 }, + { node: { type: 'panel', id: uid(), appType: null }, size: 50 }, + { node: { type: 'panel', id: uid(), appType: null }, size: 50 }, ], }), }, @@ -51,18 +48,18 @@ const templates: LayoutTemplate[] = [ name: 'Main + Side', layout: () => ({ type: 'group', - id: tplUid(), + id: uid(), direction: 'horizontal', children: [ - { node: { type: 'panel', id: tplUid(), appType: null }, size: 50 }, + { node: { type: 'panel', id: uid(), appType: null }, size: 50 }, { node: { type: 'group', - id: tplUid(), + id: uid(), direction: 'vertical', children: [ - { node: { type: 'panel', id: tplUid(), appType: null }, size: 50 }, - { node: { type: 'panel', id: tplUid(), appType: null }, size: 50 }, + { node: { type: 'panel', id: uid(), appType: null }, size: 50 }, + { node: { type: 'panel', id: uid(), appType: null }, size: 50 }, ], }, size: 50, @@ -74,11 +71,11 @@ const templates: LayoutTemplate[] = [ name: 'Sidebar', layout: () => ({ type: 'group', - id: tplUid(), + id: uid(), direction: 'horizontal', children: [ - { node: { type: 'panel', id: tplUid(), appType: null }, size: 25 }, - { node: { type: 'panel', id: tplUid(), appType: null }, size: 75 }, + { node: { type: 'panel', id: uid(), appType: null }, size: 25 }, + { node: { type: 'panel', id: uid(), appType: null }, size: 75 }, ], }), }, @@ -86,17 +83,17 @@ const templates: LayoutTemplate[] = [ name: '2x2 Grid', layout: () => ({ type: 'group', - id: tplUid(), + id: uid(), direction: 'vertical', children: [ { node: { type: 'group', - id: tplUid(), + id: uid(), direction: 'horizontal', children: [ - { node: { type: 'panel', id: tplUid(), appType: null }, size: 50 }, - { node: { type: 'panel', id: tplUid(), appType: null }, size: 50 }, + { node: { type: 'panel', id: uid(), appType: null }, size: 50 }, + { node: { type: 'panel', id: uid(), appType: null }, size: 50 }, ], }, size: 50, @@ -104,11 +101,11 @@ const templates: LayoutTemplate[] = [ { node: { type: 'group', - id: tplUid(), + id: uid(), direction: 'horizontal', children: [ - { node: { type: 'panel', id: tplUid(), appType: null }, size: 50 }, - { node: { type: 'panel', id: tplUid(), appType: null }, size: 50 }, + { node: { type: 'panel', id: uid(), appType: null }, size: 50 }, + { node: { type: 'panel', id: uid(), appType: null }, size: 50 }, ], }, size: 50, @@ -120,22 +117,22 @@ const templates: LayoutTemplate[] = [ name: 'Cols + Bottom', layout: () => ({ type: 'group', - id: tplUid(), + id: uid(), direction: 'vertical', children: [ { node: { type: 'group', - id: tplUid(), + id: uid(), direction: 'horizontal', children: [ - { node: { type: 'panel', id: tplUid(), appType: null }, size: 50 }, - { node: { type: 'panel', id: tplUid(), appType: null }, size: 50 }, + { node: { type: 'panel', id: uid(), appType: null }, size: 50 }, + { node: { type: 'panel', id: uid(), appType: null }, size: 50 }, ], }, size: 70, }, - { node: { type: 'panel', id: tplUid(), appType: null }, size: 30 }, + { node: { type: 'panel', id: uid(), appType: null }, size: 30 }, ], }), }, diff --git a/src/workspaces/officerdev/src/components/Workspace/index.ts b/src/workspaces/officerdev/src/components/Workspace/index.ts index 2693b69c..bf3bf49e 100644 --- a/src/workspaces/officerdev/src/components/Workspace/index.ts +++ b/src/workspaces/officerdev/src/components/Workspace/index.ts @@ -14,6 +14,7 @@ export type { } from './types'; export type { DropPosition } from './layout-utils'; export { + uid, createDefaultLayout, splitPanel, removePanel, diff --git a/src/workspaces/officerdev/src/components/Workspace/layout-utils.ts b/src/workspaces/officerdev/src/components/Workspace/layout-utils.ts index 4fcfe517..d8419413 100644 --- a/src/workspaces/officerdev/src/components/Workspace/layout-utils.ts +++ b/src/workspaces/officerdev/src/components/Workspace/layout-utils.ts @@ -1,7 +1,8 @@ import type { LayoutNode, LayoutPanel, LayoutGroup, PanelConfig } from './types'; let counter = 0; -const uid = () => `p-${Date.now()}-${++counter}`; +/** The only way to mint a panel id. Time-stamped, so ids stay unique across page loads and dashboards. */ +export const uid = () => `p-${Date.now()}-${++counter}`; export const createDefaultLayout = (): LayoutPanel => ({ type: 'panel', diff --git a/src/workspaces/officerdev/src/useAppRegistry.ts b/src/workspaces/officerdev/src/useAppRegistry.ts deleted file mode 100644 index cb46d942..00000000 --- a/src/workspaces/officerdev/src/useAppRegistry.ts +++ /dev/null @@ -1,5 +0,0 @@ -import type { AppRegistryMap } from './components/Workspace'; - -export function useAppRegistry(): AppRegistryMap { - return {}; -} diff --git a/test-setup.ts b/test-setup.ts index fc3b58e6..7fb6068e 100644 --- a/test-setup.ts +++ b/test-setup.ts @@ -9,6 +9,9 @@ Object.assign(globalThis, { navigator: window.navigator, location: window.location, history: window.history, + // Several modules read storage at import time, so this must exist before the first import, not first render. + localStorage: window.localStorage, + sessionStorage: window.sessionStorage, HTMLElement: window.HTMLElement, Element: window.Element, Node: window.Node,