Skip to content

Commit

Permalink
fix: type support for Promise<void> before hook (#693)
Browse files Browse the repository at this point in the history
## 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.

---------

Signed-off-by: Michael Beemer <[email protected]>
Co-authored-by: Todd Baert <[email protected]>
  • Loading branch information
beeme1mr and toddbaert authored Nov 30, 2023
1 parent 2d1b8eb commit 0b9ca18
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 12 deletions.
2 changes: 1 addition & 1 deletion packages/server/src/hooks/hook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
>;
46 changes: 35 additions & 11 deletions packages/server/test/hooks.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(),
Expand All @@ -250,7 +273,8 @@ describe('Hooks', () => {
[clientPropToOverwrite434]: true,
[invocationProp434]: true,
[invocationPropToOverwrite434]: true,
[hookProp434]: true,
[syncHookProp434]: true,
[asyncHookProp434]: true,
}),
expect.anything(),
);
Expand Down

0 comments on commit 0b9ca18

Please sign in to comment.