Repository: open-feature/js-sdk
Pull request #693 (2023-11-27 21:53:37)

fix: type support for Promise<void> before hook
## This PR

- Updates the server-side before hook type to perform async work without modifying context.

### Related Issues

Fixes #689

### Notes

I added a test but the test runner isn't not validating types.



toddbaert: @beeme1mr I made slight changes to the title. Hope you don't mind. Feel free to merge if you're OK with it.

Patch:
diff --git a/packages/server/src/hooks/hook.ts b/packages/server/src/hooks/hook.ts
index 606c2ed0..b98d8364 100644
--- a/packages/server/src/hooks/hook.ts
+++ b/packages/server/src/hooks/hook.ts
@@ -2,6 +2,6 @@ import { BaseHook, EvaluationContext, FlagValue } from '@openfeature/core';
 
 export type Hook = BaseHook<
   FlagValue,
-  Promise<EvaluationContext | Promise<void>> | EvaluationContext | void,
+  Promise<EvaluationContext | void> | EvaluationContext | void,
   Promise<void> | void
 >;


Test patch:
diff --git a/packages/server/test/hooks.spec.ts b/packages/server/test/hooks.spec.ts
index 4dd60dc9..3dcb439d 100644
--- a/packages/server/test/hooks.spec.ts
+++ b/packages/server/test/hooks.spec.ts
@@ -207,7 +207,8 @@ describe('Hooks', () => {
       const clientPropToOverwrite434 = 'clientPropToOverwrite';
       const invocationProp434 = 'invocationProp';
       const invocationPropToOverwrite434 = 'invocationPropToOverwrite';
-      const hookProp434 = 'hookProp';
+      const syncHookProp434 = 'syncHookProp';
+      const asyncHookProp434 = 'asyncHookProp';
 
       OpenFeature.setContext({
         [globalProp434]: true,
@@ -223,21 +224,43 @@ describe('Hooks', () => {
         [invocationPropToOverwrite434]: false,
         [clientPropToOverwrite434]: true,
       };
-      const hookContext = {
+      const syncHookContext = {
         [invocationPropToOverwrite434]: true,
-        [hookProp434]: true,
+        [syncHookProp434]: true,
+      };
+      const asyncHookContext = {
+        [asyncHookProp434]: true,
       };
 
       const localClient = OpenFeature.getClient('merge-test', 'test', clientContext);
 
+      const syncVoidHook: Hook = {
+        before: () => {
+          // synchronous hook that doesn't modify context
+        },
+      };
+
+      const syncContextHook: Hook = {
+        before: () => {
+          return syncHookContext;
+        },
+      };
+
+      const asyncVoidHook: Hook = {
+        before: async () => {
+          // asynchronous hook that doesn't modify context
+          await Promise.resolve();
+        },
+      };
+
+      const asyncContextHook: Hook = {
+        before: () => {
+          return Promise.resolve(asyncHookContext);
+        },
+      };
+
       await localClient.getBooleanValue(FLAG_KEY, false, invocationContext, {
-        hooks: [
-          {
-            before: () => {
-              return hookContext;
-            },
-          },
-        ],
+        hooks: [syncVoidHook, syncContextHook, asyncVoidHook, asyncContextHook],
       });
       expect(MOCK_PROVIDER.resolveBooleanEvaluation).toHaveBeenCalledWith(
         expect.anything(),
@@ -250,7 +273,8 @@ describe('Hooks', () => {
           [clientPropToOverwrite434]: true,
           [invocationProp434]: true,
           [invocationPropToOverwrite434]: true,
-          [hookProp434]: true,
+          [syncHookProp434]: true,
+          [asyncHookProp434]: true,
         }),
         expect.anything(),
       );