Skip to content

Commit 6a22483

Browse files
authored
feat: add support for abort controllers to event handlers (#1151)
Signed-off-by: Michael Beemer <[email protected]>
1 parent a703ee7 commit 6a22483

File tree

9 files changed

+120
-60
lines changed

9 files changed

+120
-60
lines changed

packages/react/src/evaluation/use-feature-flag.ts

+7-11
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,7 @@ function attachHandlersAndResolve<T extends FlagValue>(
280280
const defaultedOptions = { ...DEFAULT_OPTIONS, ...useProviderOptions(), ...normalizeOptions(options) };
281281
const client = useOpenFeatureClient();
282282
const status = useOpenFeatureClientStatus();
283+
const controller = new AbortController();
283284

284285
// suspense
285286
if (defaultedOptions.suspendUntilReady && status === ProviderStatus.NOT_READY) {
@@ -322,28 +323,23 @@ function attachHandlersAndResolve<T extends FlagValue>(
322323
useEffect(() => {
323324
if (status === ProviderStatus.NOT_READY) {
324325
// update when the provider is ready
325-
client.addHandler(ProviderEvents.Ready, updateEvaluationDetailsCallback);
326+
client.addHandler(ProviderEvents.Ready, updateEvaluationDetailsCallback, { signal: controller.signal });
326327
}
327328

328329
if (defaultedOptions.updateOnContextChanged) {
329330
// update when the context changes
330-
client.addHandler(ProviderEvents.ContextChanged, updateEvaluationDetailsCallback);
331+
client.addHandler(ProviderEvents.ContextChanged, updateEvaluationDetailsCallback, { signal: controller.signal });
331332
}
332-
return () => {
333-
// cleanup the handlers
334-
client.removeHandler(ProviderEvents.Ready, updateEvaluationDetailsCallback);
335-
client.removeHandler(ProviderEvents.ContextChanged, updateEvaluationDetailsCallback);
336-
};
337-
}, []);
338333

339-
useEffect(() => {
340334
if (defaultedOptions.updateOnConfigurationChanged) {
341335
// update when the provider configuration changes
342-
client.addHandler(ProviderEvents.ConfigurationChanged, configurationChangeCallback);
336+
client.addHandler(ProviderEvents.ConfigurationChanged, configurationChangeCallback, {
337+
signal: controller.signal,
338+
});
343339
}
344340
return () => {
345341
// cleanup the handlers
346-
client.removeHandler(ProviderEvents.ConfigurationChanged, configurationChangeCallback);
342+
controller.abort();
347343
};
348344
}, []);
349345

packages/react/src/provider/use-open-feature-client-status.ts

+8-12
Original file line numberDiff line numberDiff line change
@@ -10,22 +10,18 @@ import { ProviderEvents } from '@openfeature/web-sdk';
1010
export function useOpenFeatureClientStatus(): ProviderStatus {
1111
const client = useOpenFeatureClient();
1212
const [status, setStatus] = useState<ProviderStatus>(client.providerStatus);
13+
const controller = new AbortController();
1314

1415
useEffect(() => {
1516
const updateStatus = () => setStatus(client.providerStatus);
16-
client.addHandler(ProviderEvents.ConfigurationChanged, updateStatus);
17-
client.addHandler(ProviderEvents.ContextChanged, updateStatus);
18-
client.addHandler(ProviderEvents.Error, updateStatus);
19-
client.addHandler(ProviderEvents.Ready, updateStatus);
20-
client.addHandler(ProviderEvents.Stale, updateStatus);
21-
client.addHandler(ProviderEvents.Reconciling, updateStatus);
17+
client.addHandler(ProviderEvents.ConfigurationChanged, updateStatus, { signal: controller.signal });
18+
client.addHandler(ProviderEvents.ContextChanged, updateStatus, { signal: controller.signal });
19+
client.addHandler(ProviderEvents.Error, updateStatus, { signal: controller.signal });
20+
client.addHandler(ProviderEvents.Ready, updateStatus, { signal: controller.signal });
21+
client.addHandler(ProviderEvents.Stale, updateStatus, { signal: controller.signal });
22+
client.addHandler(ProviderEvents.Reconciling, updateStatus, { signal: controller.signal });
2223
return () => {
23-
client.removeHandler(ProviderEvents.ConfigurationChanged, updateStatus);
24-
client.removeHandler(ProviderEvents.ContextChanged, updateStatus);
25-
client.removeHandler(ProviderEvents.Error, updateStatus);
26-
client.removeHandler(ProviderEvents.Ready, updateStatus);
27-
client.removeHandler(ProviderEvents.Stale, updateStatus);
28-
client.removeHandler(ProviderEvents.Reconciling, updateStatus);
24+
controller.abort();
2925
};
3026
}, [client]);
3127

packages/server/src/client/internal/open-feature-client.ts

+8-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import type {
1212
OpenFeatureError,
1313
FlagMetadata,
1414
ResolutionDetails,
15+
EventOptions,
1516
} from '@openfeature/core';
1617
import {
1718
ErrorCode,
@@ -79,7 +80,7 @@ export class OpenFeatureClient implements Client {
7980
return this.providerStatusAccessor();
8081
}
8182

82-
addHandler(eventType: ProviderEvents, handler: EventHandler): void {
83+
addHandler(eventType: ProviderEvents, handler: EventHandler, options?: EventOptions): void {
8384
this.emitterAccessor().addHandler(eventType, handler);
8485
const shouldRunNow = statusMatchesEvent(eventType, this._providerStatus);
8586

@@ -95,6 +96,12 @@ export class OpenFeatureClient implements Client {
9596
this._logger?.error('Error running event handler:', err);
9697
}
9798
}
99+
100+
if (options?.signal && typeof options.signal.addEventListener === 'function') {
101+
options.signal.addEventListener('abort', () => {
102+
this.removeHandler(eventType, handler);
103+
});
104+
}
98105
}
99106

100107
removeHandler(eventType: ProviderEvents, handler: EventHandler) {

packages/server/test/events.spec.ts

+30-1
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,21 @@ describe('Events', () => {
449449
expect(OpenFeature.getHandlers(eventType)).toHaveLength(0);
450450
});
451451

452-
it('The API provides a function allowing the removal of event handlers', () => {
452+
it('The event handler can be removed using an abort signal', () => {
453+
const abortController = new AbortController();
454+
const handler1 = jest.fn();
455+
const handler2 = jest.fn();
456+
const eventType = ProviderEvents.Stale;
457+
458+
OpenFeature.addHandler(eventType, handler1, { signal: abortController.signal });
459+
OpenFeature.addHandler(eventType, handler2);
460+
expect(OpenFeature.getHandlers(eventType)).toHaveLength(2);
461+
462+
abortController.abort();
463+
expect(OpenFeature.getHandlers(eventType)).toHaveLength(1);
464+
});
465+
466+
it('The API provides a function allowing the removal of event handlers from client', () => {
453467
const client = OpenFeature.getClient(domain);
454468
const handler = jest.fn();
455469
const eventType = ProviderEvents.Stale;
@@ -459,6 +473,21 @@ describe('Events', () => {
459473
client.removeHandler(eventType, handler);
460474
expect(client.getHandlers(eventType)).toHaveLength(0);
461475
});
476+
477+
it('The event handler on the client can be removed using an abort signal', () => {
478+
const abortController = new AbortController();
479+
const client = OpenFeature.getClient(domain);
480+
const handler1 = jest.fn();
481+
const handler2 = jest.fn();
482+
const eventType = ProviderEvents.Stale;
483+
484+
client.addHandler(eventType, handler1, { signal: abortController.signal });
485+
client.addHandler(eventType, handler2);
486+
expect(client.getHandlers(eventType)).toHaveLength(2);
487+
488+
abortController.abort();
489+
expect(client.getHandlers(eventType)).toHaveLength(1);
490+
});
462491
});
463492

464493
describe('Requirement 5.3.1', () => {

packages/shared/src/events/eventing.ts

+7
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,17 @@ export type EventDetails<
6666
export type EventHandler<
6767
T extends ServerProviderEvents | ClientProviderEvents = ServerProviderEvents | ClientProviderEvents,
6868
> = (eventDetails?: EventDetails<T>) => Promise<unknown> | unknown;
69+
export type EventOptions = {
70+
signal?: AbortSignal;
71+
};
6972

7073
export interface Eventing<T extends ServerProviderEvents | ClientProviderEvents> {
7174
/**
7275
* Adds a handler for the given provider event type.
7376
* The handlers are called in the order they have been added.
7477
* @param eventType The provider event type to listen to
7578
* @param {EventHandler} handler The handler to run on occurrence of the event type
79+
* @param {EventOptions} options Optional options such as signal for aborting
7680
*/
7781
addHandler(
7882
eventType: T extends ClientProviderEvents
@@ -83,14 +87,17 @@ export interface Eventing<T extends ServerProviderEvents | ClientProviderEvents>
8387
? ClientProviderEvents.ConfigurationChanged
8488
: ServerProviderEvents.ConfigurationChanged
8589
>,
90+
options?: EventOptions,
8691
): void;
8792
addHandler(
8893
eventType: T extends ClientProviderEvents ? ClientNotChangeEvents : ServerNotChangeEvents,
8994
handler: EventHandler<T extends ClientProviderEvents ? ClientNotChangeEvents : ServerNotChangeEvents>,
95+
options?: EventOptions,
9096
): void;
9197
addHandler(
9298
eventType: T extends ClientProviderEvents ? ClientProviderEvents : ServerProviderEvents,
9399
handler: EventHandler<T extends ClientProviderEvents ? ClientProviderEvents : ServerProviderEvents>,
100+
options?: EventOptions,
94101
): void;
95102

96103
/**

packages/shared/src/open-feature.ts

+12-7
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,13 @@ import type {
77
EventDetails,
88
EventHandler,
99
Eventing,
10-
GenericEventEmitter} from './events';
11-
import {
12-
AllProviderEvents,
13-
statusMatchesEvent,
10+
EventOptions,
11+
GenericEventEmitter,
1412
} from './events';
13+
import { AllProviderEvents, statusMatchesEvent } from './events';
1514
import { isDefined } from './filter';
1615
import type { BaseHook, EvaluationLifeCycle } from './hooks';
17-
import type { Logger, ManageLogger} from './logger';
16+
import type { Logger, ManageLogger } from './logger';
1817
import { DefaultLogger, SafeLogger } from './logger';
1918
import type { ClientProviderStatus, CommonProvider, ProviderMetadata, ServerProviderStatus } from './provider';
2019
import { objectOrUndefined, stringOrUndefined } from './type-guards';
@@ -154,8 +153,9 @@ export abstract class OpenFeatureCommonAPI<
154153
* API (global) events run for all providers.
155154
* @param {AnyProviderEvent} eventType The provider event type to listen to
156155
* @param {EventHandler} handler The handler to run on occurrence of the event type
156+
* @param {EventOptions} options Optional options such as signal for aborting
157157
*/
158-
addHandler<T extends AnyProviderEvent>(eventType: T, handler: EventHandler): void {
158+
addHandler<T extends AnyProviderEvent>(eventType: T, handler: EventHandler, options?: EventOptions): void {
159159
[...new Map([[undefined, this._defaultProvider]]), ...this._domainScopedProviders].forEach((keyProviderTuple) => {
160160
const domain = keyProviderTuple[0];
161161
const provider = keyProviderTuple[1].provider;
@@ -173,6 +173,11 @@ export abstract class OpenFeatureCommonAPI<
173173
});
174174

175175
this._apiEmitter.addHandler(eventType, handler);
176+
if (options?.signal && typeof options.signal.addEventListener === 'function') {
177+
options.signal.addEventListener('abort', () => {
178+
this.removeHandler(eventType, handler);
179+
});
180+
}
176181
}
177182

178183
/**
@@ -248,7 +253,7 @@ export abstract class OpenFeatureCommonAPI<
248253
// initialize the provider if it implements "initialize" and it's not already registered
249254
if (typeof provider.initialize === 'function' && !this.allProviders.includes(provider)) {
250255
initializationPromise = provider
251-
.initialize?.(domain ? this._domainScopedContext.get(domain) ?? this._context : this._context)
256+
.initialize?.(domain ? (this._domainScopedContext.get(domain) ?? this._context) : this._context)
252257
?.then(() => {
253258
wrappedProvider.status = this._statusEnumType.READY;
254259
// fetch the most recent event emitters, some may have been added during init

packages/shared/test/events.spec.ts

+10-26
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,23 @@ class TestEventEmitter extends GenericEventEmitter<AnyProviderEvent> {
1414
}
1515
}
1616

17-
// a little function to make sure we're at least waiting for the event loop
17+
// a little function to make sure we're at least waiting for the event loop
1818
// to clear before we start making assertions
1919
const wait = (millis = 0) => {
20-
return new Promise(resolve => {setTimeout(resolve, millis);});
20+
return new Promise((resolve) => {
21+
setTimeout(resolve, millis);
22+
});
2123
};
2224

2325
describe('GenericEventEmitter', () => {
26+
const emitter = new TestEventEmitter();
27+
28+
afterEach(() => {
29+
emitter.removeAllHandlers();
30+
});
31+
2432
describe('addHandler should', function () {
2533
it('attach handler for event type', async function () {
26-
const emitter = new TestEventEmitter();
27-
2834
const handler1 = jest.fn();
2935
emitter.addHandler(AllProviderEvents.Ready, handler1);
3036
emitter.emit(AllProviderEvents.Ready);
@@ -35,8 +41,6 @@ describe('GenericEventEmitter', () => {
3541
});
3642

3743
it('attach several handlers for event type', async function () {
38-
const emitter = new TestEventEmitter();
39-
4044
const handler1 = jest.fn();
4145
const handler2 = jest.fn();
4246
const handler3 = jest.fn();
@@ -64,7 +68,6 @@ describe('GenericEventEmitter', () => {
6468
debug: () => done(),
6569
};
6670

67-
const emitter = new TestEventEmitter();
6871
emitter.setLogger(logger);
6972

7073
emitter.addHandler(AllProviderEvents.Ready, async () => {
@@ -74,8 +77,6 @@ describe('GenericEventEmitter', () => {
7477
});
7578

7679
it('trigger handler for event type', async function () {
77-
const emitter = new TestEventEmitter();
78-
7980
const handler1 = jest.fn();
8081
emitter.addHandler(AllProviderEvents.Ready, handler1);
8182
emitter.emit(AllProviderEvents.Ready);
@@ -87,7 +88,6 @@ describe('GenericEventEmitter', () => {
8788

8889
it('trigger handler for event type with event data', async function () {
8990
const event: ReadyEvent = { message: 'message' };
90-
const emitter = new TestEventEmitter();
9191

9292
const handler1 = jest.fn();
9393
emitter.addHandler(AllProviderEvents.Ready, handler1);
@@ -99,8 +99,6 @@ describe('GenericEventEmitter', () => {
9999
});
100100

101101
it('trigger several handlers for event type', async function () {
102-
const emitter = new TestEventEmitter();
103-
104102
const handler1 = jest.fn();
105103
const handler2 = jest.fn();
106104
const handler3 = jest.fn();
@@ -121,8 +119,6 @@ describe('GenericEventEmitter', () => {
121119

122120
describe('removeHandler should', () => {
123121
it('remove single handler', async function () {
124-
const emitter = new TestEventEmitter();
125-
126122
const handler1 = jest.fn();
127123
emitter.addHandler(AllProviderEvents.Ready, handler1);
128124

@@ -138,8 +134,6 @@ describe('GenericEventEmitter', () => {
138134

139135
describe('removeAllHandlers should', () => {
140136
it('remove all handlers for event type', async function () {
141-
const emitter = new TestEventEmitter();
142-
143137
const handler1 = jest.fn();
144138
const handler2 = jest.fn();
145139
emitter.addHandler(AllProviderEvents.Ready, handler1);
@@ -156,8 +150,6 @@ describe('GenericEventEmitter', () => {
156150
});
157151

158152
it('remove same handler when assigned to multiple events', async function () {
159-
const emitter = new TestEventEmitter();
160-
161153
const handler = jest.fn();
162154
emitter.addHandler(AllProviderEvents.Stale, handler);
163155
emitter.addHandler(AllProviderEvents.ContextChanged, handler);
@@ -174,8 +166,6 @@ describe('GenericEventEmitter', () => {
174166
});
175167

176168
it('allow addition/removal of duplicate handlers', async function () {
177-
const emitter = new TestEventEmitter();
178-
179169
const handler = jest.fn();
180170
emitter.addHandler(AllProviderEvents.Stale, handler);
181171
emitter.addHandler(AllProviderEvents.Stale, handler);
@@ -191,8 +181,6 @@ describe('GenericEventEmitter', () => {
191181
});
192182

193183
it('allow duplicate event handlers and call them', async function () {
194-
const emitter = new TestEventEmitter();
195-
196184
const handler = jest.fn();
197185
emitter.addHandler(AllProviderEvents.Stale, handler);
198186
emitter.addHandler(AllProviderEvents.Stale, handler);
@@ -205,8 +193,6 @@ describe('GenericEventEmitter', () => {
205193
});
206194

207195
it('remove all handlers only for event type', async function () {
208-
const emitter = new TestEventEmitter();
209-
210196
const handler1 = jest.fn();
211197
const handler2 = jest.fn();
212198
emitter.addHandler(AllProviderEvents.Ready, handler1);
@@ -223,8 +209,6 @@ describe('GenericEventEmitter', () => {
223209
});
224210

225211
it('remove all handlers if no event type is given', async function () {
226-
const emitter = new TestEventEmitter();
227-
228212
const handler1 = jest.fn();
229213
const handler2 = jest.fn();
230214
emitter.addHandler(AllProviderEvents.Ready, handler1);

packages/web/src/client/internal/open-feature-client.ts

+8-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import type {
1212
OpenFeatureError,
1313
FlagMetadata,
1414
ResolutionDetails,
15+
EventOptions,
1516
} from '@openfeature/core';
1617
import {
1718
ErrorCode,
@@ -74,7 +75,7 @@ export class OpenFeatureClient implements Client {
7475
return this.providerStatusAccessor();
7576
}
7677

77-
addHandler(eventType: ProviderEvents, handler: EventHandler): void {
78+
addHandler(eventType: ProviderEvents, handler: EventHandler, options: EventOptions): void {
7879
this.emitterAccessor().addHandler(eventType, handler);
7980
const shouldRunNow = statusMatchesEvent(eventType, this.providerStatus);
8081

@@ -90,6 +91,12 @@ export class OpenFeatureClient implements Client {
9091
this._logger?.error('Error running event handler:', err);
9192
}
9293
}
94+
95+
if (options?.signal && typeof options.signal.addEventListener === 'function') {
96+
options.signal.addEventListener('abort', () => {
97+
this.removeHandler(eventType, handler);
98+
});
99+
}
93100
}
94101

95102
removeHandler(notificationType: ProviderEvents, handler: EventHandler): void {

0 commit comments

Comments
 (0)