From fc317840a79d0aea7f5de7ff27f9a25a537a814e Mon Sep 17 00:00:00 2001 From: Meier Lukas Date: Tue, 17 Sep 2024 19:30:14 +0200 Subject: [PATCH] refactor: use suspense query instead of serverdata for app widget (#1143) * refactor: use suspense query instead of serverdata for app widget * chore: add missing translation for loading tooltip --- packages/translation/src/lang/en.ts | 1 + packages/widgets/src/app/component.tsx | 118 ++++------------ packages/widgets/src/app/index.ts | 13 +- packages/widgets/src/app/ping/ping-dot.tsx | 24 ++++ .../widgets/src/app/ping/ping-indicator.tsx | 41 ++++++ packages/widgets/src/app/serverData.ts | 28 ---- .../widgets/src/app/test/serverData.spec.ts | 132 ------------------ packages/widgets/src/definition.ts | 1 + packages/widgets/src/errors/component.tsx | 30 ++-- packages/widgets/src/index.tsx | 8 +- packages/widgets/src/server/runner.tsx | 2 +- 11 files changed, 129 insertions(+), 269 deletions(-) create mode 100644 packages/widgets/src/app/ping/ping-dot.tsx create mode 100644 packages/widgets/src/app/ping/ping-indicator.tsx delete mode 100644 packages/widgets/src/app/serverData.ts delete mode 100644 packages/widgets/src/app/test/serverData.spec.ts diff --git a/packages/translation/src/lang/en.ts b/packages/translation/src/lang/en.ts index b22979d42..f16ece370 100644 --- a/packages/translation/src/lang/en.ts +++ b/packages/translation/src/lang/en.ts @@ -556,6 +556,7 @@ export default { next: "Next", checkoutDocs: "Check out the documentation", tryAgain: "Try again", + loading: "Loading", }, iconPicker: { label: "Icon URL", diff --git a/packages/widgets/src/app/component.tsx b/packages/widgets/src/app/component.tsx index cf9422832..36472e715 100644 --- a/packages/widgets/src/app/component.tsx +++ b/packages/widgets/src/app/component.tsx @@ -1,58 +1,37 @@ "use client"; import type { PropsWithChildren } from "react"; -import { useState } from "react"; -import { Box, Center, Flex, Loader, Stack, Text, Tooltip, UnstyledButton } from "@mantine/core"; -import { IconDeviceDesktopX } from "@tabler/icons-react"; +import { Suspense } from "react"; +import { Flex, Text, Tooltip, UnstyledButton } from "@mantine/core"; import combineClasses from "clsx"; -import type { RouterOutputs } from "@homarr/api"; import { clientApi } from "@homarr/api/client"; import { parseAppHrefWithVariablesClient } from "@homarr/common/client"; import { useRegisterSpotlightActions } from "@homarr/spotlight"; -import { useScopedI18n } from "@homarr/translation/client"; +import { useI18n } from "@homarr/translation/client"; import type { WidgetComponentProps } from "../definition"; import classes from "./app.module.css"; +import { PingDot } from "./ping/ping-dot"; +import { PingIndicator } from "./ping/ping-indicator"; -export default function AppWidget({ options, serverData, isEditMode, width }: WidgetComponentProps<"app">) { - const t = useScopedI18n("widget.app"); - const isQueryEnabled = Boolean(options.appId); - const { - data: app, - isPending, - isError, - } = clientApi.app.byId.useQuery( +export default function AppWidget({ options, isEditMode }: WidgetComponentProps<"app">) { + const t = useI18n(); + const [app] = clientApi.app.byId.useSuspenseQuery( { id: options.appId, }, { - initialData: - // We need to check if the id's match because otherwise the same initialData for a changed id will be used - serverData?.app?.id === options.appId ? serverData.app : undefined, refetchOnMount: false, refetchOnWindowFocus: false, refetchOnReconnect: false, - enabled: isQueryEnabled, - }, - ); - - const [pingResult, setPingResult] = useState(null); - - const shouldRunPing = Boolean(app?.href) && options.pingEnabled; - clientApi.widget.app.updatedPing.useSubscription( - { url: parseAppHrefWithVariablesClient(app?.href ?? "") }, - { - enabled: shouldRunPing, - onData(data) { - setPingResult(data); - }, + retry: false, }, ); useRegisterSpotlightActions( `app-${options.appId}`, - app?.href + app.href ? [ { id: `app-${options.appId}`, @@ -69,44 +48,21 @@ export default function AppWidget({ options, serverData, isEditMode, width }: Wi [app, options.appId, options.openInNewTab], ); - if (isPending && isQueryEnabled) { - return ( -
- -
- ); - } - - if (isError || !isQueryEnabled) { - return ( - - - = 96 ? "2rem" : "1rem"} /> - {width >= 96 && ( - - {t("error.notFound.label")} - - )} - - - ); - } - return ( {options.showTitle && ( - - {app?.name} + + {app.name} )} - {app?.name} + {app.name} - {shouldRunPing && } + {options.pingEnabled && app.href ? ( + + } + > + + + ) : null} ); } @@ -141,31 +105,3 @@ const AppLink = ({ href, openInNewTab, enabled, children }: PropsWithChildren { - return ( - - - = 500 - ? "red" - : "green", - }} - w={16} - h={16} - > - - - ); -}; diff --git a/packages/widgets/src/app/index.ts b/packages/widgets/src/app/index.ts index cfa202e5c..506f654e5 100644 --- a/packages/widgets/src/app/index.ts +++ b/packages/widgets/src/app/index.ts @@ -1,4 +1,4 @@ -import { IconApps } from "@tabler/icons-react"; +import { IconApps, IconDeviceDesktopX } from "@tabler/icons-react"; import { createWidgetDefinition } from "../definition"; import { optionsBuilder } from "../options"; @@ -12,6 +12,11 @@ export const { definition, componentLoader, serverDataLoader } = createWidgetDef showDescriptionTooltip: factory.switch({ defaultValue: false }), pingEnabled: factory.switch({ defaultValue: false }), })), -}) - .withServerData(() => import("./serverData")) - .withDynamicImport(() => import("./component")); + errors: { + NOT_FOUND: { + icon: IconDeviceDesktopX, + message: (t) => t("widget.app.error.notFound.label"), + hideLogsLink: true, + }, + }, +}).withDynamicImport(() => import("./component")); diff --git a/packages/widgets/src/app/ping/ping-dot.tsx b/packages/widgets/src/app/ping/ping-dot.tsx new file mode 100644 index 000000000..b54262f28 --- /dev/null +++ b/packages/widgets/src/app/ping/ping-dot.tsx @@ -0,0 +1,24 @@ +import type { MantineColor } from "@mantine/core"; +import { Box, Tooltip } from "@mantine/core"; + +interface PingDotProps { + color: MantineColor; + tooltip: string; +} + +export const PingDot = ({ color, tooltip }: PingDotProps) => { + return ( + + + + + + ); +}; diff --git a/packages/widgets/src/app/ping/ping-indicator.tsx b/packages/widgets/src/app/ping/ping-indicator.tsx new file mode 100644 index 000000000..058600257 --- /dev/null +++ b/packages/widgets/src/app/ping/ping-indicator.tsx @@ -0,0 +1,41 @@ +import { useState } from "react"; + +import type { RouterOutputs } from "@homarr/api"; +import { clientApi } from "@homarr/api/client"; +import { parseAppHrefWithVariablesClient } from "@homarr/common/client"; + +import { PingDot } from "./ping-dot"; + +interface PingIndicatorProps { + href: string; +} + +export const PingIndicator = ({ href }: PingIndicatorProps) => { + const [ping] = clientApi.widget.app.ping.useSuspenseQuery( + { + url: parseAppHrefWithVariablesClient(href), + }, + { + refetchOnMount: false, + refetchOnWindowFocus: false, + }, + ); + + const [pingResult, setPingResult] = useState(ping); + + clientApi.widget.app.updatedPing.useSubscription( + { url: parseAppHrefWithVariablesClient(href) }, + { + onData(data) { + setPingResult(data); + }, + }, + ); + + return ( + = 500 ? "red" : "green"} + tooltip={"statusCode" in pingResult ? pingResult.statusCode.toString() : pingResult.error} + /> + ); +}; diff --git a/packages/widgets/src/app/serverData.ts b/packages/widgets/src/app/serverData.ts deleted file mode 100644 index 4b8d7204a..000000000 --- a/packages/widgets/src/app/serverData.ts +++ /dev/null @@ -1,28 +0,0 @@ -"use server"; - -import type { RouterOutputs } from "@homarr/api"; -import { api } from "@homarr/api/server"; -import { parseAppHrefWithVariablesServer } from "@homarr/common/server"; - -import type { WidgetProps } from "../definition"; - -export default async function getServerDataAsync({ options }: WidgetProps<"app">) { - if (!options.appId) { - return { app: null, pingResult: null }; - } - - try { - const app = await api.app.byId({ id: options.appId }); - let pingResult: RouterOutputs["widget"]["app"]["ping"] | null = null; - - if (app.href && options.pingEnabled) { - pingResult = await api.widget.app.ping({ - url: parseAppHrefWithVariablesServer(app.href), - }); - } - - return { app, pingResult }; - } catch { - return { app: null, pingResult: null }; - } -} diff --git a/packages/widgets/src/app/test/serverData.spec.ts b/packages/widgets/src/app/test/serverData.spec.ts deleted file mode 100644 index 999d7d1e8..000000000 --- a/packages/widgets/src/app/test/serverData.spec.ts +++ /dev/null @@ -1,132 +0,0 @@ -import { TRPCError } from "@trpc/server"; -import { describe, expect, test, vi } from "vitest"; - -import type { RouterOutputs } from "@homarr/api"; -import { api } from "@homarr/api/server"; -import { objectKeys } from "@homarr/common"; - -import type { WidgetProps } from "../../definition"; -import getServerDataAsync from "../serverData"; - -const mockApp = (override: Partial) => - ({ - id: "1", - name: "Mock app", - iconUrl: "https://some.com/icon.png", - description: null, - href: "https://google.ch", - ...override, - }) satisfies RouterOutputs["app"]["byId"]; - -vi.mock("@homarr/api/server", () => ({ - api: { - app: { - byId: () => null, - }, - widget: { - app: { - ping: () => null, - }, - }, - }, -})); -vi.mock("@homarr/common/server", () => ({ - parseAppHrefWithVariablesServer: () => "http://localhost", -})); - -describe("getServerDataAsync should load app and ping result", () => { - test("when appId is empty it should return null for app and pingResult", async () => { - // Arrange - const options = { - appId: "", - pingEnabled: true, - }; - - // Act - const result = await getServerDataAsync({ options } as unknown as WidgetProps<"app">); - - // Assert - expect(result.app).toBeNull(); - expect(result.pingResult).toBeNull(); - }); - - test("when app exists and ping is disabled it should return existing app and pingResult null", async () => { - // Arrange - const spy = vi.spyOn(api.app, "byId"); - const options = { - appId: "1", - pingEnabled: false, - }; - const mockedApp = mockApp({}); - spy.mockImplementation(() => Promise.resolve(mockedApp)); - - // Act - const result = await getServerDataAsync({ options } as unknown as WidgetProps<"app">); - - // Assert - expect(result.pingResult).toBeNull(); - objectKeys(mockedApp).forEach((key) => expect(result.app?.[key]).toBe(mockedApp[key])); - }); - - test("when app exists without href and ping enabled it should return existing app and pingResult null", async () => { - // Arrange - const spy = vi.spyOn(api.app, "byId"); - const options = { - appId: "1", - pingEnabled: true, - }; - const mockedApp = mockApp({ href: null }); - spy.mockImplementation(() => Promise.resolve(mockedApp)); - - // Act - const result = await getServerDataAsync({ options } as unknown as WidgetProps<"app">); - - // Assert - expect(result.pingResult).toBeNull(); - objectKeys(mockedApp).forEach((key) => expect(result.app?.[key]).toBe(mockedApp[key])); - }); - - test("when app does not exist it should return for both null", async () => { - // Arrange - const spy = vi.spyOn(api.app, "byId"); - const options = { - appId: "1", - pingEnabled: true, - }; - spy.mockImplementation(() => - Promise.reject( - new TRPCError({ - code: "NOT_FOUND", - }), - ), - ); - - // Act - const result = await getServerDataAsync({ options } as unknown as WidgetProps<"app">); - - // Assert - expect(result.pingResult).toBeNull(); - expect(result.app).toBeNull(); - }); - - test("when app found and ping enabled it should return existing app and pingResult", async () => { - // Arrange - const spyById = vi.spyOn(api.app, "byId"); - const spyPing = vi.spyOn(api.widget.app, "ping"); - const options = { - appId: "1", - pingEnabled: true, - }; - const mockedApp = mockApp({}); - const pingResult = { statusCode: 200, url: "http://localhost" }; - spyById.mockImplementation(() => Promise.resolve(mockedApp)); - spyPing.mockImplementation(() => Promise.resolve(pingResult)); - - // Act - const result = await getServerDataAsync({ options } as unknown as WidgetProps<"app">); - - // Assert - expect(result.pingResult).toBe(pingResult); - expect(result.app).toBe(mockedApp); - }); -}); diff --git a/packages/widgets/src/definition.ts b/packages/widgets/src/definition.ts index 9bb36b275..46346bddb 100644 --- a/packages/widgets/src/definition.ts +++ b/packages/widgets/src/definition.ts @@ -71,6 +71,7 @@ export interface WidgetDefinition { { icon: TablerIcon; message: stringOrTranslation; + hideLogsLink?: boolean; } > >; diff --git a/packages/widgets/src/errors/component.tsx b/packages/widgets/src/errors/component.tsx index b27b52875..5638ef321 100644 --- a/packages/widgets/src/errors/component.tsx +++ b/packages/widgets/src/errors/component.tsx @@ -5,6 +5,7 @@ import type { DefaultErrorData } from "@trpc/server/unstable-core-do-not-import" import type { WidgetKind } from "@homarr/definitions"; +import type { WidgetDefinition } from ".."; import { widgetImports } from ".."; import { ErrorBoundaryError } from "./base"; import { BaseWidgetError } from "./base-component"; @@ -22,21 +23,28 @@ export const WidgetError = ({ error, resetErrorBoundary, kind }: WidgetErrorProp return ; } - if (error instanceof TRPCClientError && "code" in error.data) { - const errorData = error.data as DefaultErrorData; - - if (!("errors" in currentDefinition && errorData.code in currentDefinition.errors)) return null; - - const errorDefinition = currentDefinition.errors[errorData.code as keyof typeof currentDefinition.errors]; - - return ; - } - - return ( + const commonFallbackError = ( string }).toString()} onRetry={resetErrorBoundary} /> ); + + if (error instanceof TRPCClientError && "code" in error.data) { + const errorData = error.data as DefaultErrorData; + + if (!("errors" in currentDefinition)) return commonFallbackError; + + const errors: Exclude = currentDefinition.errors; + const errorDefinition = errors[errorData.code]; + + if (!errorDefinition) return commonFallbackError; + + return ( + + ); + } + + return commonFallbackError; }; diff --git a/packages/widgets/src/index.tsx b/packages/widgets/src/index.tsx index 4c1b04b8c..842f201ba 100644 --- a/packages/widgets/src/index.tsx +++ b/packages/widgets/src/index.tsx @@ -1,7 +1,7 @@ import type { ComponentType } from "react"; import type { Loader } from "next/dynamic"; import dynamic from "next/dynamic"; -import { Loader as UiLoader } from "@mantine/core"; +import { Center, Loader as UiLoader } from "@mantine/core"; import type { WidgetKind } from "@homarr/definitions"; @@ -65,7 +65,11 @@ export const loadWidgetDynamic = (kind: TKind) => { const newlyLoadedComponent = dynamic>( widgetImports[kind].componentLoader as Loader>, { - loading: () => , + loading: () => ( +
+ +
+ ), }, ); diff --git a/packages/widgets/src/server/runner.tsx b/packages/widgets/src/server/runner.tsx index 53f960cfe..91effec0f 100644 --- a/packages/widgets/src/server/runner.tsx +++ b/packages/widgets/src/server/runner.tsx @@ -37,7 +37,7 @@ interface ItemDataLoaderProps { const ItemDataLoader = async ({ item }: ItemDataLoaderProps) => { const widgetImport = widgetImports[item.kind]; - if (!("serverDataLoader" in widgetImport)) { + if (!("serverDataLoader" in widgetImport) || !widgetImport.serverDataLoader) { return ; } const loader = await widgetImport.serverDataLoader();