Skip to content

feat: add support for overriding OneSignal script src #202

Merged
sherwinski merged 1 commit intomainfrom
cd_update
Feb 25, 2026
Merged

feat: add support for overriding OneSignal script src #202
sherwinski merged 1 commit intomainfrom
cd_update

Conversation

@onesignal-deploy
Copy link
Collaborator

@onesignal-deploy onesignal-deploy commented Feb 23, 2026

Features

  • add support for overriding OneSignal script src

@sherwinski sherwinski changed the title build: sync with web-shim-codegen v3.0.8 feat: add support for overriding OneSignal script src Feb 25, 2026
@sherwinski
Copy link
Contributor

@claude review this PR

@claude
Copy link

claude bot commented Feb 25, 2026

Claude finished @sherwinski's task in 1m 13s —— View job


PR Review

  • Gather context and understand request
  • Read CLAUDE.md for repo guidelines (none found)
  • Analyze PR changes (index.ts)
  • Post review feedback

Summary

This PR adds a scriptSrc option to IInitObject that lets users override the OneSignal SDK script URL — useful for self-hosting to satisfy strict CSP or COEP policies. The changes are clean and well-scoped. A few observations below.


Observations

Behavior change: script loading is deferred to init()

Previously, addSDKScript() was called at module load time inside the if (typeof window !== 'undefined') block. It's now removed from there and only called inside init(). This is actually a nice improvement — the SDK script won't be fetched until the user explicitly calls init() — but it is a subtle breaking change for any code that relied on the script being loaded eagerly on import. Worth calling out in release notes.

Duplicate script guard (index.ts:29–31)

if (document.getElementById(ONESIGNAL_SDK_ID)) {
  return;
}

Good defensive addition. This correctly prevents double-injection if addSDKScript is somehow called more than once.

scriptSrc is forwarded to the native SDK (index.ts:118)

options (including scriptSrc) is passed directly to window.OneSignal.init(options). The native SDK won't recognize scriptSrc, but since IInitObject has [key: string]: unknown, TypeScript won't complain. In practice this is likely harmless, but it's worth verifying that the native SDK doesn't reject unknown options.

Removed explanatory comments (index.ts:5–6)

The PR removes these comments:

// true if the script is successfully loaded from CDN.
// true if the script fails to load from CDN. A separate flag is necessary
// to disambiguate between a CDN load failure and a delayed call to
// OneSignal#init.

The second comment in particular added useful context distinguishing isOneSignalScriptFailed from a delayed init() call. Minor, but worth keeping.

JSDoc on scriptSrc is well-written

/**
 * Overrides the default OneSignal SDK script URL.
 * Use this to self-host the SDK script on your own domain, e.g. to comply
 * with strict Cross-Origin-Embedder-Policy (COEP) or Content-Security-Policy (CSP) headers.
 * @default 'https://cdn.onesignal.com/sdks/web/v16/OneSignalSDK.page.js'
 */
scriptSrc?: string;

Good — the use-case motivation and @default annotation are both helpful for consumers.


Verdict

Looks good overall. The feature is useful, implementation is clean, and the duplicate-guard is a nice bonus. The main things to keep in mind before merging:

  1. Document the deferred-loading behavior change (script no longer loads on module import).
  2. Verify the native SDK handles the unknown scriptSrc option gracefully (likely a no-op, but worth confirming).
  3. Consider restoring the comment on isOneSignalScriptFailed to preserve that context.

@sherwinski sherwinski merged commit 54231ac into main Feb 25, 2026
7 checks passed
@onesignal-deploy
Copy link
Collaborator Author

🎉 This PR is included in version 3.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants