-
Notifications
You must be signed in to change notification settings - Fork 0
feat(events): Use new event dispatcher API #103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,7 +59,7 @@ | |
"webpack-cli": "^4.10.0" | ||
}, | ||
"dependencies": { | ||
"@eppo/js-client-sdk-common": "4.3.0" | ||
"@eppo/js-client-sdk-common": "https://github.com/Eppo-exp/js-sdk-common.git#66aa69971c488927297e7aa611300d6dda0fd706" | ||
}, | ||
"packageManager": "[email protected]+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,140 @@ | ||
import BrowserNetworkStatusListener from './browser-network-status-listener'; | ||
|
||
describe('BrowserNetworkStatusListener', () => { | ||
let originalNavigator: Navigator; | ||
let originalWindow: Window; | ||
|
||
beforeEach(() => { | ||
// Save original references | ||
originalNavigator = global.navigator; | ||
originalWindow = global.window; | ||
|
||
// Mock `navigator.onLine` | ||
Object.defineProperty(global, 'navigator', { | ||
value: { onLine: true }, | ||
writable: true, | ||
}); | ||
|
||
const listeners: Map<string, (offline: boolean) => void> = new Map(); | ||
Object.defineProperty(global, 'window', { | ||
value: { | ||
addEventListener: (evt: string, fn: () => void) => { | ||
listeners.set(evt, fn); | ||
}, | ||
removeEventListener: () => null, | ||
dispatchEvent: (event: Event) => { | ||
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion | ||
const listener = listeners.get(event.type)!; | ||
listener(event.type === 'offline'); | ||
}, | ||
}, | ||
writable: true, | ||
}); | ||
}); | ||
|
||
afterEach(() => { | ||
// Restore original references | ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-ignore test code | ||
// noinspection JSConstantReassignment | ||
global.navigator = originalNavigator; | ||
// noinspection JSConstantReassignment | ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-ignore test code | ||
// noinspection JSConstantReassignment | ||
global.window = originalWindow; | ||
|
||
jest.clearAllMocks(); | ||
}); | ||
|
||
test('throws an error if instantiated outside a browser environment', () => { | ||
Object.defineProperty(global, 'window', { value: undefined }); | ||
|
||
expect(() => new BrowserNetworkStatusListener()).toThrow( | ||
'BrowserNetworkStatusListener can only be used in a browser environment', | ||
); | ||
}); | ||
|
||
test('correctly initializes offline state based on navigator.onLine', () => { | ||
// Online state | ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-ignore test code | ||
// noinspection JSConstantReassignment | ||
navigator.onLine = true; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎉 despite the awful |
||
const listener = new BrowserNetworkStatusListener(); | ||
expect(listener.isOffline()).toBe(false); | ||
|
||
// Offline state | ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-ignore | ||
// noinspection JSConstantReassignment | ||
navigator.onLine = false; | ||
const offlineListener = new BrowserNetworkStatusListener(); | ||
expect(offlineListener.isOffline()).toBe(true); | ||
}); | ||
|
||
test('notifies listeners when offline event is triggered', async () => { | ||
const listener = new BrowserNetworkStatusListener(); | ||
const mockCallback = jest.fn(); | ||
|
||
listener.onNetworkStatusChange(mockCallback); | ||
|
||
// Simulate offline event | ||
const offlineEvent = new Event('offline'); | ||
window.dispatchEvent(offlineEvent); | ||
await new Promise((resolve) => setTimeout(resolve, 200)); | ||
expect(mockCallback).toHaveBeenCalledWith(true); | ||
}); | ||
|
||
test('notifies listeners when online event is triggered', async () => { | ||
const listener = new BrowserNetworkStatusListener(); | ||
const mockCallback = jest.fn(); | ||
|
||
listener.onNetworkStatusChange(mockCallback); | ||
|
||
// Simulate online event | ||
const onlineEvent = new Event('online'); | ||
window.dispatchEvent(onlineEvent); | ||
await new Promise((resolve) => setTimeout(resolve, 200)); | ||
expect(mockCallback).toHaveBeenCalledWith(false); | ||
}); | ||
|
||
test('removes listeners and does not notify them after removal', () => { | ||
const listener = new BrowserNetworkStatusListener(); | ||
const mockCallback = jest.fn(); | ||
|
||
listener.onNetworkStatusChange(mockCallback); | ||
listener.removeNetworkStatusChange(mockCallback); | ||
|
||
// Simulate offline event | ||
const offlineEvent = new Event('offline'); | ||
window.dispatchEvent(offlineEvent); | ||
|
||
expect(mockCallback).not.toHaveBeenCalled(); | ||
}); | ||
|
||
test('debounces notifications for rapid online/offline changes', () => { | ||
jest.useFakeTimers(); | ||
const listener = new BrowserNetworkStatusListener(); | ||
const mockCallback = jest.fn(); | ||
|
||
listener.onNetworkStatusChange(mockCallback); | ||
|
||
// Simulate rapid online/offline changes | ||
const offlineEvent = new Event('offline'); | ||
const onlineEvent = new Event('online'); | ||
window.dispatchEvent(offlineEvent); | ||
window.dispatchEvent(onlineEvent); | ||
|
||
// Fast-forward time by less than debounce duration | ||
jest.advanceTimersByTime(100); | ||
|
||
expect(mockCallback).not.toHaveBeenCalled(); | ||
|
||
// Fast-forward time past debounce duration | ||
jest.advanceTimersByTime(200); | ||
|
||
expect(mockCallback).toHaveBeenCalledWith(false); // Online state | ||
jest.useRealTimers(); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
import { NetworkStatusListener } from '@eppo/js-client-sdk-common'; | ||
|
||
const debounceDurationMs = 200; | ||
|
||
/** A NetworkStatusListener that listens for online/offline events in the browser. */ | ||
export default class BrowserNetworkStatusListener implements NetworkStatusListener { | ||
private readonly listeners: ((isOffline: boolean) => void)[] = []; | ||
private _isOffline: boolean; | ||
private debounceTimer: NodeJS.Timeout | null = null; | ||
|
||
constructor() { | ||
if (typeof window === 'undefined') { | ||
throw new Error('BrowserNetworkStatusListener can only be used in a browser environment'); | ||
} | ||
// guard against navigator API not being available (oder browsers) | ||
// noinspection SuspiciousTypeOfGuard | ||
this._isOffline = typeof navigator.onLine === 'boolean' ? !navigator.onLine : false; | ||
window.addEventListener('offline', () => this.notifyListeners(true)); | ||
window.addEventListener('online', () => this.notifyListeners(false)); | ||
} | ||
|
||
isOffline(): boolean { | ||
return this._isOffline; | ||
} | ||
|
||
onNetworkStatusChange(callback: (isOffline: boolean) => void): void { | ||
this.listeners.push(callback); | ||
} | ||
|
||
removeNetworkStatusChange(callback: (isOffline: boolean) => void): void { | ||
const index = this.listeners.indexOf(callback); | ||
if (index !== -1) { | ||
this.listeners.splice(index, 1); | ||
} | ||
} | ||
|
||
private notifyListeners(isOffline: boolean): void { | ||
if (this.debounceTimer) { | ||
clearTimeout(this.debounceTimer); | ||
} | ||
this.debounceTimer = setTimeout(() => { | ||
this._isOffline = isOffline; | ||
[...this.listeners].forEach((listener) => listener(isOffline)); | ||
}, debounceDurationMs); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
/** | ||
* @jest-environment jsdom | ||
*/ | ||
|
||
import LocalStorageBackedNamedEventQueue from './local-storage-backed-named-event-queue'; | ||
|
||
describe('LocalStorageBackedNamedEventQueue', () => { | ||
const queueName = 'testQueue'; | ||
let queue: LocalStorageBackedNamedEventQueue<string>; | ||
|
||
beforeEach(() => { | ||
localStorage.clear(); | ||
queue = new LocalStorageBackedNamedEventQueue(queueName); | ||
}); | ||
|
||
it('should initialize with an empty queue', () => { | ||
expect(queue.length).toBe(0); | ||
}); | ||
|
||
it('should persist and retrieve events correctly via push and iterator', () => { | ||
queue.push('event1'); | ||
queue.push('event2'); | ||
|
||
expect(queue.length).toBe(2); | ||
|
||
const events = Array.from(queue); | ||
expect(events).toEqual(['event1', 'event2']); | ||
}); | ||
|
||
it('should persist and retrieve events correctly via push and shift', () => { | ||
queue.push('event1'); | ||
queue.push('event2'); | ||
|
||
const firstEvent = queue.shift(); | ||
expect(firstEvent).toBe('event1'); | ||
expect(queue.length).toBe(1); | ||
|
||
const secondEvent = queue.shift(); | ||
expect(secondEvent).toBe('event2'); | ||
expect(queue.length).toBe(0); | ||
}); | ||
|
||
it('should remove events from localStorage after shift', () => { | ||
queue.push('event1'); | ||
const eventKey = Object.keys(localStorage).find( | ||
(key) => key.includes(queueName) && localStorage.getItem(key)?.includes('event1'), | ||
); | ||
|
||
expect(eventKey).toBeDefined(); | ||
queue.shift(); | ||
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion | ||
expect(localStorage.getItem(eventKey!)).toBeNull(); | ||
}); | ||
|
||
it('should reconstruct the queue from localStorage', () => { | ||
queue.push('event1'); | ||
queue.push('event2'); | ||
|
||
const newQueueInstance = new LocalStorageBackedNamedEventQueue<string>(queueName); | ||
expect(newQueueInstance.length).toBe(2); | ||
|
||
const events = Array.from(newQueueInstance); | ||
expect(events).toEqual(['event1', 'event2']); | ||
}); | ||
|
||
it('should handle empty shift gracefully', () => { | ||
expect(queue.shift()).toBeUndefined(); | ||
}); | ||
|
||
it('should not fail if localStorage state is corrupted', () => { | ||
localStorage.setItem(`eventQueue:${queueName}`, '{ corrupted state }'); | ||
|
||
const newQueueInstance = new LocalStorageBackedNamedEventQueue<string>(queueName); | ||
expect(newQueueInstance.length).toBe(0); | ||
}); | ||
|
||
it('should handle events with the same content correctly using consistent hashing', () => { | ||
queue.push('event1'); | ||
queue.push('event1'); // Push the same event content twice | ||
|
||
expect(queue.length).toBe(2); | ||
|
||
const events = Array.from(queue); | ||
expect(events).toEqual(['event1', 'event1']); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,93 @@ | ||
import { applicationLogger } from '@eppo/js-client-sdk-common'; | ||
import NamedEventQueue from '@eppo/js-client-sdk-common/dist/events/named-event-queue'; | ||
|
||
import { takeWhile } from '../util'; | ||
|
||
/** A localStorage-backed NamedEventQueue. */ | ||
export default class LocalStorageBackedNamedEventQueue<T> implements NamedEventQueue<T> { | ||
private readonly localStorageKey: string; | ||
private eventKeys: string[] = []; | ||
|
||
constructor(public readonly name: string) { | ||
this.localStorageKey = `eventQueue:${this.name}`; | ||
this.loadStateFromLocalStorage(); | ||
} | ||
|
||
splice(count: number): T[] { | ||
const arr = Array.from({ length: count }, () => this.shift()); | ||
return takeWhile(arr, (item) => item !== undefined) as T[]; | ||
} | ||
|
||
isEmpty(): boolean { | ||
return this.length === 0; | ||
} | ||
|
||
get length(): number { | ||
return this.eventKeys.length; | ||
} | ||
|
||
push(event: T): void { | ||
const eventKey = this.generateEventKey(event); | ||
const serializedEvent = JSON.stringify(event); | ||
localStorage.setItem(eventKey, serializedEvent); | ||
this.eventKeys.push(eventKey); | ||
this.saveStateToLocalStorage(); | ||
} | ||
|
||
*[Symbol.iterator](): IterableIterator<T> { | ||
for (const key of this.eventKeys) { | ||
const eventData = localStorage.getItem(key); | ||
if (eventData) { | ||
yield JSON.parse(eventData); | ||
} | ||
} | ||
} | ||
|
||
shift(): T | undefined { | ||
if (this.eventKeys.length === 0) { | ||
return undefined; | ||
} | ||
const eventKey = this.eventKeys.shift()!; | ||
Check warning on line 50 in src/events/local-storage-backed-named-event-queue.ts
|
||
const eventData = localStorage.getItem(eventKey); | ||
if (eventData) { | ||
localStorage.removeItem(eventKey); | ||
this.saveStateToLocalStorage(); | ||
return JSON.parse(eventData); | ||
} | ||
return undefined; | ||
} | ||
|
||
private loadStateFromLocalStorage(): void { | ||
const serializedState = localStorage.getItem(this.localStorageKey); | ||
if (serializedState) { | ||
try { | ||
this.eventKeys = JSON.parse(serializedState); | ||
} catch { | ||
applicationLogger.error( | ||
`Failed to parse event queue ${this.name} state. Initializing empty queue.`, | ||
); | ||
Comment on lines
+66
to
+68
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I appreciate the fallback modes (vs exploding) |
||
this.eventKeys = []; | ||
} | ||
} | ||
} | ||
|
||
private saveStateToLocalStorage(): void { | ||
const serializedState = JSON.stringify(this.eventKeys); | ||
localStorage.setItem(this.localStorageKey, serializedState); | ||
} | ||
|
||
private generateEventKey(event: T): string { | ||
const hash = this.hashEvent(event); | ||
return `eventQueue:${this.name}:${hash}`; | ||
} | ||
|
||
private hashEvent(event: T): string { | ||
const serializedEvent = JSON.stringify(event); | ||
let hash = 0; | ||
for (let i = 0; i < serializedEvent.length; i++) { | ||
hash = (hash << 5) - hash + serializedEvent.charCodeAt(i); | ||
hash |= 0; // Convert to 32bit integer | ||
} | ||
Comment on lines
+87
to
+90
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason we're doing this ourselves vs some hashing library? Or reusing the hashing code we do for shard numbers? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good point, actually no reason. I'll look into reusing that |
||
return hash.toString(36); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a reminder not to check this in