From 6800448bc1c96959b448e7795ad879fc78d35a06 Mon Sep 17 00:00:00 2001 From: Antonia van Eek Date: Tue, 22 Oct 2024 15:28:29 +0200 Subject: [PATCH] use intl in error screen, refactor ReactIntegration, added tests --- src/packages/runtime/CustomElement.test.ts | 26 +++++++- src/packages/runtime/CustomElement.ts | 52 +++++++--------- src/packages/runtime/ErrorScreen.tsx | 30 +++++++-- src/packages/runtime/errors.ts | 1 + src/packages/runtime/i18n.ts | 2 +- .../ReactIntegration.test.ts | 62 ++++++++++++++----- .../react-integration/ReactIntegration.tsx | 23 ++++--- .../ReactIntegration.test.ts.snap | 2 + 8 files changed, 135 insertions(+), 63 deletions(-) diff --git a/src/packages/runtime/CustomElement.test.ts b/src/packages/runtime/CustomElement.test.ts index 9ab14cde..9021a366 100644 --- a/src/packages/runtime/CustomElement.test.ts +++ b/src/packages/runtime/CustomElement.test.ts @@ -390,7 +390,7 @@ describe("application lifecycle events", function () { const { node } = await renderComponent(elem); await waitFor(() => { const state = (node as InternalElementType).$inspectElementState?.().state; - if (state !== "destroyed") { + if (state !== "error") { throw new Error(`App did not reach destroyed state.`); } }); @@ -675,3 +675,27 @@ describe("i18n support", function () { return result; } }); + +it("renders an error screen when the app fails to start", async () => { + const elem = createCustomElement({ + async resolveConfig() { + throw new Error("help!"); + } + }); + + const { node } = await renderComponent(elem); + await waitFor(() => { + const state = (node as InternalElementType).$inspectElementState?.().state; + if (state !== "error") { + throw new Error(`App did not reach error state.`); + } + }); + + const errorScreen = node.shadowRoot?.querySelector("div"); + expect(errorScreen).not.toBe(undefined); + expect(errorScreen?.className).toBe("pioneer-root pioneer-root-error-screen"); + const includesErrorText = Array.from(errorScreen?.children ?? []).some((child) => + child.textContent?.includes("Error") + ); + expect(includesErrorText).toBe(true); +}); diff --git a/src/packages/runtime/CustomElement.ts b/src/packages/runtime/CustomElement.ts index be5f205e..d8e0b73d 100644 --- a/src/packages/runtime/CustomElement.ts +++ b/src/packages/runtime/CustomElement.ts @@ -1,13 +1,12 @@ // SPDX-FileCopyrightText: 2023 Open Pioneer project (https://github.com/open-pioneer) // SPDX-License-Identifier: Apache-2.0 -import { ComponentType } from "react"; +import { ComponentType, createElement } from "react"; import { createAbortError, createLogger, createManualPromise, destroyResource, Error, - isAbortError, ManualPromise, Resource, throwAbortError @@ -26,9 +25,9 @@ import { RUNTIME_AUTO_START } from "./builtin-services"; import { ReferenceSpec } from "./service-layer/InterfaceSpec"; -import { AppI18n, getBrowserLocales, initI18n } from "./i18n"; +import { AppI18n, createPackageIntl, getBrowserLocales, I18nConfig, initI18n } from "./i18n"; import { ApplicationLifecycleEventService } from "./builtin-services/ApplicationLifecycleEventService"; -import { ErrorScreen } from "./ErrorScreen"; +import { ErrorScreen, MESSAGES_BY_LOCALE } from "./ErrorScreen"; const LOG = createLogger("runtime:CustomElement"); /** @@ -272,8 +271,7 @@ class ApplicationInstance { private apiPromise: ManualPromise | undefined; // Present when callers are waiting for the API private api: ApiMethods | undefined; // Present once started - private state = "not-started" as "not-started" | "starting" | "started" | "destroyed"; - private locale: string | undefined; + private state = "not-started" as "not-started" | "starting" | "started" | "destroyed" | "error"; private container: HTMLDivElement | undefined; private config: ApplicationConfig | undefined; private serviceLayer: ServiceLayer | undefined; @@ -292,11 +290,12 @@ class ApplicationInstance { this.state = "starting"; this.startImpl().catch((e) => { - // this.destroy(); TODO introduce error state and do needed cleanup instead of calling destroy - if (!isAbortError(e)) { - this.showErrorScreen(); - logError(e); - } + if (this.state === "destroyed") return; + + logError(e); + this.reset(); + this.state = "error"; + this.showErrorScreen(); }); } @@ -314,6 +313,10 @@ class ApplicationInstance { } } this.state = "destroyed"; + this.reset(); + } + + private reset() { this.apiPromise?.reject(createAbortError()); this.reactIntegration = destroyResource(this.reactIntegration); this.options.shadowRoot.replaceChildren(); @@ -380,7 +383,7 @@ class ApplicationInstance { private render() { const component = this.options.elementOptions.component ?? emptyComponent; - this.reactIntegration?.render(component); + this.reactIntegration?.render(createElement(component)); } private initStyles() { @@ -468,29 +471,22 @@ class ApplicationInstance { } private showErrorScreen() { - const { shadowRoot, elementOptions, overrides } = this.options; - const container = document.createElement("div"); + const userLocales = getBrowserLocales(); + const i18nConfig = new I18nConfig(["en", "de"]); + const { messageLocale } = i18nConfig.pickSupportedLocale(undefined, userLocales); + const useLocale = messageLocale === "de" ? "de" : "en"; + const intl = createPackageIntl(messageLocale, MESSAGES_BY_LOCALE[useLocale]); + + const container = (this.container = createContainer(useLocale)); container.classList.add("pioneer-root-error-screen"); - container.style.minHeight = "100%"; - container.style.height = "100%"; - - let locale = "en"; - if (overrides?.locale && overrides.locale == "de") { - locale = "de"; - } else { - const userLocales = getBrowserLocales(); - if (userLocales[0] == "de") { - locale = "de"; - } - } - container.lang = locale; + const { shadowRoot, elementOptions } = this.options; this.reactIntegration = ReactIntegration.createForErrorScreen({ rootNode: container, container: shadowRoot, theme: elementOptions.theme }); - this.reactIntegration?.render(ErrorScreen); + this.reactIntegration?.render(createElement(ErrorScreen, { intl })); shadowRoot.replaceChildren(container); } diff --git a/src/packages/runtime/ErrorScreen.tsx b/src/packages/runtime/ErrorScreen.tsx index b3c8bb5a..5d0fef5d 100644 --- a/src/packages/runtime/ErrorScreen.tsx +++ b/src/packages/runtime/ErrorScreen.tsx @@ -10,20 +10,38 @@ import { AlertDescription, Alert } from "@open-pioneer/chakra-integration"; +import { PackageIntl } from "./i18n"; -// todo render error message if started in dev mode +const MESSAGES_DE = { + "title": "Anwendungsstart fehlgeschlagen", + "alertTitle": "Fehler", + "alertDescription": "Leider ist beim Start der Anwendung eine Fehler aufgetreten." +}; -export function ErrorScreen() { - // todo how to access lang parameter? --> pass root as ref? +const MESSAGES_EN = { + "title": "Application start failed", + "alertTitle": "Error", + "alertDescription": "Unfortunately an error occurred during application start." +}; + +export const MESSAGES_BY_LOCALE: Record<"en" | "de", Record> = { + "en": MESSAGES_EN, + "de": MESSAGES_DE +}; + +// todo render error message (and callstack) if started in dev mode + +export function ErrorScreen(props: { intl: PackageIntl }) { + const intl = props.intl; return ( - Application start failed + {intl.formatMessage({ id: "title" })} - Error + {intl.formatMessage({ id: "alertTitle" })} - Unfortunately an error occurred during application start. + {intl.formatMessage({ id: "alertDescription" })} diff --git a/src/packages/runtime/errors.ts b/src/packages/runtime/errors.ts index 5f2bb891..52a26bd8 100644 --- a/src/packages/runtime/errors.ts +++ b/src/packages/runtime/errors.ts @@ -10,6 +10,7 @@ export enum ErrorId { NOT_MOUNTED = "runtime:element-not-mounted", UNSUPPORTED_LOCALE = "runtime:unsupported-locale", CONFIG_RESOLUTION_FAILED = "runtime:config-resolution-failed", + INVALID_STATE = "runtime:invalide-state", // Service layer INTERFACE_NOT_FOUND = "runtime:interface-not-found", diff --git a/src/packages/runtime/i18n.ts b/src/packages/runtime/i18n.ts index 78514425..e1bd91a2 100644 --- a/src/packages/runtime/i18n.ts +++ b/src/packages/runtime/i18n.ts @@ -31,7 +31,7 @@ export interface AppI18n { */ export type PackageIntl = Pick & IntlFormatters; -function createPackageIntl(locale: string, messages: Record) { +export function createPackageIntl(locale: string, messages: Record) { const cache = createIntlCache(); return createIntl( { diff --git a/src/packages/runtime/react-integration/ReactIntegration.test.ts b/src/packages/runtime/react-integration/ReactIntegration.test.ts index 712abfc3..bd3dee15 100644 --- a/src/packages/runtime/react-integration/ReactIntegration.test.ts +++ b/src/packages/runtime/react-integration/ReactIntegration.test.ts @@ -6,7 +6,7 @@ import { findByTestId, findByText } from "@testing-library/dom"; import { act } from "@testing-library/react"; import { createElement } from "react"; -import { beforeEach, expect, it, MockInstance, afterEach, vi } from "vitest"; +import { beforeEach, expect, it, MockInstance, afterEach, vi, describe } from "vitest"; import { Service, ServiceConstructor } from "../Service"; import { usePropertiesInternal, useServiceInternal, useServicesInternal } from "./hooks"; import { useTheme } from "@open-pioneer/chakra-integration"; @@ -57,7 +57,7 @@ it("should allow access to service via react hook", async () => { }); act(() => { - integration.render(TestComponent); + integration.render(createElement(TestComponent)); }); const node = await findByText(wrapper, "Hello TEST"); @@ -79,7 +79,7 @@ it("should get error when using undefined service", async () => { expect(() => { act(() => { - integration.render(TestComponent); + integration.render(createElement(TestComponent)); }); }).toThrowErrorMatchingSnapshot(); expect(errorSpy).toHaveBeenCalledOnce(); @@ -99,7 +99,7 @@ it("reports a helpful error when package metadata is missing", async () => { const { integration } = createIntegration(); expect(() => { act(() => { - integration.render(TestComponent); + integration.render(createElement(TestComponent)); }); }).toThrowErrorMatchingSnapshot(); expect(errorSpy).toHaveBeenCalledOnce(); @@ -127,7 +127,7 @@ it("should allow access to service with qualifier via react hook", async () => { }); act(() => { - integration.render(TestComponent); + integration.render(createElement(TestComponent)); }); const node = await findByText(wrapper, "Hello TEST"); @@ -159,7 +159,7 @@ it("should deny access to service when the qualifier does not match", async () = expect(() => { act(() => { - integration.render(TestComponent); + integration.render(createElement(TestComponent)); }); }).toThrowErrorMatchingSnapshot(); expect(errorSpy).toHaveBeenCalledOnce(); @@ -203,7 +203,7 @@ it("should allow access to all services via react hook", async () => { }); act(() => { - integration.render(TestComponent); + integration.render(createElement(TestComponent)); }); const node = await findByText(wrapper, /^Joined Values:/); @@ -228,7 +228,7 @@ it("should deny access to all services if declaration is missing", async () => { expect(() => { act(() => { - integration.render(TestComponent); + integration.render(createElement(TestComponent)); }); }).toThrowErrorMatchingSnapshot(); expect(errorSpy).toHaveBeenCalledOnce(); @@ -247,7 +247,7 @@ it("should be able to read properties from react component", async () => { }); act(() => { - integration.render(TestComponent); + integration.render(createElement(TestComponent)); }); const node = await findByText(wrapper, "Hello USER"); @@ -271,7 +271,7 @@ it("should provide the autogenerated useService hook", async () => { }); act(() => { - integration.render(UIWithService); + integration.render(createElement(UIWithService)); }); const node = await findByText(wrapper, /^Test-UI:/); @@ -302,7 +302,7 @@ it("should provide the autogenerated useServices hook", async () => { }); act(() => { - integration.render(UIWithServices); + integration.render(createElement(UIWithServices)); }); const node = await findByText(wrapper, /^Test-UI:/); @@ -319,7 +319,7 @@ it("should provide the autogenerated useProperties hook", async () => { }); act(() => { - integration.render(UIWithProperties); + integration.render(createElement(UIWithProperties)); }); const node = await findByText(wrapper, /^Test-UI:/); @@ -340,7 +340,7 @@ it("should throw error when requesting properties from an unknown package", asyn expect(() => { act(() => { - integration.render(TestComponent); + integration.render(createElement(TestComponent)); }); }).toThrowErrorMatchingSnapshot(); expect(errorSpy).toHaveBeenCalledOnce(); @@ -370,14 +370,46 @@ it("should apply the configured chakra theme", async () => { } act(() => { - integration.render(TestComponent); + integration.render(createElement(TestComponent)); }); const node = await findByTestId(wrapper, "test-div"); expect(node.textContent).toBe("Color: #123456"); }); -// TODO: Test `createForErrorScreen` +describe("integration for error screen ", function () { + it("should create an ReactIntegration for an error screen", async () => { + const integration = ReactIntegration.createForErrorScreen({ + rootNode: document.createElement("div"), + container: document.createElement("div"), + theme: undefined + }); + + expect(integration).toBeInstanceOf(ReactIntegration); + }); + + it("should throw an error when trying to access a service on an error screen", async () => { + errorSpy.mockImplementation(doNothing); + + const integration = ReactIntegration.createForErrorScreen({ + rootNode: document.createElement("div"), + container: document.createElement("div"), + theme: undefined + }); + + function TestComponent() { + const service = useServiceInternal("test", "test.Provider") as TestProvider; + return createElement("span", undefined, `Hello ${service.value}`); + } + + expect(() => { + act(() => { + integration.render(createElement(TestComponent)); + }); + }).toThrowErrorMatchingSnapshot(); + expect(errorSpy).toHaveBeenCalledOnce(); + }); +}); interface ServiceSpec { name: string; diff --git a/src/packages/runtime/react-integration/ReactIntegration.tsx b/src/packages/runtime/react-integration/ReactIntegration.tsx index 99f7b287..7058e0b4 100644 --- a/src/packages/runtime/react-integration/ReactIntegration.tsx +++ b/src/packages/runtime/react-integration/ReactIntegration.tsx @@ -1,16 +1,16 @@ // SPDX-FileCopyrightText: 2023 Open Pioneer project (https://github.com/open-pioneer) // SPDX-License-Identifier: Apache-2.0 -import { ComponentType, StrictMode } from "react"; -import { createRoot, Root } from "react-dom/client"; +import { theme as defaultTrailsTheme } from "@open-pioneer/base-theme"; +import { CustomChakraProvider } from "@open-pioneer/chakra-integration"; import { Error } from "@open-pioneer/core"; -import { ErrorId } from "../errors"; -import { ServiceLayer } from "../service-layer/ServiceLayer"; import { PackageContext, PackageContextMethods } from "@open-pioneer/runtime-react-support"; -import { PackageRepr } from "../service-layer/PackageRepr"; +import { ReactNode, StrictMode } from "react"; +import { createRoot, Root } from "react-dom/client"; +import { ErrorId } from "../errors"; import { InterfaceSpec, renderInterfaceSpec } from "../service-layer/InterfaceSpec"; +import { PackageRepr } from "../service-layer/PackageRepr"; +import { ServiceLayer } from "../service-layer/ServiceLayer"; import { renderAmbiguousServiceChoices } from "../service-layer/ServiceLookup"; -import { CustomChakraProvider } from "@open-pioneer/chakra-integration"; -import { theme as defaultTrailsTheme } from "@open-pioneer/base-theme"; export interface ReactIntegrationOptions { packages: Map; @@ -36,8 +36,7 @@ export class ReactIntegration { options: Omit ) { const throwError = () => { - // TODO: Pick an error id - throw new Error(ErrorId.INTERNAL, "Hook cannot be used within the error screen."); + throw new Error(ErrorId.INVALID_STATE, "Hook cannot be used within the error screen."); }; const packageContext: PackageContextMethods = { getIntl: throwError, @@ -59,7 +58,7 @@ export class ReactIntegration { this.packageContext = options.packageContext; } - render(Component: ComponentType) { + render(contentNode: ReactNode) { this.root.render( - + {contentNode} @@ -85,7 +84,7 @@ function createPackageContext( packages: Map ): PackageContextMethods { const getPackage = (packageName: string): PackageRepr => { - const pkg = packages.get(packageName); // todo own error message if this.packages is undefined? + const pkg = packages.get(packageName); if (!pkg) { throw new Error( ErrorId.INTERNAL, diff --git a/src/packages/runtime/react-integration/__snapshots__/ReactIntegration.test.ts.snap b/src/packages/runtime/react-integration/__snapshots__/ReactIntegration.test.ts.snap index b029d65e..3188214f 100644 --- a/src/packages/runtime/react-integration/__snapshots__/ReactIntegration.test.ts.snap +++ b/src/packages/runtime/react-integration/__snapshots__/ReactIntegration.test.ts.snap @@ -1,5 +1,7 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html +exports[`integration for error screen > should throw an error when trying to access a service on an error screen 1`] = `[Error: runtime:invalide-state: Hook cannot be used within the error screen.]`; + exports[`reports a helpful error when package metadata is missing 1`] = `[Error: runtime:missing-package: Package 'packageMissingFromMetadata' was not found in the application's metadata while it attempted to reference the interface 'test.Provider'. Check that the dependency is declared correctly in the packages that use 'packageMissingFromMetadata'.]`; exports[`should allow access to all services via react hook 1`] = `