Closed jkasten2 closed 6 months ago
Other than what we discussed, LGTM.
One nit is the tests are logging an error (tests still pass) -- this may or may not be something that predates these changes
Error: OneSignal service worker not found!
at SubscriptionManager.subscribeFcmFromPage (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/shared/managers/SubscriptionManager.ts:513:13)
at SubscriptionManager.subscribe (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/shared/managers/SubscriptionManager.ts:162:13)
at Function.internalRegisterForPush (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/shared/helpers/SubscriptionHelper.ts:29:35)
at Function.registerForPush (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/shared/helpers/SubscriptionHelper.ts:19:12)
at Function.registerForPushNotifications (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/shared/helpers/InitHelper.ts:104:5)
at PromptsManager.internalShowNativePrompt (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/page/managers/PromptsManager.ts:189:5)
at NotificationsNamespace.requestPermission (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/onesignal/NotificationsNamespace.ts:142:5)
at PushSubscriptionNamespace.optIn (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/onesignal/PushSubscriptionNamespace.ts:96:7)
at Object.<anonymous> (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/__test__/unit/push/registerForPush.test.ts:31:5)
38 | await EventHelper.checkAndTriggerSubscriptionChanged();
39 | } catch (e) {
> 40 | Log.error(e);
| ^
41 | }
42 | break;
43 | default:
at Function.error [as internalRegisterForPush] (src/shared/helpers/SubscriptionHelper.ts:40:15)
at Function.registerForPush (src/shared/helpers/SubscriptionHelper.ts:19:12)
at Function.registerForPushNotifications (src/shared/helpers/InitHelper.ts:104:5)
at PromptsManager.internalShowNativePrompt (src/page/managers/PromptsManager.ts:[189](https://github.com/OneSignal/OneSignal-Website-SDK/actions/runs/7024669946/job/19113899601?pr=1142#step:6:190):5)
at NotificationsNamespace.requestPermission (src/onesignal/NotificationsNamespace.ts:142:5)
at PushSubscriptionNamespace.optIn (src/onesignal/PushSubscriptionNamespace.ts:96:7)
at Object.<anonymous> (__test__/unit/push/registerForPush.test.ts:31:5)
console.error
Error: OneSignal service worker not found!
at SubscriptionManager.subscribeFcmFromPage (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/shared/managers/SubscriptionManager.ts:513:13)
at SubscriptionManager.subscribe (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/shared/managers/SubscriptionManager.ts:162:13)
at Function.internalRegisterForPush (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/shared/helpers/SubscriptionHelper.ts:29:35)
at Function.registerForPush (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/shared/helpers/SubscriptionHelper.ts:19:12)
at Function.registerForPushNotifications (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/src/shared/helpers/InitHelper.ts:104:5)
at Object.<anonymous> (/home/runner/work/OneSignal-Website-SDK/OneSignal-Website-SDK/__test__/unit/push/registerForPush.test.ts:41:5)
38 | await EventHelper.checkAndTriggerSubscriptionChanged();
39 | } catch (e) {
> 40 | Log.error(e);
| ^
41 | }
42 | break;
43 | default:
at Function.error [as internalRegisterForPush] (src/shared/helpers/SubscriptionHelper.ts:40:15)
at Function.registerForPush (src/shared/helpers/SubscriptionHelper.ts:19:12)
at Function.registerForPushNotifications (src/shared/helpers/InitHelper.ts:104:5)
at Object.<anonymous> (__test__/unit/push/registerForPush.test.ts:41:5)
Description
1 Line Summary
Remove code for the OneSignal Subdomain label (os.tc) as supported was dropped in v16.
Details
Motivation
Motivation to removing this deadcode is to make upcoming changes easier to make. This PR also results in a noticeable size reduction in the SDK.
Scope
This PR addresses deleting anything related to the dropped subdomain / os.tc feature. Very minor refactoring was done to clean up some code paths related to the feature removal.
Validation
Test on Windows 11 23H2 with Chrome 119.
Tested on macOS 13.6.1 Safari 17.1
Tested on iOS 16.7.2
Tests
Info
Checklist
Programming Checklist Interfaces:
Functions:
Typescript:
Other:
elem of array
syntax. PreferforEach
or usemap
context
if possible. Instead, we can pass it to function/constructor so that we don't callOneSignal.context
Screenshots
Info
Checklist
Related Tickets
This change is![Reviewable](https://reviewable.io/review_button.svg)