Skip to content

Conversation

@fadi-george
Copy link
Collaborator

@fadi-george fadi-george commented Oct 24, 2025

Description

1 Line Summary

  • replace ONESIGNAL_EVENTS / events with type union map
// main build/releases/OneSignalSDK.page.js Size limit: 420 B Size: 420 B gzipped build/releases/OneSignalSDK.page.es6.js Size limit: 46.23 kB Size: 46.23 kB gzipped build/releases/OneSignalSDK.sw.js Size limit: 13.44 kB Size: 13.44 kB gzipped build/releases/OneSignalSDK.page.styles.css Size limit: 8.81 kB Size: 8.8 kB gzipped // current branch build/releases/OneSignalSDK.page.js Size limit: 420 B Size: 420 B gzipped build/releases/OneSignalSDK.page.es6.js Size limit: 45.63 kB Size: 45.63 kB gzipped build/releases/OneSignalSDK.sw.js Size limit: 13.44 kB Size: 13.44 kB gzipped build/releases/OneSignalSDK.page.styles.css Size limit: 8.81 kB Size: 8.8 kB gzipped 

Details

  • removes ONESIGNAL_EVENTS
  • adds new EventsMap interface/type(services/types.ts) and update Emitter to use the events type mapping
  • removes confirmation toast events field
  • updates triggerUserChanged to use UserNamespace._emitter._emit instead of OneSignalEvent so that we can avoid passing an emitter as a param

Systems Affected

  • WebSDK
  • Backend
  • Dashboard

Validation

Tests

Can add this to index.html for testing:

 OneSignalDeferred.push(function (OneSignal) { OneSignal.User.PushSubscription.addEventListener('change', (e) => { console.log('PushSubscription.addEventListener', e); }); OneSignal.Notifications.addEventListener('change', (e) => { console.log('Notifications.addEventListener', e); }); OneSignal.Notifications.addEventListener('permissionChange', (e) => { console.log('permissionChange', e); }); OneSignal.User.addEventListener('change', (e) => { console.log('User.addEventListener', e); }); }); 

New

Screenshot 2025-10-24 at 11 11 48 AM

Main

Screenshot 2025-10-24 at 11 14 44 AM

Info

Checklist

  • All the automated tests pass or I explained why that is not possible
  • I have personally tested this on my machine or explained why that is not possible
  • I have included test coverage for these changes or explained why they are not needed

Programming Checklist
Interfaces:

  • Don't use default export
  • New interfaces are in model files

Functions:

  • Don't use default export
  • All function signatures have return types
  • Helpers should not access any data but rather be given the data to operate on.

Typescript:

  • No Typescript warnings
  • Avoid silencing null/undefined warnings with the exclamation point

Other:

  • Iteration: refrain from using elem of array syntax. Prefer forEach or use map
  • Avoid using global OneSignal accessor for context if possible. Instead, we can pass it to function/constructor so that we don't call OneSignal.context

Screenshots

Info

Checklist

  • I have included screenshots/recordings of the intended results or explained why they are not needed

Related Tickets



This change is Reviewable

@fadi-george fadi-george requested review from jkasten2 and sherwinski and removed request for sherwinski October 24, 2025 18:04
Copy link
Contributor

@sherwinski sherwinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand wanting to cut down on bundle size by way of getting rid of OneSignalEvent.ts but is it worth the hit to developer experience to go from OneSignal.EVENTS.SDK_INITIALIZED to 'initialized'? The former offers discoverability while the latter requires you to remember it.

In a similar vein, __test__/unit/events/eventsUnique.test.ts was useful because it prevented duplicate event names. In this approach, we lose that safety.

Looking at the screenshots, I'm not sure I see a difference between new and main. Is it meant to confirm that it was a refactor with no noticeable changes?

await this._mountAuxiliaryContainers(options);
Log._debug('Showing OneSignal Slidedown');
Slidedown._triggerSlidedownEvent(Slidedown.EVENTS.SHOWN);
Slidedown._triggerSlidedownEvent('slidedownShown');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this need to be passed a boolean?

slidedownShown: boolean;
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants