From da57a937ed1dbdd115d8aa61cdef17a8e79e1efa Mon Sep 17 00:00:00 2001 From: ajnart Date: Sun, 23 Aug 2026 22:57:17 +0200 Subject: [PATCH] fix(custom-widgets): address review and CI --- CUSTOM_WIDGET_NORTH_STAR_AUDIT.md | 4 +-- .../_custom-widget-form-utils.spec.ts | 7 ---- .../src/jsx/runtime-components.tsx | 8 +++-- .../src/runtime/custom-jsx-renderer.tsx | 18 ++++++---- .../custom-widgets/src/test/runtime.spec.tsx | 36 ++++++------------- .../src/workbench/code-editor.module.css | 2 +- .../src/workbench/code-editor.tsx | 6 +++- .../src/workbench/direct-code-mirror.tsx | 5 ++- packages/translation/src/lang/en.json | 1 + .../src/custom-api/custom-jsx-display.tsx | 8 ++++- 10 files changed, 49 insertions(+), 46 deletions(-) diff --git a/CUSTOM_WIDGET_NORTH_STAR_AUDIT.md b/CUSTOM_WIDGET_NORTH_STAR_AUDIT.md index 37d8a875f..8f6c53b35 100644 --- a/CUSTOM_WIDGET_NORTH_STAR_AUDIT.md +++ b/CUSTOM_WIDGET_NORTH_STAR_AUDIT.md @@ -1400,8 +1400,8 @@ This is the shortest useful map from the report back to the current code. Line n | Renderer session and synchronous render | [`custom-jsx-renderer.tsx`](packages/custom-widgets/src/runtime/custom-jsx-renderer.tsx) | Stable session/binding reconciliation is delivered; interpretation and binding hashing still belong in a shared compile artifact. | | Existing editor undo/redo | [`code-editor.tsx`](packages/custom-widgets/src/workbench/code-editor.tsx) and [`direct-code-mirror.tsx`](packages/custom-widgets/src/workbench/direct-code-mirror.tsx) | CodeMirror history, depth counters, toolbar commands, and `basicSetup` already exist; the missing layer is coherent workbench-wide undo. | | Interpreter parse/render entry | [`interpreter.tsx`](packages/custom-widgets/src/jsx/interpreter.tsx) and [`interpreter-parser.ts`](packages/custom-widgets/src/jsx/interpreter-parser.ts) | Candidate for a shared generation-tagged compile artifact. | -| Runtime display bindings | [`custom-jsx-display.tsx`](packages/widgets/src/custom-api/custom-jsx-display.tsx) | Template change state clearing and fresh data/status wrapper objects. | -| Prompt and channel contract | [`ai-prompt.ts`](packages/custom-widgets/src/core/ai-prompt.ts), [`_copy-ai-prompt-button.tsx`](apps/nextjs/src/app/[locale]/manage/custom-widgets/_copy-ai-prompt-button.tsx), and [`assistant-tool-policy.ts`](apps/nextjs/src/app/api/assistant/chat/assistant-tool-policy.ts) | Clipboard final-output instruction conflicts with the in-product tool lifecycle. | +| Runtime display bindings | [`custom-jsx-display.tsx`](packages/widgets/src/custom-api/custom-jsx-display.tsx) | Published/query-cache state is now definition-scoped while template-only edits preserve compatible state; shared compile/binding artifacts remain future work. | +| Prompt and channel contract | [`ai-prompt.ts`](packages/custom-widgets/src/core/ai-prompt.ts), [`_copy-ai-prompt-button.tsx`](apps/nextjs/src/app/[locale]/manage/custom-widgets/_copy-ai-prompt-button.tsx), and [`assistant-tool-policy.ts`](apps/nextjs/src/app/api/assistant/chat/assistant-tool-policy.ts) | Clipboard, Assistant, and MCP contracts are now separate and trust-bounded; authoritative session/fingerprint/evidence context remains the next seam. | | Assistant dynamic context | [`custom-widget-authoring-context.ts`](apps/nextjs/src/app/api/assistant/chat/custom-widget-authoring-context.ts) | Adapter point for session/fingerprint/diagnostic/evidence context. | | Exact-tested creation | [`creation-procedures.ts`](packages/api/src/router/custom-widget/creation-procedures.ts), especially 53–109 | Existing invariant to preserve and generalize into evidence-bound workbench save/update. | | Preview session authority | [`preview-sessions.ts`](packages/custom-widgets/src/server/preview-sessions.ts) | Existing short-lived preview state; its internal update protection should not become a local authoring/version model. | diff --git a/apps/nextjs/src/app/[locale]/manage/custom-widgets/_custom-widget-form-utils.spec.ts b/apps/nextjs/src/app/[locale]/manage/custom-widgets/_custom-widget-form-utils.spec.ts index 0d0e9b779..6d0e652e6 100644 --- a/apps/nextjs/src/app/[locale]/manage/custom-widgets/_custom-widget-form-utils.spec.ts +++ b/apps/nextjs/src/app/[locale]/manage/custom-widgets/_custom-widget-form-utils.spec.ts @@ -84,13 +84,6 @@ describe("Custom Widget workbench preview options", () => { it("normalizes only changed secrets", () => { expect( getChangedSecrets({ - name: "Widget", - description: "", - iconUrl: "", - sources: "{}", - requests: "{}", - options: "{}", - template: "", secrets: [ { sourceId: "default", kind: "apiKey", value: "", hasValue: true }, { sourceId: "other", kind: "password", value: "replacement" }, diff --git a/packages/custom-widgets/src/jsx/runtime-components.tsx b/packages/custom-widgets/src/jsx/runtime-components.tsx index c47d94bb3..db7a5a101 100644 --- a/packages/custom-widgets/src/jsx/runtime-components.tsx +++ b/packages/custom-widgets/src/jsx/runtime-components.tsx @@ -108,13 +108,17 @@ function useBoundProps(componentName: string, props: Record): R const isBound = Boolean(binding && context && inputType && customJsxBindableComponentNames.has(componentName)); const registerInput = context?.registerInput; useEffect(() => { - if (!isBound || !binding || !registerInput || !inputType) return; + if (!isBound || !binding || !registerInput || !inputType || serializedInitialValue === undefined) return; return registerInput(binding, inputType, JSON.parse(serializedInitialValue) as WidgetInputValue); }, [binding, inputType, isBound, registerInput, serializedInitialValue]); if (!isBound || !binding || !context || !inputType) return sanitized; delete sanitized.defaultValue; delete sanitized.defaultChecked; - const currentValue = context.inputs[binding] ?? initialValue; + let currentValue = initialValue; + if (context.inputTypes[binding] === inputType && Object.hasOwn(context.inputs, binding)) { + const storedValue = context.inputs[binding]; + if (storedValue !== undefined) currentValue = storedValue; + } const update = (value: unknown, checked = false) => { const extracted = extractEventValue(value, checked); diff --git a/packages/custom-widgets/src/runtime/custom-jsx-renderer.tsx b/packages/custom-widgets/src/runtime/custom-jsx-renderer.tsx index c91d13344..71a93fa17 100644 --- a/packages/custom-widgets/src/runtime/custom-jsx-renderer.tsx +++ b/packages/custom-widgets/src/runtime/custom-jsx-renderer.tsx @@ -24,6 +24,7 @@ const permissions = new Set(["view", "modify", "full"]); export interface CustomJsxRendererMessages { noTemplate: string; templateWarnings(count: number): string; + bindingTypeConflict(name: string, firstType: WidgetInputType, secondType: WidgetInputType): string; } export interface CustomJsxRendererProps { @@ -146,6 +147,7 @@ function CustomJsxRendererSession({ const registrations = useRef(new Map()); const registrationVersion = useRef(0); const mounted = useRef(true); + const bindingTypeConflict = messages.bindingTypeConflict; useEffect(() => { mounted.current = true; return () => { @@ -163,7 +165,7 @@ function CustomJsxRendererSession({ continue; } if (existing.type === registration.type) continue; - const message = `BINDING_TYPE_CONFLICT: '${registration.name}' is bound as both ${existing.type} and ${registration.type}`; + const message = bindingTypeConflict(registration.name, existing.type, registration.type); if (!conflicts.includes(message) && conflicts.length < 5) conflicts.push(message); } setBindingErrors((current) => (sameStringArray(current, conflicts) ? current : conflicts)); @@ -172,15 +174,19 @@ function CustomJsxRendererSession({ for (const [name, registration] of active) nextTypes[name] = registration.type; const nextValues: Record = {}; for (const [name, registration] of active) { - nextValues[name] = - current.types[name] === registration.type && Object.hasOwn(current.values, name) - ? current.values[name] - : registration.initialValue; + if (current.types[name] === registration.type && Object.hasOwn(current.values, name)) { + const previousValue = current.values[name]; + if (previousValue !== undefined) { + nextValues[name] = previousValue; + continue; + } + } + nextValues[name] = registration.initialValue; } if (sameTypeRecord(current.types, nextTypes) && sameInputRecord(current.values, nextValues)) return current; return { values: nextValues, types: nextTypes }; }); - }, []); + }, [bindingTypeConflict]); const registerInput = useCallback( (name: string, type: WidgetInputType, initialValue: WidgetInputValue) => { const id = Symbol(name); diff --git a/packages/custom-widgets/src/test/runtime.spec.tsx b/packages/custom-widgets/src/test/runtime.spec.tsx index d4bf5e765..254497531 100644 --- a/packages/custom-widgets/src/test/runtime.spec.tsx +++ b/packages/custom-widgets/src/test/runtime.spec.tsx @@ -12,6 +12,7 @@ import { ActionButton, CustomJsxRenderer, CustomWidgetRuntimeProvider, SubFetch, import { createCustomJsxComponents } from "../jsx"; import { MAX_REFRESH_INTERVAL_MS, MAX_REFRESH_INTERVAL_SECONDS, normalizeRefreshInterval } from "../runtime/sub-fetch"; import type { + CustomJsxRendererMessages, CustomJsxRequestCapability, CustomWidgetPublishedQueryState, CustomWidgetRequestResult, @@ -36,6 +37,13 @@ const messages: CustomWidgetRuntimeMessages = { refresh: "Refresh", }; +const rendererMessages = { + noTemplate: "No template", + templateWarnings: (count: number) => `${count} warnings`, + bindingTypeConflict: (name: string, firstType: string, secondType: string) => + `Input ${name} conflicts between ${firstType} and ${secondType}`, +} satisfies CustomJsxRendererMessages; + let root: Root; let host: HTMLDivElement; @@ -125,7 +133,7 @@ describe("Custom Widget runtime ports", () => { createBindings={() => { throw new RangeError("Response data exceeded the depth limit"); }} - messages={{ noTemplate: "No template", templateWarnings: (count) => `${count} warnings` }} + messages={rendererMessages} />, createPort(), ); @@ -147,7 +155,7 @@ describe("Custom Widget runtime ports", () => { data={{}} components={components} createBindings={() => ({})} - messages={{ noTemplate: "No template", templateWarnings: (count) => `${count} warnings` }} + messages={rendererMessages} />, createPort(), ); @@ -159,11 +167,6 @@ describe("Custom Widget runtime ports", () => { TablerIcon: (() => null) as never, copyLabels: { copy: "Copy", copied: "Copied" }, }); - const rendererMessages = { - noTemplate: "No template", - templateWarnings: (count: number) => `${count} warnings`, - }; - await render( Unsafe"} @@ -183,11 +186,6 @@ describe("Custom Widget runtime ports", () => { TablerIcon: (() => null) as never, copyLabels: { copy: "Copy", copied: "Copied" }, }); - const rendererMessages = { - noTemplate: "No template", - templateWarnings: (count: number) => `${count} warnings`, - }; - await render( <> @@ -217,10 +215,6 @@ describe("Custom Widget runtime ports", () => { it("exposes temporary bindings through inputs without browser storage", async () => { const setItem = vi.spyOn(Storage.prototype, "setItem"); - const rendererMessages = { - noTemplate: "No template", - templateWarnings: (count: number) => `${count} warnings`, - }; const components = createCustomJsxComponents({ TablerIcon: (() => null) as never, copyLabels: { copy: "Copy", copied: "Copied" }, @@ -252,7 +246,7 @@ describe("Custom Widget runtime ports", () => { data={{}} components={components} createBindings={() => ({})} - messages={{ noTemplate: "No template", templateWarnings: (count) => `${count} warnings` }} + messages={rendererMessages} />, createPort(), ); @@ -273,10 +267,6 @@ describe("Custom Widget runtime ports", () => { }); it("resets temporary bindings when the template changes", async () => { - const rendererMessages = { - noTemplate: "No template", - templateWarnings: (count: number) => `${count} warnings`, - }; const components = createCustomJsxComponents({ TablerIcon: (() => null) as never, copyLabels: { copy: "Copy", copied: "Copied" }, @@ -313,10 +303,6 @@ describe("Custom Widget runtime ports", () => { }); it("preserves temporary bindings when data changes without changing the template", async () => { - const rendererMessages = { - noTemplate: "No template", - templateWarnings: (count: number) => `${count} warnings`, - }; const components = createCustomJsxComponents({ TablerIcon: (() => null) as never, copyLabels: { copy: "Copy", copied: "Copied" }, diff --git a/packages/custom-widgets/src/workbench/code-editor.module.css b/packages/custom-widgets/src/workbench/code-editor.module.css index 8a9c8d30d..62678dfed 100644 --- a/packages/custom-widgets/src/workbench/code-editor.module.css +++ b/packages/custom-widgets/src/workbench/code-editor.module.css @@ -28,7 +28,7 @@ color: light-dark(var(--mantine-color-gray-7), var(--mantine-color-gray-4)); } .accessibleMuted[data-invalid="true"] { - color: var(--mantine-color-red-6); + color: light-dark(var(--mantine-color-red-9), var(--mantine-color-red-4)); } .viewport { max-height: min(62vh, 680px); diff --git a/packages/custom-widgets/src/workbench/code-editor.tsx b/packages/custom-widgets/src/workbench/code-editor.tsx index c311b5f9f..71ae99a90 100644 --- a/packages/custom-widgets/src/workbench/code-editor.tsx +++ b/packages/custom-widgets/src/workbench/code-editor.tsx @@ -104,6 +104,7 @@ export function CustomWidgetCodeEditor(props: CustomWidgetCodeEditorProps) { const formattedValue = useMemo(() => formatCode(props.value, props.language), [props.language, props.value]); const errorCount = diagnostics.filter(({ severity }) => severity === "error").length; const warningCount = diagnostics.length - errorCount; + const isCharacterLimitExceeded = props.maxLength !== undefined && props.value.length > props.maxLength; const copy = async () => { try { await navigator.clipboard.writeText(props.value); @@ -175,6 +176,7 @@ export function CustomWidgetCodeEditor(props: CustomWidgetCodeEditorProps) { label={props.label} placeholder={props.placeholder} theme={colorScheme} + invalid={isCharacterLimitExceeded} height={props.height ?? (props.language === "jsx" ? "320px" : "220px")} readOnly={props.readOnly ?? false} diagnosticMessage={props.messages.diagnostic} @@ -211,9 +213,11 @@ export function CustomWidgetCodeEditor(props: CustomWidgetCodeEditorProps) { props.maxLength)} + data-invalid={isCharacterLimitExceeded} + aria-live="polite" > {props.messages.characters(props.value.length, props.maxLength)} diff --git a/packages/custom-widgets/src/workbench/direct-code-mirror.tsx b/packages/custom-widgets/src/workbench/direct-code-mirror.tsx index 352ffbfa6..1fc8e736d 100644 --- a/packages/custom-widgets/src/workbench/direct-code-mirror.tsx +++ b/packages/custom-widgets/src/workbench/direct-code-mirror.tsx @@ -29,6 +29,7 @@ interface DirectCodeMirrorProps { placeholder?: string; height: string; theme: "light" | "dark"; + invalid: boolean; readOnly: boolean; diagnosticMessage(diagnostic: EditorDiagnostic): string; onChange(value: string): void; @@ -124,9 +125,10 @@ export default function DirectCodeMirror(props: DirectCodeMirrorProps) { tabindex: "0", "aria-label": props.label, "aria-labelledby": props.labelledBy, + "aria-invalid": props.invalid.toString(), }), ); - }, [compartments.accessibility, instanceId, props.id, props.label, props.labelledBy]); + }, [compartments.accessibility, instanceId, props.id, props.invalid, props.label, props.labelledBy]); useEffect(() => { reconfigure(instanceId, compartments.editability, createEditabilityExtension(props.readOnly)); }, [compartments.editability, instanceId, props.readOnly]); @@ -166,6 +168,7 @@ function createExtensions(instanceId: string, props: DirectCodeMirrorProps, comp tabindex: "0", "aria-label": props.label, "aria-labelledby": props.labelledBy, + "aria-invalid": props.invalid.toString(), }), ), compartments.editability.of(createEditabilityExtension(props.readOnly)), diff --git a/packages/translation/src/lang/en.json b/packages/translation/src/lang/en.json index 2652ca33d..ed10a80bb 100644 --- a/packages/translation/src/lang/en.json +++ b/packages/translation/src/lang/en.json @@ -851,6 +851,7 @@ "line": "Line {line, number}", "ast": "{value}", "bindingTypeConflict": "The input \"{value}\" is shared by controls with incompatible value types.", + "runtimeBindingTypeConflict": "The input \"{value}\" is bound as both {firstType} and {secondType}.", "dynamicRequestId": "A literal requestId is required. Dynamic request IDs are not accepted.", "dynamicInputBinding": "A literal input name is required for bind.", "inlineRequestProps": "Network components accept requestId and params; inline URL, method, header, and body props are blocked.", diff --git a/packages/widgets/src/custom-api/custom-jsx-display.tsx b/packages/widgets/src/custom-api/custom-jsx-display.tsx index bbf9ce77e..a957b9f8a 100644 --- a/packages/widgets/src/custom-api/custom-jsx-display.tsx +++ b/packages/widgets/src/custom-api/custom-jsx-display.tsx @@ -3,7 +3,7 @@ import { useCallback, useMemo, useState } from "react"; import { useI18n } from "@homarr/translation/client"; -import type { CustomWidgetPublishedQueryState } from "@homarr/custom-widgets/runtime"; +import type { CustomJsxRendererMessages, CustomWidgetPublishedQueryState } from "@homarr/custom-widgets/runtime"; import { CustomJsxRenderer, parseRequestCapabilities } from "@homarr/custom-widgets/runtime"; import { createCustomWidgetComponents, SAFE_BINDINGS } from "./jsx-components"; @@ -21,6 +21,7 @@ interface ScopedQueryState { export default function CustomJsxDisplay({ data }: { data: Record }) { const t = useI18n("widget.customApi.customJsx"); const actionT = useI18n("common.action"); + const diagnosticsT = useI18n("customWidget.editor.diagnostics"); const capabilities = useMemo(() => parseRequestCapabilities(data.requestCapabilities), [data.requestCapabilities]); const copyLabel = actionT("copy"); const copiedLabel = t("copied"); @@ -28,6 +29,10 @@ export default function CustomJsxDisplay({ data }: { data: Record createCustomWidgetComponents({ copy: copyLabel, copied: copiedLabel }), [copiedLabel, copyLabel], ); + const bindingTypeConflict = useCallback( + (name, firstType, secondType) => diagnosticsT("runtimeBindingTypeConflict", { value: name, firstType, secondType }), + [diagnosticsT], + ); const definitionId = typeof data.widgetDefinitionId === "string" ? data.widgetDefinitionId : undefined; const itemId = typeof data.widgetItemId === "string" ? data.widgetItemId : undefined; const previewSessionId = typeof data.previewSessionId === "string" ? data.previewSessionId : undefined; @@ -77,6 +82,7 @@ export default function CustomJsxDisplay({ data }: { data: Record t("templateWarnings", { count: String(count) }), + bindingTypeConflict, }} /> );