fix: address widget reload review findings
This commit is contained in:
10 files changed
+206
-29
No files matched your search
@@ -40,7 +40,13 @@ const BoardSelectionGridProvider = ({ children }: PropsWithChildren) => (
|
||||
</GridEditorRegistryProvider>
|
||||
);
|
||||
|
||||
export const ClientBoard = ({ initialTimestamp }: { initialTimestamp: number }) => {
|
||||
export const ClientBoard = ({
|
||||
initialTimestamp,
|
||||
initialTimeZone,
|
||||
}: {
|
||||
initialTimestamp: number;
|
||||
initialTimeZone: string;
|
||||
}) => {
|
||||
const board = useRequiredBoard();
|
||||
const t = useI18n("board.landmark");
|
||||
const tPreview = useI18n("board.setting.section.layout.preview");
|
||||
@@ -157,11 +163,15 @@ export const ClientBoard = ({ initialTimestamp }: { initialTimestamp: number })
|
||||
);
|
||||
|
||||
if (representativeWidth === null) {
|
||||
return <WidgetTimeProvider initialTimestamp={initialTimestamp}>{content}</WidgetTimeProvider>;
|
||||
return (
|
||||
<WidgetTimeProvider initialTimestamp={initialTimestamp} initialTimeZone={initialTimeZone}>
|
||||
{content}
|
||||
</WidgetTimeProvider>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<WidgetTimeProvider initialTimestamp={initialTimestamp}>
|
||||
<WidgetTimeProvider initialTimestamp={initialTimestamp} initialTimeZone={initialTimeZone}>
|
||||
<Stack align="center" gap="xs" p="md" mih="100%">
|
||||
<Text size="xs" c="dimmed" fw={500}>
|
||||
{tPreview("editorWidthLabel", { layoutName: currentLayout.name, width: representativeWidth })}
|
||||
|
||||
@@ -96,7 +96,10 @@ export const createBoardContentPage = <TParams extends Record<string, unknown>>(
|
||||
return (
|
||||
<HydrationBoundary state={dehydrate(queryClient)}>
|
||||
<IntegrationProvider integrations={integrations}>
|
||||
<ClientBoard initialTimestamp={Date.now()} />
|
||||
<ClientBoard
|
||||
initialTimestamp={Date.now()}
|
||||
initialTimeZone={Intl.DateTimeFormat().resolvedOptions().timeZone || "UTC"}
|
||||
/>
|
||||
</IntegrationProvider>
|
||||
</HydrationBoundary>
|
||||
);
|
||||
|
||||
@@ -4,6 +4,8 @@ import { Suspense, use, useMemo } from "react";
|
||||
import type { ToolCallMessagePartProps } from "@assistant-ui/react";
|
||||
import { Alert, Box, Button, Group, Skeleton, Stack, Text, ThemeIcon } from "@mantine/core";
|
||||
import { IconAlertTriangle, IconCheck, IconSettings } from "@tabler/icons-react";
|
||||
import { ErrorBoundary } from "react-error-boundary";
|
||||
import type { FallbackProps } from "react-error-boundary";
|
||||
|
||||
import { clientApi } from "@homarr/api/client";
|
||||
import { useSession } from "@homarr/auth/client";
|
||||
@@ -73,9 +75,27 @@ export const AssistantConfigureWidgetTool = (props: AssistantConfigureWidgetTool
|
||||
if (!completeArgs) return <AssistantConfigureWidgetToolContent {...props} definition={undefined} />;
|
||||
|
||||
return (
|
||||
<Suspense fallback={<AssistantConfigureWidgetSkeleton />}>
|
||||
<LoadedAssistantConfigureWidgetTool {...props} args={completeArgs} />
|
||||
</Suspense>
|
||||
<ErrorBoundary FallbackComponent={AssistantWidgetDefinitionErrorFallback} resetKeys={[completeArgs.kind]}>
|
||||
<Suspense fallback={<AssistantConfigureWidgetSkeleton />}>
|
||||
<LoadedAssistantConfigureWidgetTool {...props} args={completeArgs} />
|
||||
</Suspense>
|
||||
</ErrorBoundary>
|
||||
);
|
||||
};
|
||||
|
||||
const AssistantWidgetDefinitionErrorFallback = ({ resetErrorBoundary }: FallbackProps) => {
|
||||
const t = useI18n("assistant.configureWidget");
|
||||
const actionT = useI18n("common.action");
|
||||
|
||||
return (
|
||||
<Alert color="red" variant="light" title={t("definitionLoadErrorTitle")} icon={<IconAlertTriangle size={18} />}>
|
||||
<Stack gap="sm">
|
||||
<Text size="sm">{t("definitionLoadErrorDescription")}</Text>
|
||||
<Button variant="light" color="red" size="compact-sm" w="fit-content" onClick={resetErrorBoundary}>
|
||||
{actionT("tryAgain")}
|
||||
</Button>
|
||||
</Stack>
|
||||
</Alert>
|
||||
);
|
||||
};
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { readdirSync, readFileSync } from "node:fs";
|
||||
import { dirname, join } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { parse } from "@babel/parser";
|
||||
import { describe, expect, test } from "vitest";
|
||||
|
||||
const sourceRoot = join(dirname(fileURLToPath(import.meta.url)), "../..");
|
||||
@@ -13,18 +14,105 @@ const collectRuntimeSourceFiles = (directory: string): string[] =>
|
||||
return [path];
|
||||
});
|
||||
|
||||
const rootWidgetsModule = "@homarr/widgets";
|
||||
|
||||
interface AstNode {
|
||||
type: string;
|
||||
[key: string]: unknown;
|
||||
}
|
||||
|
||||
const isAstNode = (value: unknown): value is AstNode =>
|
||||
typeof value === "object" && value !== null && "type" in value && typeof value.type === "string";
|
||||
|
||||
const getAstNode = (value: unknown) => (isAstNode(value) ? value : undefined);
|
||||
const getAstNodes = (value: unknown) => (Array.isArray(value) ? value.filter(isAstNode) : []);
|
||||
|
||||
const isRootWidgetsModule = (node: unknown) => {
|
||||
const astNode = getAstNode(node);
|
||||
return astNode?.type === "StringLiteral" && astNode.value === rootWidgetsModule;
|
||||
};
|
||||
|
||||
const hasRuntimeRootWidgetImport = (source: string, fileName: string): boolean => {
|
||||
const sourceFile = parse(source, {
|
||||
sourceFilename: fileName,
|
||||
sourceType: "unambiguous",
|
||||
plugins: ["typescript", "jsx", "decorators-legacy", "importAttributes"],
|
||||
});
|
||||
let found = false;
|
||||
|
||||
const visit = (node: AstNode) => {
|
||||
if (found) return;
|
||||
|
||||
if (node.type === "ImportDeclaration" && isRootWidgetsModule(node.source)) {
|
||||
const specifiers = getAstNodes(node.specifiers);
|
||||
if (
|
||||
node.importKind !== "type" &&
|
||||
(specifiers.length === 0 ||
|
||||
specifiers.some((specifier) => specifier.type !== "ImportSpecifier" || specifier.importKind !== "type"))
|
||||
) {
|
||||
found = true;
|
||||
}
|
||||
} else if (
|
||||
(node.type === "ExportNamedDeclaration" || node.type === "ExportAllDeclaration") &&
|
||||
isRootWidgetsModule(node.source)
|
||||
) {
|
||||
const specifiers = getAstNodes(node.specifiers);
|
||||
if (
|
||||
node.exportKind !== "type" &&
|
||||
(node.type === "ExportAllDeclaration" ||
|
||||
specifiers.length === 0 ||
|
||||
specifiers.some((specifier) => specifier.exportKind !== "type"))
|
||||
) {
|
||||
found = true;
|
||||
}
|
||||
} else if (node.type === "TSImportEqualsDeclaration" && node.importKind !== "type") {
|
||||
const moduleReference = getAstNode(node.moduleReference);
|
||||
if (moduleReference?.type === "TSExternalModuleReference" && isRootWidgetsModule(moduleReference.expression)) {
|
||||
found = true;
|
||||
}
|
||||
} else if (node.type === "CallExpression") {
|
||||
const callee = getAstNode(node.callee);
|
||||
const argument = getAstNodes(node.arguments)[0];
|
||||
if (
|
||||
isRootWidgetsModule(argument) &&
|
||||
(callee?.type === "Import" || (callee?.type === "Identifier" && callee.name === "require"))
|
||||
) {
|
||||
found = true;
|
||||
}
|
||||
}
|
||||
|
||||
for (const value of Object.values(node)) {
|
||||
if (isAstNode(value)) visit(value);
|
||||
else if (Array.isArray(value)) value.filter(isAstNode).forEach(visit);
|
||||
}
|
||||
};
|
||||
|
||||
visit(sourceFile as unknown as AstNode);
|
||||
return found;
|
||||
};
|
||||
|
||||
describe("widget import boundary", () => {
|
||||
test("application runtime code uses narrow widget exports", () => {
|
||||
const offenders = collectRuntimeSourceFiles(sourceRoot).filter((path) => {
|
||||
const source = readFileSync(path, "utf8");
|
||||
const importStatements = source.match(/import[\s\S]*?;/g) ?? [];
|
||||
return (
|
||||
importStatements.some(
|
||||
(statement) => !/^import\s+type\b/.test(statement) && /\sfrom\s+["']@homarr\/widgets["']/.test(statement),
|
||||
) || /import\(["']@homarr\/widgets["']\)/.test(source)
|
||||
);
|
||||
return hasRuntimeRootWidgetImport(source, path);
|
||||
});
|
||||
|
||||
expect(offenders).toEqual([]);
|
||||
});
|
||||
|
||||
test.each([
|
||||
['import type { WidgetDefinition } from "@homarr/widgets";', false],
|
||||
['import { type WidgetDefinition } from "@homarr/widgets";', false],
|
||||
['export type { WidgetDefinition } from "@homarr/widgets";', false],
|
||||
["const example = 'import { widgets } from \"@homarr/widgets\";';", false],
|
||||
['// import { widgets } from "@homarr/widgets";', false],
|
||||
['import { widgets } from "@homarr/widgets";', true],
|
||||
['import { type WidgetDefinition, widgets } from "@homarr/widgets";', true],
|
||||
['export * from "@homarr/widgets";', true],
|
||||
['import /* chunk */ ( /* source */ "@homarr/widgets" );', true],
|
||||
['require("@homarr/widgets");', true],
|
||||
])("detects runtime root imports in %s", (source, expected) => {
|
||||
expect(hasRuntimeRootWidgetImport(source, "fixture.ts")).toBe(expected);
|
||||
});
|
||||
});
|
||||
@@ -108,7 +108,13 @@ describe("lazy widget application graph", () => {
|
||||
const context = await browser.newContext({ serviceWorkers: "block" });
|
||||
const page = await context.newPage();
|
||||
const pageErrors: Error[] = [];
|
||||
const hydrationErrors: string[] = [];
|
||||
page.on("pageerror", (error) => pageErrors.push(error));
|
||||
page.on("console", (message) => {
|
||||
if (message.type() === "error" && /hydration|did not match/i.test(message.text())) {
|
||||
hydrationErrors.push(message.text());
|
||||
}
|
||||
});
|
||||
let releaseRefresh: (() => void) | undefined;
|
||||
|
||||
try {
|
||||
@@ -177,6 +183,7 @@ describe("lazy widget application graph", () => {
|
||||
await expect(downloadsWidget.locator('output[aria-live="polite"]')).toHaveCount(0);
|
||||
await page.unroute(downloadsRequestPattern, delayDownloadsRefresh);
|
||||
expect(pageErrors).toEqual([]);
|
||||
expect(hydrationErrors).toEqual([]);
|
||||
|
||||
const serviceWorkerContext = await browser.newContext();
|
||||
const serviceWorkerPage = await serviceWorkerContext.newPage();
|
||||
|
||||
@@ -7771,6 +7771,8 @@
|
||||
"ready": "Widget configuration ready",
|
||||
"cancelled": "Widget was not added",
|
||||
"automaticContinue": "Accepting the proposed widget configuration automatically",
|
||||
"definitionLoadErrorTitle": "Widget settings could not be loaded",
|
||||
"definitionLoadErrorDescription": "Homarr could not load this widget's settings. Try again.",
|
||||
"loadErrorTitle": "Integrations could not be loaded",
|
||||
"loadErrorDescription": "Homarr could not load the integrations available for this widget.",
|
||||
"noIntegrationTitle": "A compatible integration is required",
|
||||
|
||||
@@ -4,6 +4,7 @@ import { Box, Center, Stack, Text, Title } from "@mantine/core";
|
||||
import dayjs from "dayjs";
|
||||
import advancedFormat from "dayjs/plugin/advancedFormat";
|
||||
|
||||
import { useWidgetLocalTimeZone } from "../common/widget-time-context";
|
||||
import type { WidgetComponentProps } from "../definition";
|
||||
import { AdvancedClockView } from "./advanced-view";
|
||||
import { clockTimeFormatShowsSeconds, resolveClockTimeFormat } from "./format";
|
||||
@@ -17,7 +18,10 @@ export default function ClockWidget({ options, width, height, displayMode }: Wid
|
||||
const isAdvanced = displayMode === "advanced";
|
||||
const showSeconds = clockTimeFormatShowsSeconds(options.customTimeFormat);
|
||||
const time = useCurrentTime({ showSeconds });
|
||||
const requestedTimeZone = options.useCustomTimezone ? options.timezone : getResolvedLocalTimeZone();
|
||||
const initialLocalTimeZone = useWidgetLocalTimeZone();
|
||||
const requestedTimeZone = options.useCustomTimezone
|
||||
? options.timezone
|
||||
: (initialLocalTimeZone ?? getResolvedLocalTimeZone());
|
||||
const primaryTimeZoneInvalid = !isTimeZoneSupported(requestedTimeZone);
|
||||
const primaryTimeZone = primaryTimeZoneInvalid ? "UTC" : requestedTimeZone;
|
||||
const zonedTime = time === null ? null : dayjs(time).tz(primaryTimeZone);
|
||||
|
||||
@@ -6,9 +6,13 @@ import { renderToString } from "react-dom/server";
|
||||
import { afterEach, describe, expect, test, vi } from "vitest";
|
||||
|
||||
import { useWidgetNow } from "./use-widget-now";
|
||||
import { WidgetTimeProvider } from "./widget-time-context";
|
||||
import { useWidgetLocalTimeZone, WidgetTimeProvider } from "./widget-time-context";
|
||||
|
||||
const TimeProbe = () => <time>{useWidgetNow("minute")?.toISOString() ?? "missing"}</time>;
|
||||
const TimeProbe = () => (
|
||||
<time>
|
||||
{useWidgetNow("minute")?.toISOString() ?? "missing"}|{useWidgetLocalTimeZone() ?? "missing"}
|
||||
</time>
|
||||
);
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
@@ -16,12 +20,13 @@ afterEach(() => {
|
||||
});
|
||||
|
||||
describe("WidgetTimeProvider", () => {
|
||||
test("uses the same non-placeholder timestamp for server rendering and hydration", async () => {
|
||||
test("hydrates with server time and timezone before switching to browser values", async () => {
|
||||
vi.useFakeTimers();
|
||||
const timestamp = new Date("2026-08-22T12:34:00.000Z").getTime();
|
||||
vi.setSystemTime(timestamp);
|
||||
const serverTimestamp = new Date("2026-08-22T12:34:00.000Z").getTime();
|
||||
const clientTimestamp = serverTimestamp + 60_000;
|
||||
vi.setSystemTime(serverTimestamp);
|
||||
const content = (
|
||||
<WidgetTimeProvider initialTimestamp={timestamp}>
|
||||
<WidgetTimeProvider initialTimestamp={serverTimestamp} initialTimeZone="UTC">
|
||||
<TimeProbe />
|
||||
</WidgetTimeProvider>
|
||||
);
|
||||
@@ -29,11 +34,15 @@ describe("WidgetTimeProvider", () => {
|
||||
const host = document.createElement("div");
|
||||
host.innerHTML = html;
|
||||
const consoleError = vi.spyOn(console, "error").mockImplementation(() => undefined);
|
||||
vi.spyOn(Intl.DateTimeFormat.prototype, "resolvedOptions").mockReturnValue({
|
||||
timeZone: "Europe/Paris",
|
||||
} as Intl.ResolvedDateTimeFormatOptions);
|
||||
vi.setSystemTime(clientTimestamp);
|
||||
|
||||
expect(host.textContent).toBe("2026-08-22T12:34:00.000Z");
|
||||
expect(host.textContent).toBe("2026-08-22T12:34:00.000Z|UTC");
|
||||
const root = hydrateRoot(host, content);
|
||||
await act(async () => undefined);
|
||||
expect(host.textContent).toBe("2026-08-22T12:34:00.000Z");
|
||||
expect(host.textContent).toBe("2026-08-22T12:35:00.000Z|Europe/Paris");
|
||||
expect(consoleError).not.toHaveBeenCalled();
|
||||
|
||||
await act(async () => root.unmount());
|
||||
|
||||
@@ -1,12 +1,46 @@
|
||||
"use client";
|
||||
|
||||
import type { PropsWithChildren } from "react";
|
||||
import { createContext, useContext } from "react";
|
||||
import { createContext, useContext, useEffect, useMemo, useState } from "react";
|
||||
|
||||
const WidgetTimeContext = createContext<number | null>(null);
|
||||
interface WidgetTimeContextValue {
|
||||
initialTimestamp: number | null;
|
||||
localTimeZone: string | null;
|
||||
}
|
||||
|
||||
export const WidgetTimeProvider = ({ children, initialTimestamp }: PropsWithChildren<{ initialTimestamp: number }>) => (
|
||||
<WidgetTimeContext.Provider value={initialTimestamp}>{children}</WidgetTimeContext.Provider>
|
||||
);
|
||||
const WidgetTimeContext = createContext<WidgetTimeContextValue>({ initialTimestamp: null, localTimeZone: null });
|
||||
|
||||
export const useWidgetInitialTimestamp = () => useContext(WidgetTimeContext);
|
||||
interface WidgetTimeProviderProps {
|
||||
initialTimestamp: number;
|
||||
initialTimeZone: string;
|
||||
}
|
||||
|
||||
export const WidgetTimeProvider = ({
|
||||
children,
|
||||
initialTimestamp,
|
||||
initialTimeZone,
|
||||
}: PropsWithChildren<WidgetTimeProviderProps>) => {
|
||||
const [localTimeZone, setLocalTimeZone] = useState(() => getSupportedTimeZone(initialTimeZone));
|
||||
|
||||
useEffect(() => {
|
||||
setLocalTimeZone(getResolvedLocalTimeZone());
|
||||
}, []);
|
||||
|
||||
const value = useMemo(() => ({ initialTimestamp, localTimeZone }), [initialTimestamp, localTimeZone]);
|
||||
return <WidgetTimeContext.Provider value={value}>{children}</WidgetTimeContext.Provider>;
|
||||
};
|
||||
|
||||
export const useWidgetInitialTimestamp = () => useContext(WidgetTimeContext).initialTimestamp;
|
||||
export const useWidgetLocalTimeZone = () => useContext(WidgetTimeContext).localTimeZone;
|
||||
|
||||
const getResolvedLocalTimeZone = () => getSupportedTimeZone(Intl.DateTimeFormat().resolvedOptions().timeZone);
|
||||
|
||||
const getSupportedTimeZone = (timeZone: string | undefined) => {
|
||||
if (!timeZone) return "UTC";
|
||||
try {
|
||||
new Intl.DateTimeFormat("en", { timeZone }).format(0);
|
||||
return timeZone;
|
||||
} catch {
|
||||
return "UTC";
|
||||
}
|
||||
};
|
||||
@@ -1,3 +1,3 @@
|
||||
export { WidgetTimeProvider, useWidgetInitialTimestamp } from "./common/widget-time-context";
|
||||
export { WidgetTimeProvider, useWidgetInitialTimestamp, useWidgetLocalTimeZone } from "./common/widget-time-context";
|
||||
export { getNextWidgetTickDelay, useWidgetNow } from "./common/use-widget-now";
|
||||
export type { WidgetClockPrecision } from "./common/use-widget-now";
|
||||
Reference in new issue
Block a user