Skip to content

Commit

Permalink
feat: Remove event target shim. (#545)
Browse files Browse the repository at this point in the history
Unlike the EventEmitter in the node SDK the EventTarget was not
providing an interface directly on the LDClient. The LDClient has its
own off/on interface which is not directly implemented via an event
target. Which means removing it simplifies the implementation instead of
complicating it.

Second it is in the common code and node in a leaf implementation. Which
means it requires a polyfill where it is not supported. Like when using
hermes with React Native.

Typically we dispatch using a micro-task, but EventTarget dispatches
synchronously. For now this maintains the synchronous behavior.

Fixes: #412
  • Loading branch information
kinyoklion authored Aug 15, 2024
1 parent c04a9fd commit 448ad67
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 90 deletions.
3 changes: 1 addition & 2 deletions packages/sdk/react-native/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@
"dependencies": {
"@launchdarkly/js-client-sdk-common": "1.3.0",
"@react-native-async-storage/async-storage": "^1.21.0",
"base64-js": "^1.5.1",
"event-target-shim": "^6.0.2"
"base64-js": "^1.5.1"
},
"devDependencies": {
"@launchdarkly/private-js-mocks": "0.0.1",
Expand Down
3 changes: 0 additions & 3 deletions packages/sdk/react-native/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,9 @@
*
* @packageDocumentation
*/
import { setupPolyfill } from './polyfills';
import ReactNativeLDClient from './ReactNativeLDClient';
import RNOptions from './RNOptions';

setupPolyfill();

export * from '@launchdarkly/js-client-sdk-common';

export * from './hooks';
Expand Down
23 changes: 0 additions & 23 deletions packages/sdk/react-native/src/polyfills/CustomEvent.ts

This file was deleted.

11 changes: 1 addition & 10 deletions packages/sdk/react-native/src/polyfills/index.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,4 @@
import EventTarget from 'event-target-shim';

import { type Hasher, sha256 } from '../fromExternal/js-sha256';
import { base64FromByteArray, btoa } from './btoa';
import CustomEvent from './CustomEvent';

function setupPolyfill() {
Object.assign(global, {
EventTarget,
CustomEvent,
});
}
export { base64FromByteArray, btoa, type Hasher, setupPolyfill, sha256 };
export { base64FromByteArray, btoa, type Hasher, sha256 };
29 changes: 24 additions & 5 deletions packages/shared/sdk-client/src/api/LDEmitter.test.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,21 @@
import { LDContext } from '@launchdarkly/js-sdk-common';
import { LDContext, LDLogger } from '@launchdarkly/js-sdk-common';

import LDEmitter from './LDEmitter';

describe('LDEmitter', () => {
const error = { type: 'network', message: 'unreachable' };
let emitter: LDEmitter;
let logger: LDLogger;

beforeEach(() => {
jest.resetAllMocks();
emitter = new LDEmitter();
logger = {
debug: jest.fn(),
info: jest.fn(),
warn: jest.fn(),
error: jest.fn(),
};
emitter = new LDEmitter(logger);
});

test('subscribe and handle', () => {
Expand Down Expand Up @@ -77,12 +84,13 @@ describe('LDEmitter', () => {

test('on listener with arguments', () => {
const context = { kind: 'user', key: 'test-user-1' };
const onListener = jest.fn((c: LDContext) => c);
const arg2 = { test: 'test' };
const onListener = jest.fn((c: LDContext, a2: any) => [c, a2]);

emitter.on('change', onListener);
emitter.emit('change', context);
emitter.emit('change', context, arg2);

expect(onListener).toBeCalledWith(context);
expect(onListener).toBeCalledWith(context, arg2);
});

test('unsubscribe one of many listeners', () => {
Expand Down Expand Up @@ -131,4 +139,15 @@ describe('LDEmitter', () => {
expect(errorHandler1).not.toBeCalled();
expect(errorHandler2).not.toBeCalled();
});

it('handles errors generated by the callback', () => {
emitter.on('error', () => {
throw new Error('toast');
});
// Should not have an uncaught exception.
emitter.emit('error');
expect(logger.error).toHaveBeenCalledWith(
'Encountered error invoking handler for "error", detail: "Error: toast"',
);
});
});
64 changes: 17 additions & 47 deletions packages/shared/sdk-client/src/api/LDEmitter.ts
Original file line number Diff line number Diff line change
@@ -1,53 +1,20 @@
import { LDLogger } from '@launchdarkly/js-sdk-common';

export type EventName = 'error' | 'change';

type CustomEventListeners = {
original: Function;
custom: Function;
};
/**
* Native api usage: EventTarget.
*
* This is an event emitter using the standard built-in EventTarget web api.
* https://developer.mozilla.org/en-US/docs/Web/API/EventTarget
*
* In react-native use event-target-shim to polyfill EventTarget. This is safe
* because the react-native repo uses it too.
* https://github.com/mysticatea/event-target-shim
*/
export default class LDEmitter {
private et: EventTarget = new EventTarget();
private listeners: Map<EventName, Function[]> = new Map();

private listeners: Map<EventName, CustomEventListeners[]> = new Map();
constructor(private logger?: LDLogger) {}

/**
* Cache all listeners in a Map so we can remove them later
* @param name string event name
* @param originalListener pointer to the original function as specified by
* the consumer
* @param customListener pointer to the custom function based on original
* listener. This is needed to allow for CustomEvents.
* @private
*/
private saveListener(name: EventName, originalListener: Function, customListener: Function) {
const listener = { original: originalListener, custom: customListener };
on(name: EventName, listener: Function) {
if (!this.listeners.has(name)) {
this.listeners.set(name, [listener]);
} else {
this.listeners.get(name)?.push(listener);
}
}

on(name: EventName, listener: Function) {
const customListener = (e: Event) => {
const { detail } = e as CustomEvent;

// invoke listener with args from CustomEvent
listener(...detail);
};
this.saveListener(name, listener, customListener);
this.et.addEventListener(name, customListener);
}

/**
* Unsubscribe one or all events.
*
Expand All @@ -61,11 +28,8 @@ export default class LDEmitter {
}

if (listener) {
const toBeRemoved = existingListeners.find((c) => c.original === listener);
this.et.removeEventListener(name, toBeRemoved?.custom as any);

// remove from internal cache
const updated = existingListeners.filter((l) => l.original !== listener);
const updated = existingListeners.filter((fn) => fn !== listener);
if (updated.length === 0) {
this.listeners.delete(name);
} else {
Expand All @@ -74,15 +38,21 @@ export default class LDEmitter {
return;
}

// remove all listeners
existingListeners.forEach((l) => {
this.et.removeEventListener(name, l.custom as any);
});
// listener was not specified, so remove them all for that event
this.listeners.delete(name);
}

private invokeListener(listener: Function, name: EventName, ...detail: any[]) {
try {
listener(...detail);
} catch (err) {
this.logger?.error(`Encountered error invoking handler for "${name}", detail: "${err}"`);
}
}

emit(name: EventName, ...detail: any[]) {
this.et.dispatchEvent(new CustomEvent(name, { detail }));
const listeners = this.listeners.get(name);
listeners?.forEach((listener) => this.invokeListener(listener, name, ...detail));
}

eventNames(): string[] {
Expand Down

0 comments on commit 448ad67

Please sign in to comment.