Repository: open-feature/js-sdk
Pull request #816 (2024-02-14 18:00:00)

fix: improve event handler types
Before this change the `flagsChanged` prop on `EventDetails` was of type unknown for all event types, and a cast was needed.

After this change, `flagsChanged` is ONLY defined on `ProviderEvents.ConfigurationChanged`, where it should be, as (`string[] | undefined`) . In other words:

```typescript
client.addHandler(ProviderEvents.Ready, (details) => {
  expect(details?.flagsChanged?.length).toEqual(1);  // does not compile
});
```

```typescript
client.addHandler(ProviderEvents.ConfigurationChanged, (details) => {
  expect(details?.flagsChanged?.length).toEqual(1);  // DOES compile
});
```

Additionally, `flagsChanged` does not autocomplete for events handlers other than `ProviderEvents.ConfigurationChanged`.

toddbaert: I'm going to rebase this on https://github.com/open-feature/js-sdk/pull/795 when it's merged.

Patch:
diff --git a/packages/client/src/client/client.ts b/packages/client/src/client/client.ts
index fd8efe7e..699359e6 100644
--- a/packages/client/src/client/client.ts
+++ b/packages/client/src/client/client.ts
@@ -1,8 +1,9 @@
 import { ClientMetadata, EvaluationLifeCycle, Eventing, ManageLogger } from '@openfeature/core';
 import { Features } from '../evaluation';
 import { ProviderStatus } from '../provider';
+import { ProviderEvents } from '../events';
 
-export interface Client extends EvaluationLifeCycle<Client>, Features, ManageLogger<Client>, Eventing {
+export interface Client extends EvaluationLifeCycle<Client>, Features, ManageLogger<Client>, Eventing<ProviderEvents> {
   readonly metadata: ClientMetadata;
   /**
    * Returns the status of the associated provider.
diff --git a/packages/client/src/client/open-feature-client.ts b/packages/client/src/client/open-feature-client.ts
index 3fca4d6b..c9bf0857 100644
--- a/packages/client/src/client/open-feature-client.ts
+++ b/packages/client/src/client/open-feature-client.ts
@@ -11,6 +11,7 @@ import {
   Logger,
   OpenFeatureError,
   ProviderFatalError,
+  ProviderNotReadyError,
   ResolutionDetails,
   SafeLogger,
   StandardResolutionReasons,
@@ -23,7 +24,6 @@ import { Hook } from '../hooks';
 import { OpenFeature } from '../open-feature';
 import { Provider, ProviderStatus } from '../provider';
 import { Client } from './client';
-import { ProviderNotReadyError } from '@openfeature/core';
 
 type OpenFeatureClientOptions = {
   /**
diff --git a/packages/server/src/client/client.ts b/packages/server/src/client/client.ts
index 1cce15bd..6c31fa6e 100644
--- a/packages/server/src/client/client.ts
+++ b/packages/server/src/client/client.ts
@@ -7,13 +7,14 @@ import {
 } from '@openfeature/core';
 import { Features } from '../evaluation';
 import { ProviderStatus } from '../provider';
+import { ProviderEvents } from '../events';
 
 export interface Client
   extends EvaluationLifeCycle<Client>,
     Features,
     ManageContext<Client>,
     ManageLogger<Client>,
-    Eventing {
+    Eventing<ProviderEvents> {
   readonly metadata: ClientMetadata;
   /**
    * Returns the status of the associated provider.
diff --git a/packages/shared/src/events/eventing.ts b/packages/shared/src/events/eventing.ts
index c17e7457..fb489027 100644
--- a/packages/shared/src/events/eventing.ts
+++ b/packages/shared/src/events/eventing.ts
@@ -1,5 +1,5 @@
 import { ErrorCode } from '../evaluation';
-import { AllProviderEvents, AnyProviderEvent } from './events';
+import { ClientProviderEvents, ServerProviderEvents, AnyProviderEvent } from './events';
 
 export type EventMetadata = {
   [key: string]: string | boolean | number;
@@ -19,48 +19,86 @@ type CommonEventProps = {
   metadata?: EventMetadata;
 };
 
-
 export type ReadyEvent = CommonEventProps;
 export type ErrorEvent = CommonEventProps;
 export type StaleEvent = CommonEventProps;
 export type ReconcilingEvent = CommonEventProps & { errorCode: ErrorCode };
 export type ConfigChangeEvent = CommonEventProps & { flagsChanged?: string[] };
 
-type EventMap = {
-  [AllProviderEvents.Ready]: ReadyEvent;
-  [AllProviderEvents.Error]: ErrorEvent;
-  [AllProviderEvents.Stale]: StaleEvent;
-  [AllProviderEvents.ContextChanged]: CommonEventProps;
-  [AllProviderEvents.ConfigurationChanged]: ConfigChangeEvent;
-  [AllProviderEvents.Reconciling]: ReconcilingEvent;
+type ServerEventMap = {
+  [ServerProviderEvents.Ready]: ReadyEvent;
+  [ServerProviderEvents.Error]: ErrorEvent;
+  [ServerProviderEvents.Stale]: StaleEvent;
+  [ServerProviderEvents.ConfigurationChanged]: ConfigChangeEvent;
+};
+
+type ClientEventMap = {
+  [ClientProviderEvents.Ready]: ReadyEvent;
+  [ClientProviderEvents.Error]: ErrorEvent;
+  [ClientProviderEvents.Stale]: StaleEvent;
+  [ClientProviderEvents.ConfigurationChanged]: ConfigChangeEvent;
+  [ClientProviderEvents.Reconciling]: CommonEventProps;
+  [ClientProviderEvents.ContextChanged]: CommonEventProps;
 };
 
-export type EventContext<  U extends Record<string, unknown> = Record<string, unknown>
-> = EventMap[AllProviderEvents] & U;
+type ServerNotChangeEvents =
+  | ServerProviderEvents.Ready
+  | ServerProviderEvents.Error
+  | ServerProviderEvents.Stale;
+type ClientNotChangeEvents =
+  | ClientProviderEvents.Ready
+  | ClientProviderEvents.Error
+  | ClientProviderEvents.Stale
+  | ClientProviderEvents.ContextChanged
+  | ClientProviderEvents.Reconciling;
+export type NotChangeEvents = ServerNotChangeEvents | ClientNotChangeEvents;
+
+export type EventContext<
+  U extends Record<string, unknown> = Record<string, unknown>,
+  T extends ServerProviderEvents | ClientProviderEvents = ServerProviderEvents | ClientProviderEvents,
+> = (T extends ClientProviderEvents ? ClientEventMap[T] : T extends ServerProviderEvents ? ServerEventMap[T] : never) &
+  U;
 
-export type EventDetails = EventContext & CommonEventDetails;
-export type EventHandler = (eventDetails?: EventDetails) => Promise<unknown> | unknown;
+export type EventDetails<
+  T extends ServerProviderEvents | ClientProviderEvents = ServerProviderEvents | ClientProviderEvents,
+> = EventContext<Record<string, unknown>, T> & CommonEventDetails;
+export type EventHandler<
+  T extends ServerProviderEvents | ClientProviderEvents = ServerProviderEvents | ClientProviderEvents,
+> = (eventDetails?: EventDetails<T>) => Promise<unknown> | unknown;
 
-export interface Eventing {
+export interface Eventing<T extends ServerProviderEvents | ClientProviderEvents> {
   /**
    * Adds a handler for the given provider event type.
    * The handlers are called in the order they have been added.
-   * @param {AnyProviderEvent} eventType The provider event type to listen to
+   * @param eventType The provider event type to listen to
    * @param {EventHandler} handler The handler to run on occurrence of the event type
    */
-  addHandler(eventType: AnyProviderEvent, handler: EventHandler): void;
+  addHandler(
+    eventType: T extends ClientProviderEvents
+      ? ClientProviderEvents.ConfigurationChanged
+      : ServerProviderEvents.ConfigurationChanged,
+    handler: EventHandler<
+      T extends ClientProviderEvents
+        ? ClientProviderEvents.ConfigurationChanged
+        : ServerProviderEvents.ConfigurationChanged
+    >,
+  ): void;
+  addHandler(
+    eventType: T extends ClientProviderEvents ? ClientNotChangeEvents : ServerNotChangeEvents,
+    handler: EventHandler<T extends ClientProviderEvents ? ClientNotChangeEvents : ServerNotChangeEvents>,
+  ): void;
 
   /**
    * Removes a handler for the given provider event type.
    * @param {AnyProviderEvent} eventType The provider event type to remove the listener for
    * @param {EventHandler} handler The handler to remove for the provider event type
    */
-  removeHandler(eventType: AnyProviderEvent, handler: EventHandler): void;
+  removeHandler(eventType: T, handler: EventHandler<T>): void;
 
   /**
    * Gets the current handlers for the given provider event type.
    * @param {AnyProviderEvent} eventType The provider event type to get the current handlers for
    * @returns {EventHandler[]} The handlers currently attached to the given provider event type
    */
-  getHandlers(eventType: AnyProviderEvent): EventHandler[];
+  getHandlers(eventType: T): EventHandler<T>[];
 }
diff --git a/packages/shared/src/open-feature.ts b/packages/shared/src/open-feature.ts
index 7248dde9..4ba7e376 100644
--- a/packages/shared/src/open-feature.ts
+++ b/packages/shared/src/open-feature.ts
@@ -7,7 +7,7 @@ import {
   EventHandler,
   Eventing,
   GenericEventEmitter,
-  statusMatchesEvent,
+  statusMatchesEvent
 } from './events';
 import { isDefined } from './filter';
 import { BaseHook, EvaluationLifeCycle } from './hooks';
@@ -74,7 +74,7 @@ export class ProviderWrapper<P extends CommonProvider<AnyProviderStatus>, S exte
 }
 
 export abstract class OpenFeatureCommonAPI<S extends AnyProviderStatus, P extends CommonProvider<S> = CommonProvider<S>, H extends BaseHook = BaseHook>
-  implements Eventing, EvaluationLifeCycle<OpenFeatureCommonAPI<S, P>>, ManageLogger<OpenFeatureCommonAPI<S, P>>
+  implements Eventing<AnyProviderEvent>, EvaluationLifeCycle<OpenFeatureCommonAPI<S, P>>, ManageLogger<OpenFeatureCommonAPI<S, P>>
 {
   // accessor for the type of the ProviderStatus enum (client or server)
   protected abstract readonly _statusEnumType: typeof ClientProviderStatus | typeof ServerProviderStatus;


Test patch:
diff --git a/packages/client/test/events.spec.ts b/packages/client/test/events.spec.ts
index d4060f73..f417fb62 100644
--- a/packages/client/test/events.spec.ts
+++ b/packages/client/test/events.spec.ts
@@ -507,14 +507,17 @@ describe('Events', () => {
     it('It defines a mechanism for signalling `PROVIDER_CONFIGURATION_CHANGED`', (done) => {
       const provider = new MockProvider();
       const client = OpenFeature.getClient(domain);
+      const changedFlag = 'fake-flag';
 
-      client.addHandler(ProviderEvents.ConfigurationChanged, () => {
+      client.addHandler(ProviderEvents.ConfigurationChanged, (details) => {
+        expect(details?.flagsChanged?.length).toEqual(1);
+        expect(details?.flagsChanged).toEqual([changedFlag]);
         done();
       });
 
       OpenFeature.setProvider(domain, provider);
       // emit a change event from the mock provider
-      provider.events?.emit(ProviderEvents.ConfigurationChanged);
+      provider.events?.emit(ProviderEvents.ConfigurationChanged, { flagsChanged: [changedFlag] });
     });
   });
 
diff --git a/packages/server/test/events.spec.ts b/packages/server/test/events.spec.ts
index 1569b2b7..4198d7da 100644
--- a/packages/server/test/events.spec.ts
+++ b/packages/server/test/events.spec.ts
@@ -487,14 +487,17 @@ describe('Events', () => {
     it('It defines a mechanism for signalling `PROVIDER_CONFIGURATION_CHANGED`', (done) => {
       const provider = new MockProvider();
       const client = OpenFeature.getClient(domain);
+      const changedFlag = 'fake-flag';
 
-      client.addHandler(ProviderEvents.ConfigurationChanged, () => {
+      client.addHandler(ProviderEvents.ConfigurationChanged, (details) => {
+        expect(details?.flagsChanged?.length).toEqual(1);
+        expect(details?.flagsChanged).toEqual([changedFlag]);
         done();
       });
 
       OpenFeature.setProvider(domain, provider);
       // emit a change event from the mock provider
-      provider.events?.emit(ProviderEvents.ConfigurationChanged);
+      provider.events?.emit(ProviderEvents.ConfigurationChanged, { flagsChanged: [changedFlag] });
     });
   });