Skip to content

chore: more size reduction #1401

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Oct 24, 2025
Merged

chore: more size reduction #1401

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c655d58
more prefixing of methods
fadi-george Oct 15, 2025
3476ce2
break up sw static class methods
fadi-george Oct 15, 2025
7641df6
break up tag utils
fadi-george Oct 15, 2025
cf34e45
more renames
fadi-george Oct 15, 2025
71785c1
breakup limit store
fadi-george Oct 15, 2025
8620cee
rename limit store
fadi-george Oct 15, 2025
e9bdc4b
more method renames
fadi-george Oct 15, 2025
8f8cfbd
breakup main helpers and remove unused functions
fadi-george Oct 15, 2025
853472e
address feedback
fadi-george Oct 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion __test__/support/helpers/setup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ export const setupSubscriptionModel = async (
export const setupLoadStylesheet = async () => {
vi.spyOn(
OneSignal._context._dynamicResourceLoader,
'loadSdkStylesheet',
'_loadSdkStylesheet',
).mockResolvedValue(ResourceLoadState.Loaded);
};

Expand Down
36 changes: 16 additions & 20 deletions __test__/unit/models/path.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,40 +3,42 @@ import Path from '../../../src/shared/models/Path';
describe('Path tests', () => {
test(`should return correct components for a simple web path`, () => {
const path = new Path('/web-folder/assets/service-worker.js');
expect(path.getFileName()).toBe('service-worker.js');
expect(path.getFullPath()).toBe('/web-folder/assets/service-worker.js');
expect(path._getFileName()).toBe('service-worker.js');
expect(path._getFullPath()).toBe('/web-folder/assets/service-worker.js');
});

test(`should return correct components for a file-based path`, () => {
const path = new Path('file:///c:/web-folder/assets/service-worker.js');
expect(path.getFileName()).toBe('service-worker.js');
expect(path.getFullPath()).toBe(
expect(path._getFileName()).toBe('service-worker.js');
expect(path._getFullPath()).toBe(
'file:///c:/web-folder/assets/service-worker.js',
);
});

test(`should return case-sensitive correct components for a file-based path`, () => {
const path = new Path('/WeB-FoLdEr/AsSeTs/SeRvIcE-WoRkEr.js');
expect(path.getFileName()).toBe('SeRvIcE-WoRkEr.js');
expect(path.getFullPath()).toBe('/WeB-FoLdEr/AsSeTs/SeRvIcE-WoRkEr.js');
expect(path._getFileName()).toBe('SeRvIcE-WoRkEr.js');
expect(path._getFullPath()).toBe('/WeB-FoLdEr/AsSeTs/SeRvIcE-WoRkEr.js');
});

test(`should return correct components for a double-extension path`, () => {
const path = new Path('/web-folder/assets/service-worker.js.php');
expect(path.getFileName()).toBe('service-worker.js.php');
expect(path.getFullPath()).toBe('/web-folder/assets/service-worker.js.php');
expect(path._getFileName()).toBe('service-worker.js.php');
expect(path._getFullPath()).toBe(
'/web-folder/assets/service-worker.js.php',
);
});

test(`should return correct components for a root-relative path`, () => {
const path = new Path('/service-worker.js');
expect(path.getFileName()).toBe('service-worker.js');
expect(path.getFullPath()).toBe('/service-worker.js');
expect(path._getFileName()).toBe('service-worker.js');
expect(path._getFullPath()).toBe('/service-worker.js');
});

test(`should return correct components for an absolute web path`, () => {
const path = new Path('https://site.com/web-folder/service-worker.js');
expect(path.getFileName()).toBe('service-worker.js');
expect(path.getFullPath()).toBe(
expect(path._getFileName()).toBe('service-worker.js');
expect(path._getFullPath()).toBe(
'https://site.com/web-folder/service-worker.js',
);
});
Expand All @@ -45,21 +47,15 @@ describe('Path tests', () => {
const path = new Path(
'https://site.com/web-folder/service-worker.js?appId=12345',
);
expect(path.getFullPath()).toBe(
'https://site.com/web-folder/service-worker.js?appId=12345',
);
});
test(`should include query string in path with query`, () => {
const path = new Path(
expect(path._getFullPath()).toBe(
'https://site.com/web-folder/service-worker.js?appId=12345',
);
expect(path.getFileNameWithQuery()).toBe('service-worker.js?appId=12345');
});

test(`should not include query string in path filename`, () => {
const path = new Path(
'https://site.com/web-folder/service-worker.js?appId=12345',
);
expect(path.getFileName()).toBe('service-worker.js');
expect(path._getFileName()).toBe('service-worker.js');
});
});
6 changes: 3 additions & 3 deletions __test__/unit/pushSubscription/nativePermissionChange.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ import { MockServiceWorker } from '__test__/support/mocks/MockServiceWorker';
import { clearStore, db, getOptionsValue } from 'src/shared/database/client';
import { setAppState as setDBAppState } from 'src/shared/database/config';
import type { AppState } from 'src/shared/database/types';
import { checkAndTriggerNotificationPermissionChanged } from 'src/shared/helpers/main';
import * as PermissionUtils from 'src/shared/helpers/permissions';
import Emitter from 'src/shared/libraries/Emitter';
import { checkAndTriggerSubscriptionChanged } from 'src/shared/listeners';
import MainHelper from '../../../src/shared/helpers/MainHelper';

vi.mock('src/shared/libraries/Log');
const triggerNotificationSpy = vi.spyOn(
Expand Down Expand Up @@ -50,7 +50,7 @@ describe('Notification Types are set correctly on subscription change', () => {
});
await setDbPermission('granted');

await MainHelper._checkAndTriggerNotificationPermissionChanged();
await checkAndTriggerNotificationPermissionChanged();
expect(triggerNotificationSpy).not.toHaveBeenCalled();
});

Expand All @@ -74,7 +74,7 @@ describe('Notification Types are set correctly on subscription change', () => {
permChangeStringListener,
);

await MainHelper._checkAndTriggerNotificationPermissionChanged();
await checkAndTriggerNotificationPermissionChanged();

// should update the db
const dbPermission = await getOptionsValue<NotificationPermission>(
Expand Down
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,12 @@
},
{
"path": "./build/releases/OneSignalSDK.page.es6.js",
"limit": "47.565 kB",
"limit": "46.601 kB",
"gzip": true
},
{
"path": "./build/releases/OneSignalSDK.sw.js",
"limit": "13.624 kB",
"limit": "13.442 kB",
"gzip": true
},
{
Expand Down
4 changes: 2 additions & 2 deletions src/core/CoreModuleDirector.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
} from 'src/shared/subscriptions/constants';
import type { SubscriptionChannelValue } from 'src/shared/subscriptions/types';
import { logMethodCall } from 'src/shared/utils/utils';
import MainHelper from '../shared/helpers/MainHelper';
import { getCurrentPushToken } from '../shared/helpers/main';
import { RawPushSubscription } from '../shared/models/RawPushSubscription';
import CoreModule from './CoreModule';
import { IdentityModel } from './models/IdentityModel';
Expand Down Expand Up @@ -115,7 +115,7 @@ export class CoreModuleDirector {
SubscriptionModel | undefined
> {
logMethodCall('CoreModuleDirector.getPushSubscriptionModelByCurrentToken');
const pushToken = await MainHelper._getCurrentPushToken();
const pushToken = await getCurrentPushToken();
if (pushToken) {
return this._getSubscriptionOfTypeWithToken(
SubscriptionChannel.Push,
Expand Down
4 changes: 2 additions & 2 deletions src/core/controllers/CustomEventController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import MainHelper from '../../shared/helpers/MainHelper';
import { getAppId } from '../../shared/helpers/main';
import { IdentityModelStore } from '../modelStores/IdentityModelStore';
import { TrackCustomEventOperation } from '../operations/TrackCustomEventOperation';
import type {
Expand All @@ -22,7 +22,7 @@ export class CustomEventController implements ICustomEventController {
}

_sendCustomEvent(event: ICustomEvent): void {
const appId = MainHelper._getAppId();
const appId = getAppId();
const identityModel = this._identityModelStore.model;

const op = new TrackCustomEventOperation({
Expand Down
4 changes: 2 additions & 2 deletions src/core/listeners/IdentityModelStoreListener.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import MainHelper from 'src/shared/helpers/MainHelper';
import { getAppId } from 'src/shared/helpers/main';
import { type IdentityModel } from '../models/IdentityModel';
import { type IdentityModelStore } from '../modelStores/IdentityModelStore';
import { DeleteAliasOperation } from '../operations/DeleteAliasOperation';
Expand Down Expand Up @@ -26,7 +26,7 @@ export class IdentityModelStoreListener extends SingletonModelStoreListener<Iden
_oldValue: unknown,
newValue: unknown,
): Operation {
const appId = MainHelper._getAppId();
const appId = getAppId();
if (newValue != null && typeof newValue === 'string') {
return new SetAliasOperation(
appId,
Expand Down
4 changes: 2 additions & 2 deletions src/core/listeners/PropertiesModelStoreListener.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import MainHelper from 'src/shared/helpers/MainHelper';
import { getAppId } from 'src/shared/helpers/main';
import { PropertiesModel } from '../models/PropertiesModel';
import { type PropertiesModelStore } from '../modelStores/PropertiesModelStore';
import { type Operation } from '../operations/Operation';
Expand Down Expand Up @@ -28,7 +28,7 @@ export class PropertiesModelStoreListener extends SingletonModelStoreListener<Pr
_oldValue: unknown,
newValue: unknown,
): Operation | null {
const appId = MainHelper._getAppId();
const appId = getAppId();

return new SetPropertyOperation(
appId,
Expand Down
8 changes: 4 additions & 4 deletions src/core/listeners/SubscriptionModelStoreListener.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import MainHelper from 'src/shared/helpers/MainHelper';
import { getAppId } from 'src/shared/helpers/main';
import { NotificationType } from 'src/shared/subscriptions/constants';
import type { NotificationTypeValue } from 'src/shared/subscriptions/types';
import { SubscriptionModel } from '../models/SubscriptionModel';
Expand Down Expand Up @@ -29,7 +29,7 @@ export class SubscriptionModelStoreListener extends ModelStoreListener<Subscript
const { enabled, notification_types } =
SubscriptionModelStoreListener._getSubscriptionEnabledAndStatus(model);

const appId = MainHelper._getAppId();
const appId = getAppId();
return new CreateSubscriptionOperation({
appId,
onesignalId: this._identityModelStore.model._onesignalId,
Expand All @@ -42,7 +42,7 @@ export class SubscriptionModelStoreListener extends ModelStoreListener<Subscript
}

_getRemoveOperation(model: SubscriptionModel): Operation {
const appId = MainHelper._getAppId();
const appId = getAppId();
return new DeleteSubscriptionOperation(
appId,
this._identityModelStore.model._onesignalId,
Expand All @@ -53,7 +53,7 @@ export class SubscriptionModelStoreListener extends ModelStoreListener<Subscript
_getUpdateOperation(model: SubscriptionModel): Operation {
const { enabled, notification_types } =
SubscriptionModelStoreListener._getSubscriptionEnabledAndStatus(model);
const appId = MainHelper._getAppId();
const appId = getAppId();

return new UpdateSubscriptionOperation({
appId,
Expand Down
8 changes: 4 additions & 4 deletions src/core/modelRepo/ModelStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export abstract class ModelStore<

_onChanged(args: ModelChangedArgs, tag: string): void {
this._persist();
this._changeSubscription.fire((handler) =>
this._changeSubscription._fire((handler) =>
handler._onModelUpdated(args, tag),
);
}
Expand All @@ -116,7 +116,7 @@ export abstract class ModelStore<
for (const item of this._models) {
// no longer listen for changes to this model
item._unsubscribe(this);
this._changeSubscription.fire((handler) =>
this._changeSubscription._fire((handler) =>
handler._onModelRemoved(item, tag),
);
db.delete(this._modelName, item._modelId);
Expand All @@ -136,7 +136,7 @@ export abstract class ModelStore<
model._subscribe(this);
this._persist();

this._changeSubscription.fire((handler) =>
this._changeSubscription._fire((handler) =>
handler._onModelAdded(model, tag),
);
}
Expand All @@ -151,7 +151,7 @@ export abstract class ModelStore<
await db.delete(this._modelName, model._modelId);
this._persist();

this._changeSubscription.fire((handler) =>
this._changeSubscription._fire((handler) =>
handler._onModelRemoved(model, tag),
);
}
Expand Down
4 changes: 2 additions & 2 deletions src/core/modelStores/SingletonModelStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export class SingletonModelStore<TModel extends Model>
const existingModel = this.model;
existingModel._initializeFromModel(existingModel._modelId, model);
this.store._persist();
this.changeSubscription.fire((handler) =>
this.changeSubscription._fire((handler) =>
handler._onModelReplaced(existingModel, tag),
);
}
Expand All @@ -65,7 +65,7 @@ export class SingletonModelStore<TModel extends Model>
}

_onModelUpdated(args: ModelChangedArgs, tag: ModelChangeTagValue): void {
this.changeSubscription.fire((handler) =>
this.changeSubscription._fire((handler) =>
handler._onModelUpdated(args, tag),
);
}
Expand Down
4 changes: 3 additions & 1 deletion src/core/models/Model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,9 @@ export class Model<U extends object = object, T extends U & object = U & object>
oldValue,
newValue,
};
this._changeNotifier.fire((handler) => handler._onChanged(changeArgs, tag));
this._changeNotifier._fire((handler) =>
handler._onChanged(changeArgs, tag),
);
}

/**
Expand Down
7 changes: 3 additions & 4 deletions src/entries/worker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
/**
* New clients will only be including this entry file, which will result in a reduced service worker size.
*/
import { OneSignalServiceWorker } from '../sw/serviceWorker/ServiceWorker';
import { run } from '../sw/serviceWorker/ServiceWorker';

// Expose this class to the global scope
declare const self: ServiceWorkerGlobalScope;
self.OneSignalWorker = OneSignalServiceWorker;
// The run() is already called in ServiceWorker.ts, but importing it ensures it's not tree-shaken
void run;
Copy link
Contributor

@sherwinski sherwinski Oct 23, 2025

Choose a reason for hiding this comment

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

I would need confirmation that this actually works: e.g. does it actually make it to the final bundle? Even then, this approach seems confusing and potentially fragile.

I'm suspicious that it's not needed especially because run is already invoked at the top-level here.

My suggestion would be to either move that line here

import { run } from '../sw/serviceWorker/ServiceWorker';

run();

or import the entire file (although the bundle size may take a hit)

import '../sw/serviceWorker/ServiceWorker';

// The run() is already called in ServiceWorker.ts, but importing it ensures it's not tree-shaken
void run;

Copy link
Collaborator Author

@fadi-george fadi-george Oct 23, 2025

Choose a reason for hiding this comment

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

Both work but for now ive moved run called to worker.ts

Copy link
Contributor

@sherwinski sherwinski Oct 23, 2025

Choose a reason for hiding this comment

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

Should this call be removed then?

Copy link
Collaborator Author

@fadi-george fadi-george Oct 24, 2025

Choose a reason for hiding this comment

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

It is removed

1 change: 0 additions & 1 deletion src/global.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ declare global {

interface WorkerGlobalScope {
OneSignal: _OneSignal;
OneSignalWorker: typeof import('./sw/serviceWorker/ServiceWorker').OneSignalServiceWorker;
_workerMessenger: import('./sw/serviceWorker/WorkerMessengerSW').WorkerMessengerSW;
shouldLog: boolean;
}
Expand Down
4 changes: 2 additions & 2 deletions src/onesignal/OneSignal.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
import { setPushToken } from 'src/shared/database/subscription';
import type { SubscriptionSchema } from 'src/shared/database/types';
import { registerForPushNotifications } from 'src/shared/helpers/init';
import MainHelper from 'src/shared/helpers/MainHelper';
import * as MainHelper from 'src/shared/helpers/main';
import Log from 'src/shared/libraries/Log';
import { IDManager } from 'src/shared/managers/IDManager';
import { SubscriptionManagerPage } from 'src/shared/managers/subscription/page';
Expand Down Expand Up @@ -1202,5 +1202,5 @@
'_subscribeFcmFromPage',
);

const showLocalNotificationSpy = vi.spyOn(MainHelper, '_showLocalNotification');
const showLocalNotificationSpy = vi.spyOn(MainHelper, 'showLocalNotification');
showLocalNotificationSpy.mockImplementation(async () => {});

Check failure on line 1206 in src/onesignal/OneSignal.test.ts

View workflow job for this annotation

GitHub Actions / test 

Unexpected empty async arrow function
6 changes: 3 additions & 3 deletions src/onesignal/OneSignal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
removeLegacySubscriptionOptions,
setConsentRequired as setStorageConsentRequired,
} from 'src/shared/helpers/localStorage';
import { checkAndTriggerNotificationPermissionChanged } from 'src/shared/helpers/main';
import {
_onSubscriptionChanged,
checkAndTriggerSubscriptionChanged,
Expand All @@ -38,7 +39,6 @@
import LoginManager from '../page/managers/LoginManager';
import Context from '../page/models/Context';
import type { OneSignalDeferredLoadedCallback } from '../page/models/OneSignalDeferredLoadedCallback';
import MainHelper from '../shared/helpers/MainHelper';
import Emitter from '../shared/libraries/Emitter';
import Log from '../shared/libraries/Log';
import DebugNamespace from './DebugNamesapce';
Expand Down Expand Up @@ -168,7 +168,7 @@
private static async _delayedInit(): Promise<void> {
OneSignal._pendingInit = false;
// Ignore Promise as doesn't return until the service worker becomes active.
OneSignal._context._workerMessenger.listen();
OneSignal._context._workerMessenger._listen();

async function __init() {
if (OneSignal._initAlreadyCalled) return;
Expand All @@ -188,7 +188,7 @@
window.addEventListener('focus', () => {
// Checks if permission changed every time a user focuses on the page,
// since a user has to click out of and back on the page to check permissions
MainHelper._checkAndTriggerNotificationPermissionChanged();
checkAndTriggerNotificationPermissionChanged();
});

await initSaveState();
Expand Down Expand Up @@ -277,7 +277,7 @@
private static _pendingInit = true;

static _emitter: Emitter = new Emitter();
static _cache: any = {};

Check warning on line 280 in src/onesignal/OneSignal.ts

View workflow job for this annotation

GitHub Actions / test 

Unexpected any. Specify a different type
static _initCalled = false;
static _initAlreadyCalled = false;
static _context: Context;
Expand Down
Loading
Loading