Skip to content

Commit

Permalink
PR Feedback.
Browse files Browse the repository at this point in the history
  • Loading branch information
kinyoklion committed Oct 3, 2024
1 parent 4a15070 commit c224cd6
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 10 deletions.
6 changes: 3 additions & 3 deletions packages/shared/sdk-client/__tests__/HookRunner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ describe('given a hook runner and test hook', () => {
it('should execute identify hooks', () => {
const context: LDContext = { kind: 'user', key: 'user-123' };
const timeout = 10;
const identifyResult: IdentifyResult = 'completed';
const identifyResult: IdentifyResult = { status: 'completed' };

const identifyCallback = hookRunner.identify(context, timeout);
identifyCallback(identifyResult);
Expand Down Expand Up @@ -170,7 +170,7 @@ describe('given a hook runner and test hook', () => {
const errorHookRunner = new HookRunner(logger, [errorHook]);

const identifyCallback = errorHookRunner.identify({ kind: 'user', key: 'user-123' }, 1000);
identifyCallback('error');
identifyCallback({ status: 'error' });

expect(logger.error).toHaveBeenCalledWith(
expect.stringContaining(
Expand All @@ -182,7 +182,7 @@ describe('given a hook runner and test hook', () => {
it('should pass identify series data from before to after hooks', () => {
const context: LDContext = { kind: 'user', key: 'user-123' };
const timeout = 10;
const identifyResult: IdentifyResult = 'completed';
const identifyResult: IdentifyResult = { status: 'completed' };

testHook.beforeIdentify = jest
.fn()
Expand Down
14 changes: 10 additions & 4 deletions packages/shared/sdk-client/__tests__/LDClientImpl.hooks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ it('should use hooks registered during configuration', async () => {
expect(testHook.afterIdentify).toHaveBeenCalledWith(
{ context: { key: 'user-key' }, timeout: undefined },
{},
'completed',
{ status: 'completed' },
);
expect(testHook.beforeEvaluation).toHaveBeenCalledWith(
{ context: { key: 'user-key' }, defaultValue: false, flagKey: 'flag-key' },
Expand Down Expand Up @@ -114,7 +114,7 @@ it('should execute hooks that are added using addHook', async () => {
expect(addedHook.afterIdentify).toHaveBeenCalledWith(
{ context: { key: 'user-key' }, timeout: undefined },
{},
'completed',
{ status: 'completed' },
);
expect(addedHook.beforeEvaluation).toHaveBeenCalledWith(
{ context: { key: 'user-key' }, defaultValue: false, flagKey: 'flag-key' },
Expand Down Expand Up @@ -158,6 +158,12 @@ it('should execute both initial hooks and hooks added using addHook', async () =
{
sendEvents: false,
hooks: [initialHook],
logger: {
debug: jest.fn(),
info: jest.fn(),
warn: jest.fn(),
error: jest.fn(),
},
},
factory,
);
Expand Down Expand Up @@ -187,7 +193,7 @@ it('should execute both initial hooks and hooks added using addHook', async () =
expect(initialHook.afterIdentify).toHaveBeenCalledWith(
{ context: { key: 'user-key' }, timeout: undefined },
{},
'completed',
{ status: 'completed' },
);
expect(initialHook.beforeEvaluation).toHaveBeenCalledWith(
{ context: { key: 'user-key' }, defaultValue: false, flagKey: 'flag-key' },
Expand All @@ -214,7 +220,7 @@ it('should execute both initial hooks and hooks added using addHook', async () =
expect(addedHook.afterIdentify).toHaveBeenCalledWith(
{ context: { key: 'user-key' }, timeout: undefined },
{},
'completed',
{ status: 'completed' },
);
expect(addedHook.beforeEvaluation).toHaveBeenCalledWith(
{ context: { key: 'user-key' }, defaultValue: false, flagKey: 'flag-key' },
Expand Down
4 changes: 2 additions & 2 deletions packages/shared/sdk-client/src/LDClientImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,11 +254,11 @@ export default class LDClientImpl implements LDClient {

return identifyPromise.then(
(res) => {
afterIdentify('completed');
afterIdentify({ status: 'completed' });
return res;
},
(e) => {
afterIdentify('error');
afterIdentify({ status: 'error' });
throw e;
},
);
Expand Down
9 changes: 8 additions & 1 deletion packages/shared/sdk-client/src/api/integrations/Hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,19 @@ export interface IdentifySeriesData {
readonly [index: string]: unknown;
}

/**
* The status an identify operation completed with.
*/
export type IdentifyStatus = 'completed' | 'error';

/**
* The result applies to a single identify operation. An operation may complete
* with an error and then later complete successfully. Only the first completion
* will be executed in the evaluation series.
*/
export type IdentifyResult = 'completed' | 'error';
export interface IdentifyResult {
status: IdentifyStatus;
}

/**
* Interface for extending SDK functionality via hooks.
Expand Down

0 comments on commit c224cd6

Please sign in to comment.