diff --git a/src/events/EventManager.test.ts b/src/events/EventManager.test.ts index f8e7af2d..30a54443 100644 --- a/src/events/EventManager.test.ts +++ b/src/events/EventManager.test.ts @@ -95,6 +95,29 @@ describe('EventManager', () => { new EventManager(null as never); expect(freshModule.onPermissionChanged).not.toHaveBeenCalled(); }); + + test('should not subscribe again when called on an already subscribed manager', () => { + eventManager.setupListeners(); + + expect(mockModule.onPermissionChanged).toHaveBeenCalledOnce(); + expect(mockModule.onNotificationWillDisplay).toHaveBeenCalledOnce(); + expect(eventManager['nativeSubscriptions'].length).toBe(10); + }); + }); + + describe('clearListeners', () => { + test('should remove native subscriptions and drop handlers', () => { + const subscriptions = eventManager['nativeSubscriptions'].slice(); + eventManager.addEventListener(PERMISSION_CHANGED, vi.fn()); + + eventManager.clearListeners(); + + subscriptions.forEach((sub) => { + expect(sub.remove).toHaveBeenCalledOnce(); + }); + expect(eventManager['nativeSubscriptions'].length).toBe(0); + expect(eventManager['eventListenerArrayMap'].size).toBe(0); + }); }); describe('addEventListener', () => { @@ -513,6 +536,29 @@ describe('EventManager', () => { expect(handler2).not.toHaveBeenCalled(); expect(handler3).toHaveBeenCalledWith(pushChangedPayload); }); + + test('should still reach later handlers when one removes itself mid-dispatch', () => { + const selfRemoving = vi.fn(() => { + eventManager.removeEventListener(SUBSCRIPTION_CHANGED, selfRemoving); + }); + const next = vi.fn(); + + eventManager.addEventListener(SUBSCRIPTION_CHANGED, selfRemoving); + eventManager.addEventListener(SUBSCRIPTION_CHANGED, next); + + const emitCallback = callbacks.get('onSubscriptionChanged')!; + emitCallback(pushChangedPayload); + + expect(selfRemoving).toHaveBeenCalledOnce(); + expect(next).toHaveBeenCalledWith(pushChangedPayload); + + selfRemoving.mockClear(); + next.mockClear(); + emitCallback(pushChangedPayload); + + expect(selfRemoving).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledOnce(); + }); }); }); diff --git a/src/events/EventManager.ts b/src/events/EventManager.ts index e2cfc9f9..9cd81680 100644 --- a/src/events/EventManager.ts +++ b/src/events/EventManager.ts @@ -61,7 +61,7 @@ export default class EventManager { } setupListeners() { - if (this.RNOneSignal == null) return; + if (this.RNOneSignal == null || this.nativeSubscriptions.length > 0) return; this.nativeSubscriptions.push( this.RNOneSignal.onPermissionChanged((payload) => { @@ -156,7 +156,7 @@ export default class EventManager { private dispatchHandlers(eventName: string, payload: unknown) { const handlerArray = this.eventListenerArrayMap.get(eventName); if (handlerArray) { - handlerArray.forEach((handler) => { + handlerArray.slice().forEach((handler) => { handler(payload); }); } diff --git a/src/index.test.ts b/src/index.test.ts index ef6a3a88..99bc865f 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -33,14 +33,23 @@ const isValidCallbackSpy = vi.spyOn(helpers, 'isValidCallback'); const addEventManagerListenerSpy = vi.spyOn(EventManager.prototype, 'addEventListener'); const removeEventManagerListenerSpy = vi.spyOn(EventManager.prototype, 'removeEventListener'); -const filterEventListener = ( +const GLOBAL_KEY = '__oneSignalEventManager'; + +const dispatchEvent = ( eventName: K, -): EventListenerMap[K] => { - return addEventManagerListenerSpy.mock.calls.filter( - (call: [string, unknown]) => call[0] === eventName, - )[0][1] as EventListenerMap[K]; + payload: Parameters[0], +) => { + const manager = (globalThis as Record)[GLOBAL_KEY] as EventManager; + manager['eventListenerArrayMap'] + .get(eventName) + ?.slice() + .forEach((handler) => { + handler(payload); + }); }; +const flushPromises = () => new Promise((resolve) => setTimeout(resolve, 0)); + describe('OneSignal', () => { beforeEach(() => { mockPlatform.OS = 'ios'; @@ -69,8 +78,7 @@ describe('OneSignal', () => { expect(mockRNOneSignal.initialize).toHaveBeenCalledWith(APP_ID); // test permission change listener - const changeFn = filterEventListener(PERMISSION_CHANGED); - changeFn(true); + dispatchEvent(PERMISSION_CHANGED, true); const permission = OneSignal.Notifications.hasPermission(); expect(permission).toBe(true); @@ -87,13 +95,12 @@ describe('OneSignal', () => { optedIn: true, }, }; - const subscriptionChangeFn = filterEventListener(SUBSCRIPTION_CHANGED); - subscriptionChangeFn(pushData); + dispatchEvent(SUBSCRIPTION_CHANGED, pushData); const pushSubscription = OneSignal.User.pushSubscription.getPushSubscriptionId(); expect(pushSubscription).toBe('subscription-id'); // reset push subscription - subscriptionChangeFn({ + dispatchEvent(SUBSCRIPTION_CHANGED, { ...pushData, current: { id: '', token: '', optedIn: false }, }); @@ -104,6 +111,62 @@ describe('OneSignal', () => { OneSignal.initialize(APP_ID); expect(mockRNOneSignal.initialize).not.toHaveBeenCalled(); }); + + test('should keep a permission event that arrives before the startup read resolves', async () => { + let resolveStartupRead: ((granted: boolean) => void) | undefined; + vi.mocked(mockRNOneSignal.hasNotificationPermission).mockReturnValueOnce( + new Promise((resolve) => { + resolveStartupRead = resolve; + }), + ); + + OneSignal.initialize(APP_ID); + dispatchEvent(PERMISSION_CHANGED, true); + resolveStartupRead!(false); + await flushPromises(); + + expect(OneSignal.Notifications.hasPermission()).toBe(true); + + dispatchEvent(PERMISSION_CHANGED, false); + }); + + test('should keep a subscription event that arrives before the startup reads resolve', async () => { + let resolveStartupRead: ((id: string) => void) | undefined; + vi.mocked(mockRNOneSignal.getPushSubscriptionId).mockReturnValueOnce( + new Promise((resolve) => { + resolveStartupRead = resolve; + }), + ); + + OneSignal.initialize(APP_ID); + dispatchEvent(SUBSCRIPTION_CHANGED, { + previous: { id: '', token: '', optedIn: false }, + current: { id: PUSH_ID, token: PUSH_TOKEN, optedIn: true }, + }); + resolveStartupRead!('stale-id'); + await flushPromises(); + + expect(OneSignal.User.pushSubscription.getPushSubscriptionId()).toBe(PUSH_ID); + + dispatchEvent(SUBSCRIPTION_CHANGED, { + previous: { id: PUSH_ID, token: PUSH_TOKEN, optedIn: true }, + current: { id: '', token: '', optedIn: false }, + }); + }); + + test('should warn instead of rejecting when a startup read fails', async () => { + vi.mocked(mockRNOneSignal.hasNotificationPermission).mockRejectedValueOnce( + new Error('native failure'), + ); + + OneSignal.initialize(APP_ID); + await flushPromises(); + + expect(console.warn).toHaveBeenCalledWith( + 'OneSignal: failed to read initial state', + expect.any(Error), + ); + }); }); describe('login', () => { diff --git a/src/index.ts b/src/index.ts index 1b4770a7..212299d1 100644 --- a/src/index.ts +++ b/src/index.ts @@ -58,6 +58,10 @@ export enum LogLevel { } let notificationPermission = false; +let permissionObserverAdded = false; +let subscriptionObserverAdded = false; +let permissionVersion = 0; +let subscriptionVersion = 0; let pushSub: PushSubscriptionState = { id: '', @@ -66,21 +70,37 @@ let pushSub: PushSubscriptionState = { }; async function _addPermissionObserver() { - OneSignal.Notifications.addEventListener('permissionChange', (granted: boolean) => { - notificationPermission = granted; - }); + const version = ++permissionVersion; + if (!permissionObserverAdded) { + OneSignal.Notifications.addEventListener('permissionChange', (granted: boolean) => { + permissionVersion++; + notificationPermission = granted; + }); + permissionObserverAdded = true; + } - notificationPermission = await RNOneSignal.hasNotificationPermission(); + const permission = await RNOneSignal.hasNotificationPermission(); + if (version === permissionVersion) notificationPermission = permission; } async function _addPushSubscriptionObserver() { - OneSignal.User.pushSubscription.addEventListener('change', (subscriptionChange) => { - pushSub = subscriptionChange.current; - }); + const version = ++subscriptionVersion; + if (!subscriptionObserverAdded) { + OneSignal.User.pushSubscription.addEventListener('change', (subscriptionChange) => { + subscriptionVersion++; + pushSub = subscriptionChange.current; + }); + subscriptionObserverAdded = true; + } - pushSub.id = (await RNOneSignal.getPushSubscriptionId()) ?? undefined; - pushSub.token = (await RNOneSignal.getPushSubscriptionToken()) ?? undefined; - pushSub.optedIn = await RNOneSignal.getOptedIn(); + const [id, token, optedIn] = await Promise.all([ + RNOneSignal.getPushSubscriptionId(), + RNOneSignal.getPushSubscriptionToken(), + RNOneSignal.getOptedIn(), + ]); + if (version === subscriptionVersion) { + pushSub = { id: id ?? undefined, token: token ?? undefined, optedIn }; + } } export namespace OneSignal { @@ -90,8 +110,9 @@ export namespace OneSignal { RNOneSignal.initialize(appId); - void _addPermissionObserver(); - void _addPushSubscriptionObserver(); + void Promise.all([_addPermissionObserver(), _addPushSubscriptionObserver()]).catch((error) => { + console.warn('OneSignal: failed to read initial state', error); + }); } /**