Improve TypeScript Support (#25)

.changeset/long-cups-drive.md

@@ -0,0 +1,53 @@
+---
+"@open-pioneer/runtime": major
+---
+
+**Breaking Change**: change how services integrate into TypeScript (fixes #22).
+The old TypeScript integration had unexpected edge cases, see the linked issue.
+
+NOTE: The changes below have no impact on runtime behavior, but they may trigger TypeScript errors in your code.
+
+-   To register a service's type with TypeScript, one previously used a block such as this:
+
+    ```ts
+    // OLD! Can be removed
+    import "@open-pioneer/runtime";
+    declare module "@open-pioneer/runtime" {
+        interface ServiceRegistry {
+            "http.HttpService": HttpService;
+        }
+    }
+    ```
+
+    The new method requires the developer to change the service's declaration.
+    Simply add `extends DeclaredService<"SERVICE_ID">` to your service interface, where `SERVICE_ID` should match the service's interface name (`"provides"` in `build.config.mjs`).
+
+    ```diff
+    + import { DeclaredService } from "@open-pioneer/runtime";
+
+    - export interface HttpService {
+    + export interface HttpService extends DeclaredService<"http.HttpService"> {
+
+    - import "@open-pioneer/runtime";
+    - declare module "@open-pioneer/runtime" {
+    -     interface ServiceRegistry {
+    -         "http.HttpService": HttpService;
+    -     }
+    - }
+    ```
+
+-   To use a service from React code (i.e. `useService` and `useServices`), you must now use the explicit service type in the hook's generic parameter list. Otherwise the hook will simply return `unknown`:
+
+    ```diff
+    + import { HttpService } from "@open-pioneer/http";
+    - const httpService = useService("http.HttpService");
+    + const httpService = useService<HttpService>("http.HttpService");
+    ```
+
+    This change was necessary to fix an issue where the global registration of the service interface (and its association with the string constant) was not available.
+
+    The system will still check that the provided string matches the string constant used in the service's declaration (`DeclaredService<...>`), so type safety is preserved.
+
+-   The types `InterfaceName` and `ServiceType<I>` have been removed. Use explicit service interfaces instead.
+-   The interfaces `ServiceRegistry` and `PropertiesRegistry` have been removed as global registration is no longer possible.
+-   The type `RawApplicationProperties` has been removed. Use `ApplicationProperties` instead.

.changeset/smooth-cups-deny.md

@@ -0,0 +1,7 @@
+---
+"@open-pioneer/integration": major
+"@open-pioneer/http": major
+"@open-pioneer/base-theme": major
+---
+
+Compatibility with @open-pioneer/runtime@^2

.husky/pre-commit

@@ -9,12 +9,12 @@ fi
 echo '--- checking for consistent dependencies across packages'
 pnpm lint-shared-versions
 
-echo '--- run linting --- '
-pnpm lint
-
 echo '--- run prettier ---'
 pnpm prettier-check
 
+echo '--- run linting --- '
+pnpm lint
+
 echo '--- run typescript check ---'
 pnpm check-types
 

src/packages/http/index.ts

@@ -1,12 +1,13 @@
 // SPDX-FileCopyrightText: 2023 Open Pioneer project (https://github.com/open-pioneer)
 // SPDX-License-Identifier: Apache-2.0
+import { DeclaredService } from "@open-pioneer/runtime";
 
 /**
  * Central service for sending HTTP requests.
  *
  * Use the interface `"http.HttpService"` to obtain an instance of this service.
  */
-export interface HttpService {
+export interface HttpService extends DeclaredService<"http.HttpService"> {
     /**
      * Requests the given `resource` via HTTP and returns the response.
      *
@@ -20,13 +21,3 @@ export interface HttpService {
      */
     fetch(resource: RequestInfo | URL, init?: RequestInit): Promise<Response>;
 }
-
-import "@open-pioneer/runtime";
-declare module "@open-pioneer/runtime" {
-    interface ServiceRegistry {
-        "http.HttpService": HttpService;
-    }
-}
-
-// Get rid of empty chunk warning
-export default undefined;

src/packages/integration/api.ts

@@ -1,13 +1,20 @@
 // SPDX-FileCopyrightText: 2023 Open Pioneer project (https://github.com/open-pioneer)
 // SPDX-License-Identifier: Apache-2.0
-import { type ApiExtension, type ApiMethods, type ApiMethod } from "@open-pioneer/runtime";
+import {
+    type ApiExtension,
+    type ApiMethods,
+    type ApiMethod,
+    DeclaredService
+} from "@open-pioneer/runtime";
+
+export { ApiExtension, ApiMethod, ApiMethods }; // re-export for consistency
 
 /**
  * Emits events to users of the current web component.
  *
  * Use the interface `"integration.ExternalEventService"` to obtain an instance of this service.
  */
-export interface ExternalEventService {
+export interface ExternalEventService extends DeclaredService<"integration.ExternalEventService"> {
     /**
      * Emits an event to the host site as a [CustomEvent](https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent/CustomEvent).
      *
@@ -45,13 +52,3 @@ export interface ExternalEventService {
      */
     emitEvent(event: Event): void;
 }
-
-export { ApiExtension, ApiMethod, ApiMethods }; // re-export for consistency
-
-import "@open-pioneer/runtime";
-declare module "@open-pioneer/runtime" {
-    interface ServiceRegistry {
-        "integration.ApiExtension": ApiExtension;
-        "integration.ExternalEventService": ExternalEventService;
-    }
-}

src/packages/runtime/CustomElement.test.ts

@@ -26,6 +26,10 @@ interface InternalElementType extends ApplicationElement {
     $inspectElementState?(): any;
 }
 
+afterEach(() => {
+    vi.restoreAllMocks();
+});
+
 describe("simple rendering", function () {
     const SIMPLE_STYLE = ".test { color: red }";
     const SIMPLE_ELEM = createCustomElement({
@@ -346,6 +350,8 @@ describe("application lifecycle events", function () {
     });
 
     it("does not signal 'before-stop' when start fails", async function () {
+        const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined);
+
         const events: string[] = [];
         class Listener implements ApplicationLifecycleListener {
             afterApplicationStart() {
@@ -391,6 +397,30 @@ describe("application lifecycle events", function () {
         });
 
         expect(events).toEqual([]);
+        expect(errorSpy).toMatchInlineSnapshot(`
+          [MockFunction error] {
+            "calls": [
+              [
+                "#1",
+                [Error: help!],
+              ],
+              [
+                "#2",
+                [Error: runtime:config-resolution-failed: Failed to resolve application properties.],
+              ],
+            ],
+            "results": [
+              {
+                "type": "return",
+                "value": undefined,
+              },
+              {
+                "type": "return",
+                "value": undefined,
+              },
+            ],
+          }
+        `);
     });
 });
 

src/packages/runtime/CustomElement.ts

@@ -26,7 +26,6 @@ import {
     RUNTIME_AUTO_START
 } from "./builtin-services";
 import { ReferenceSpec } from "./service-layer/InterfaceSpec";
-import { PropertiesRegistry } from "./PropertiesRegistry";
 import { AppI18n, initI18n } from "./i18n";
 import { ApplicationLifecycleEventService } from "./builtin-services/ApplicationLifecycleEventService";
 const LOG = createLogger("runtime:CustomElement");
@@ -108,8 +107,10 @@ export interface ApplicationConfig {
 
 /**
  * Allows the application to override default properties in all packages.
+ *
+ * Properties are typed when the package contains type definitions for them.
  */
-export interface RawApplicationProperties {
+export interface ApplicationProperties {
     /**
      * Key: the name of the package.
      * Value: A record of configuration properties (key/value pairs).
@@ -119,13 +120,6 @@ export interface RawApplicationProperties {
     [packageName: string]: Record<string, unknown>;
 }
 
-/**
- * Allows the application to override default properties in all packages.
- *
- * Properties are typed when the package contains type definitions for them.
- */
-export type ApplicationProperties = RawApplicationProperties & Partial<PropertiesRegistry>;
-
 /**
  * The interface implemented by web components produced via {@link createCustomElement}.
  */

src/packages/runtime/DeclaredService.test.ts

@@ -0,0 +1,48 @@
+// SPDX-FileCopyrightText: 2023 Open Pioneer project (https://github.com/open-pioneer)
+// SPDX-License-Identifier: Apache-2.0
+/* eslint-disable unused-imports/no-unused-vars */
+import { DeclaredService, InterfaceNameForServiceType } from "./DeclaredService";
+import { it } from "vitest";
+
+// Tests are on type level only
+it("dummy test to allow a file without any real tests", () => undefined);
+
+/**
+ * Returns type `true` if types A and B are equal (type false otherwise).
+ * See here: https://github.com/Microsoft/TypeScript/issues/27024#issuecomment-421529650
+ */
+// prettier-ignore
+type Equal<X, Y> =
+    (<T>() => T extends X ? 1 : 2) extends
+    (<T>() => T extends Y ? 1 : 2) ? true : false;
+
+/**
+ * Returns type `true` if T is any kind of string, `false` otherwise.
+ */
+type IsString<T> = T extends string ? true : false;
+
+// Expect all strings are allowed when service type is unknown
+{
+    type IFace = InterfaceNameForServiceType<unknown>;
+    const isString: Equal<IFace, string> = true;
+}
+
+// Expect only the declared interface name is allowed when an explicit service is provided
+{
+    interface MyService extends DeclaredService<"my.service"> {
+        foo(): void;
+    }
+
+    type IFace = InterfaceNameForServiceType<MyService>;
+    const isConstant: Equal<IFace, "my.service"> = true;
+}
+
+// Expect an error is returned when an explicit type is used that does not extend DeclaredService
+{
+    interface MyService {
+        foo(): void;
+    }
+
+    type IFace = InterfaceNameForServiceType<MyService>;
+    const isString: IsString<IFace> = false;
+}

src/packages/runtime/DeclaredService.ts

@@ -0,0 +1,65 @@
+// SPDX-FileCopyrightText: 2023 Open Pioneer project (https://github.com/open-pioneer)
+// SPDX-License-Identifier: Apache-2.0
+declare const INTERNAL_ASSOCIATED_SERVICE_METADATA: unique symbol;
+declare const ERROR: unique symbol;
+
+/**
+ * Base interface for services that are associated with a well known interface name.
+ *
+ * By using this base interface, you can ensure that users of your interface use the correct interface name.
+ *
+ * @example
+ * ```ts
+ * // MyLogger should be referenced via "my-package.Logger"
+ * export interface MyLogger extends DeclaredService<"my-package.Logger"> {
+ *     log(message: string): void;
+ * }
+ * ```
+ *
+ * > Note: TypeScript may list the `INTERNAL_ASSOCIATED_SERVICE_METADATA` property
+ * > when generating the implementation for an interface extending this type.
+ * > You can simply remove the offending line; it is not required (and not possible)
+ * > to implement that attribute - it only exists for the compiler.
+ */
+export interface DeclaredService<InterfaceName extends string> {
+    /**
+     * Internal type-level service metadata.
+     *
+     * Note: there is no need to implement this symbol attribute.
+     * It is optional and only exists for the compiler, never at runtime.
+     *
+     * @internal
+     */
+    [INTERNAL_ASSOCIATED_SERVICE_METADATA]?: ServiceMetadata<InterfaceName>;
+}
+
+/**
+ * Given a type implementing {@link DeclaredService}, this type will produce the interface name associated with the service type.
+ */
+export type AssociatedInterfaceName<T extends DeclaredService<string>> = T extends DeclaredService<
+    infer InterfaceName
+>
+    ? InterfaceName
+    : never;
+
+/**
+ * This helper type produces the expected `interfaceName` (a string parameter) for the given service type.
+ *
+ * 1. If `ServiceType` is `unknown`, it will produce `string` to allow arbitrary parameters.
+ * 2. If `ServiceType` implements {@link DeclaredService}, it will enforce the associated interface name.
+ * 3. Otherwise, a compile time error is generated.
+ */
+export type InterfaceNameForServiceType<ServiceType> = unknown extends ServiceType
+    ? string
+    : ServiceType extends DeclaredService<string>
+      ? AssociatedInterfaceName<ServiceType>
+      : {
+            [ERROR]: "TypeScript integration was not set up properly for this service. Make sure the service's TypeScript interface extends 'DeclaredService'.";
+        };
+
+/**
+ * @internal
+ */
+interface ServiceMetadata<InterfaceName> {
+    interfaceName: InterfaceName;
+}