[sherwinski]

Description

1 Line Summary

Adds a feature flag reader for the page (isFeatureEnabled) and the service worker (isFeatureEnabledInWorker), fed by GET apps/{appId}/sdk/features/web/{version} once per page load. Split out of #1531 at review request so that PR can stack on it.

Details

Page reader (src/shared/features/featureFlags.ts)

• refreshFeatureFlags(appId) runs from OneSignal.init after _initializeCoreModuleAndOSNamespaces(), with OneSignal.config.appId. Init does not wait for it. Its IndexedDB write comes after the init reads, so a stalled write cannot trip the circuit breaker while the reads run. It calls GET apps/{appId}/sdk/features/web/{VERSION} and stores features in localStorage under os_feature_flags. A failed fetch or a malformed body keeps the cached list and never blocks init. The cache is not keyed by app ID: after an app ID change, a failed fetch keeps the old app's list until the next successful fetch.
• isFeatureEnabled(flag) reads storage on every call, so a fresh list applies at once without a cold start.
• The comma-separated localStorage key os_feature_overrides forces flags on for local testing. When set, a console.warn shows the active overrides (plain console.warn, because Log is silent below trace in production builds).
• Flags defined: sdk_identity_verification. feat: [SDK-5120] inline notification icons from the service worker so CDN issues preserve the brand icon #1531 adds sdk_web_inline_notification_images.

Worker mirror

The worker has no localStorage, so refreshFeatureFlags also writes the active list (server list plus overrides) to the IndexedDB Options store under featureFlags, and isFeatureEnabledInWorker(flag) reads it there. The mirror is written on every refresh from the overrides plus the fresh or cached list, so an override reaches the worker when the fetch fails. The IndexedDB write happens before the localStorage write, and db.put resolves to undefined when the circuit breaker drops it. In that case the page list is left as is, so the page and the worker see the same list.

Source

The first two commits are cherry-picks of 44fbb438 and ad7056bc on feat/identity-verification. The patches are identical, so that branch drops them on rebase or merge. The refactor: commit is Fadi's, from fadi/pr-1534-size-example: it moves the refresh after hydration, keeps the page list as a plain array, shares one list parser between the page and the worker, and removes the Log._warn calls.

Bundle size

Page 43.09 kB → 43.29 kB (+200 B). Worker unchanged (12.48 kB). The build: commits set the page limit to 43.3 kB.

Systems Affected

• WebSDK
• Backend
• Dashboard

Validation

Tests

Info

• vp test run — 63 files, 578 tests pass.
• vp check — no new errors or warnings. main currently has 3 formatting issues and 2 type errors in Bell.test.ts from the Vite+ 1.3.0 upgrade (chore: upgrade to Vite+ 1.3.0 and TypeScript 7 #1532); they are unchanged here.
• vp run build:prod — passes with the new limit.
• New tests: src/shared/features/featureFlags.test.ts (20): reader, overrides, override warning, worker mirror, dropped-write case (page and worker both kept), override mirror on a failed fetch, storage error.

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 — the reader and the worker mirror ran on a Pixel 9 emulator as part of the feat: [SDK-5120] inline notification icons from the service worker so CDN issues preserve the brand icon #1531 manual test (Options.featureFlags held the server list plus the override, and the worker read it).
• 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

None; no UI change.

Checklist

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

---

Related Tickets

• SDK-5597
• SDK-5120 (stacked PR feat: [SDK-5120] inline notification icons from the service worker so CDN issues preserve the brand icon #1531)

---

[cursor]

Multi-model review

Claude Opus 5.5, GPT 5.6 Sol, and Grok 4.7 reviewed the feature-flag reader and the worker mirror. The design (fetch once per load, never block init, IDB write first so a wedged put leaves both sides on the last shared list) is clear. The agreement is that a few paths still break that page/worker invariant.

Act on (3/3)

• isReadwriteWedged() after db.put is the page-wide latch, not this write’s result. A successful Options put plus another store wedging, or a late IDB commit after the 1500ms timeout, updates the worker and leaves the page on the old list. The dropped-write test reproduces that split (real put, mocked latch) and never asserts the worker value. Use db.put’s return (undefined from guard vs the key) and assert both stores.
• Neither os_feature_flags nor Options.featureFlags is keyed by appId/VERSION, and initSaveState does not clear them on app change. A failed refresh after a switch keeps the previous app’s flags, including sdk_identity_verification.

Consider (3/3)

• Overrides are live on the page (getFeatureOverrides() every call) but snapshotted into IndexedDB only after a successful fetch. Add or remove os_feature_overrides while the request fails and the two sides disagree.
• void refreshFeatureFlags(...) is eventual-consistency by design and has no callers here. #1531’s one-time init decisions will still run on an empty or stale list. Expose a ready promise or re-run those decisions after refresh.
• db.put before setFeatureFlags in one try: an IDB throw skips the page write; a setItem throw after a committed put leaves the worker ahead. getFeatureOverrides() is also outside the try.

Noted: five TypeError retries on a best-effort call; extra Options readwrite racing init; fire-and-forget refresh can leak into later tests via the new default MSW handler.

Dismissed: overrides can only force flags on (intentional for local testing).

  

_{Sent by Cursor Automation: PR Reviews}

[fadi-george]

Put together an example of the hydration fix plus some size trimming on top of this branch: sherwin/sdk-5597...fadi/pr-1534-size-example

Page bundle goes from 43,492 B to 43,292 B (+203 B over main instead of +403 B), so the limit could drop to about 43.3 kB. Tests, vp check, and build:prod pass.

What changed:

• refreshFeatureFlags runs right after _initializeCoreModuleAndOSNamespaces() using OneSignal.config.appId, so the mirror write can't trip the breaker before hydration. The isValidUuid check goes away since the app ID is already validated there.
• The localStorage helpers collapse into getFeatureFlags, setFeatureFlags, and one parseList that the page and worker readers share. Readers no longer filter non-strings since only the SDK writes these lists. The server response is still filtered.
• The fetch folds into the refresh with get(...).catch(() => undefined), so a failed fetch still mirrors the cached list plus overrides.
• The page cache is written only after db.put commits, same as now.
• Dropped the Log._warn calls since they're silent in prod but still ship their strings.
• Dropped the per-app cache tracking and the value: null put. Stale flags would need an app ID change and a failed fetch on the same load, and they clear on the next successful fetch. Happy to put it back if you want that case covered.

Feel free to cherry-pick whatever's useful.

[abdulraqeeb33]

The per-app cache is right when the fetch succeeds, and a dropped mirror write leaves that page list alone.

Requesting changes on the upgrade path. A failed fetch now mirrors getFeatureFlags(), and a pre-upgrade array reads as [], so that write replaces a good worker list with [].

The app-change clear removes the page list before the worker put is known to have committed. A dropped or thrown clear leaves the worker on the other app's flags for this page. Fine as a follow-up.

Decided to roll back the per-app cache so this review is now stale