Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion .github/actions/setup-demo/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,4 @@ runs:
run: |
echo "ONESIGNAL_APP_ID=${{ inputs.onesignal-app-id }}" > .env
echo "ONESIGNAL_API_KEY=${{ inputs.onesignal-api-key }}" >> .env
echo "E2E_MODE=true" >> .env
echo "ONESIGNAL_ANDROID_CHANNEL_ID=7ec2ece9-c538-4656-9516-1316f48a005c" >> .env
2 changes: 1 addition & 1 deletion android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ dependencies {
// Exclude OkHttp from OneSignal's transitive deps: the otel module pulls in OkHttp 5.x
// (via opentelemetry-exporter-sender-okhttp) which is binary-incompatible with React Native's
// networking stack (okhttp3.internal.Util removed in 5.x). React Native already provides OkHttp 4.x.
api('com.onesignal:OneSignal:5.8.1') {
api('com.onesignal:OneSignal:5.9.2') {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟑 The PR description heading reads "Update Android SDK from 5.8.1 to 5.9.1", but android/build.gradle line 42 actually bumps the dependency to com.onesignal:OneSignal:5.9.2. Since this PR's description becomes the public CHANGELOG / GitHub release notes for 5.4.6, the version mismatch is user-visible β€” please update the heading to 5.9.2 and add any release-notes bullets for changes that landed exclusively in 5.9.2.

Extended reasoning...

What's wrong

The PR description in this 'chore: Release 5.4.6' PR contains a stale heading:

πŸ› οΈ Native Dependency Updates

  • Update Android SDK from 5.8.1 to 5.9.1

But android/build.gradle line 42 actually bumps the dependency to 5.9.2:

api('com.onesignal:OneSignal:5.9.2') {
    exclude group: 'com.squareup.okhttp3', module: 'okhttp'
}

How it happened (step-by-step proof)

Looking at the recent git log on this branch:

  1. 27e71ef β€” Update Android SDK to 5.9.0
  2. bf03cce β€” Update Android SDK to 5.9.1 ← PR description was written to match this commit
  3. 078d6db β€” Update Android SDK to 5.9.2 ← description was never updated

The PR description (and its bullet list of upstream Android-SDK PRs #2637–#2645) was authored when the dependency was still at 5.9.1 and was simply not refreshed when the follow-up bump to 5.9.2 landed. The bullet list covers fixes that went into 5.9.0/5.9.1, but anything that shipped exclusively in 5.9.2 (e.g. any patches landed on top of bf03cce in the Android SDK) is undocumented.

Why this matters

This is the release PR for 5.4.6, and its description is what becomes the public CHANGELOG entry / GitHub release notes once the tag is cut. Consumers reading the release notes will see an announcement that the underlying Android SDK was upgraded to 5.9.1, while the artifact they actually install pulls 5.9.2. This is a small but user-visible inconsistency: dependency-pinning tooling, security scanners, and curious users who diff the lockfile against the release notes will all see the mismatch.

How to fix

Either:

  1. Update the heading to read 'Update Android SDK from 5.8.1 to 5.9.2' and append the release-notes bullets for any PRs that landed in 5.9.2 on top of the existing 5.9.1 list, or
  2. If 5.9.2 was a regression and 5.9.1 is what's intended to ship, roll android/build.gradle back to 5.9.1 (the description would then match).

The first option is almost certainly what's wanted β€” the latest commit (078d6db) explicitly bumped the dependency to 5.9.2, so the code is correct and the description is stale. Nothing about runtime behavior is affected, so this is documentation-only (nit).

exclude group: 'com.squareup.okhttp3', module: 'okhttp'
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ public void invalidate() {
@Override
public void initialize(String appId) {
OneSignalWrapper.setSdkType("reactnative");
OneSignalWrapper.setSdkVersion("050405");
OneSignalWrapper.setSdkVersion("050406");

if (oneSignalInitDone) {
Logging.debug("Already initialized the OneSignal React-Native SDK", null);
Expand Down
1 change: 0 additions & 1 deletion examples/demo/.env.example
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Default App ID (used if ONESIGNAL_APP_ID is empty): 77e32082-ea27-42e3-a898-c72e141824ef
ONESIGNAL_APP_ID=your-onesignal-app-id
ONESIGNAL_API_KEY=your-onesignal-api-key
E2E_MODE=false

# Optional: Android Notification Channel ID for the WITH SOUND test notification.
# Create one in your OneSignal dashboard under Settings > Android Notification Categories.
Expand Down
3 changes: 1 addition & 2 deletions examples/demo/src/components/sections/AppSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React from 'react';
import { View, Text, TouchableOpacity, Linking, StyleSheet } from 'react-native';

import { AppColors, AppTextStyles, AppTheme, AppSpacing } from '../../theme';
import { maskValue } from '../../utils/maskValue';
import SectionCard from '../SectionCard';
import ToggleRow from '../ToggleRow';

Expand Down Expand Up @@ -33,7 +32,7 @@ export default function AppSection({
ellipsizeMode="middle"
testID="app_id_value"
>
{maskValue(appId)}
{appId}
</Text>
</View>
</View>
Expand Down
3 changes: 1 addition & 2 deletions examples/demo/src/components/sections/PushSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React from 'react';
import { View, Text, StyleSheet } from 'react-native';

import { AppColors, AppTextStyles, AppTheme, AppSpacing } from '../../theme';
import { maskValue } from '../../utils/maskValue';
import ActionButton from '../ActionButton';
import SectionCard from '../SectionCard';
import ToggleRow from '../ToggleRow';
Expand Down Expand Up @@ -35,7 +34,7 @@ export default function PushSection({
ellipsizeMode="middle"
testID="push_id_value"
>
{maskValue(pushSubscriptionId ?? 'β€”')}
{pushSubscriptionId ?? 'β€”'}
</Text>
</View>
<View style={styles.divider} />
Expand Down
6 changes: 6 additions & 0 deletions examples/demo/src/hooks/useOneSignal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
consentRequired: boolean;
privacyConsentGiven: boolean;
externalUserId: string | undefined;
oneSignalId: string | undefined;
pushSubscriptionId: string | undefined;
isPushEnabled: boolean;
hasNotificationPermission: boolean;
Expand Down Expand Up @@ -115,6 +116,7 @@
const [consentRequired, setConsentRequiredState] = useState(false);
const [privacyConsentGiven, setPrivacyConsentGivenState] = useState(false);
const [externalUserId, setExternalUserId] = useState<string | undefined>(undefined);
const [oneSignalId, setOneSignalId] = useState<string | undefined>(undefined);
const [pushSubscriptionId, setPushSubscriptionId] = useState<string | undefined>(undefined);
const [isPushEnabled, setIsPushEnabled] = useState(false);
const [hasNotificationPermission, setHasNotificationPermission] = useState(false);
Expand Down Expand Up @@ -207,6 +209,8 @@
`User changed: onesignalId=${nextOnesignalId ?? 'null'}, externalId=${event.current.externalId ?? 'null'}`,
);

setOneSignalId(nextOnesignalId ?? undefined);

if (nextOnesignalId === null) {
return;
}
Expand Down Expand Up @@ -280,6 +284,7 @@
setIsReady(true);

const initialOnesignalId = await OneSignal.User.getOnesignalId();
setOneSignalId(initialOnesignalId ?? undefined);
if (initialOnesignalId) {
await fetchUserDataFromApi();
}
Expand Down Expand Up @@ -532,12 +537,13 @@

return {
appId,
consentRequired,
privacyConsentGiven,
externalUserId,
oneSignalId,
pushSubscriptionId,
isPushEnabled,
hasNotificationPermission,

Check warning on line 546 in examples/demo/src/hooks/useOneSignal.ts

View check run for this annotation

Claude / Claude Code Review

Unused oneSignalId state in useOneSignal hook

The PR adds an `oneSignalId` state field to `useOneSignalState` (type at line 65, `useState` at line 119, setters at ~line 212 and ~line 287, return entry at line 543), but no component in `examples/demo` consumes `os.oneSignalId` β€” commits edc026d (display in PushSection) and 74dd7c1 (revert) added then removed the UI, leaving dead state behind. The cost is one unused `OneSignal.User.getOnesignalId()` await on init plus an unused `setState` on every user-state change that re-renders every consu
Comment on lines 540 to 546
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟑 The PR adds an oneSignalId state field to useOneSignalState (type at line 65, useState at line 119, setters at ~line 212 and ~line 287, return entry at line 543), but no component in examples/demo consumes os.oneSignalId β€” commits edc026d (display in PushSection) and 74dd7c1 (revert) added then removed the UI, leaving dead state behind. The cost is one unused OneSignal.User.getOnesignalId() await on init plus an unused setState on every user-state change that re-renders every consumer of OneSignalContext for nothing. Either remove the state/setter and the type/return fields, or wire oneSignalId back into the UI as originally intended.

Extended reasoning...

What the bug is

This PR adds new state plumbing for an oneSignalId value in examples/demo/src/hooks/useOneSignal.ts that no consumer ever reads. Specifically the diff introduces:

  • The field on the public return type (line 65): oneSignalId: string | undefined;
  • The state hook (line 119): const [oneSignalId, setOneSignalId] = useState<string | undefined>(undefined);
  • A setter inside the user 'change' handler (~line 212): setOneSignalId(nextOnesignalId ?? undefined);
  • A setter inside the init/load function (~line 287): setOneSignalId(initialOnesignalId ?? undefined);
  • The field on the returned object (line 543): oneSignalId,

Why this is dead code

Grepping examples/demo/src for oneSignalId shows the identifier appears only inside useOneSignal.ts itself. None of the section components (AppSection.tsx, PushSection.tsx, UserSection.tsx, etc.) nor HomeScreen.tsx reads os.oneSignalId. The git log explains why: commit edc026d chore(demo): display OneSignal ID in push section added a UI that displayed the value, and the immediately following commit 74dd7c1 chore(demo): remove OneSignal ID from push section reverted that UI β€” but the supporting hook state on the producer side was never cleaned up.

Step-by-step proof

  1. useOneSignalState is wrapped by OneSignalProvider (line ~600) which puts the return object on a React context.
  2. Every consumer pulls that context via useOneSignal(). Whenever any useState setter in the hook is called with a new value, React re-renders OneSignalProvider, generates a fresh context value, and notifies every useContext consumer.
  3. The OneSignal SDK fires the user 'change' event on login, logout, identity verification, alias updates, etc. Each fire executes setOneSignalId(nextOnesignalId ?? undefined) (line 212).
  4. Because no component reads os.oneSignalId, that setState changes the rendered output of zero components β€” but it still triggers a context re-render for every consumer of OneSignalContext.
  5. Similarly, the init flow now performs an extra await OneSignal.User.getOnesignalId() (line 287) whose result is stored only to be ignored.

Impact

Minor and demo-only. The extra getOnesignalId() await on init adds a tiny amount of latency before fetchUserDataFromApi() runs, and the extra setState on each user-state change wastes a render pass across the section components. There is no SDK behavior impact and no user-visible regression β€” this is purely housekeeping in the demo app.

How to fix

Pick one of:

  1. Remove the dead state. Delete the field at line 65, the useState at line 119, the two setOneSignalId calls (~212 and ~287), and the return entry at line 543. This restores the pre-PR behavior of the hook with no functional loss.
  2. Wire oneSignalId back into the UI. If exposing the OneSignal ID was intended (and only the UI surface area was rolled back for stylistic reasons), reintroduce a display in PushSection.tsx (or wherever) that reads os.oneSignalId so the state earns its keep.

Option 1 is the safer demo cleanup; option 2 requires a product decision about whether the ID should be surfaced.

inAppMessagesPaused,
locationShared,
aliasesList,
Expand Down
5 changes: 3 additions & 2 deletions examples/demo/src/screens/HomeScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,14 @@ import { AppColors } from '../theme';
export default function HomeScreen() {
const navigation = useNavigation();
const os = useOneSignal();
const { isReady, promptPush } = os;

const [tooltipVisible, setTooltipVisible] = useState(false);
const [activeTooltip, setActiveTooltip] = useState<TooltipData | null>(null);

useEffect(() => {
if (os.isReady) os.promptPush();
}, [os.isReady, os.promptPush]);
if (isReady) promptPush();
}, [isReady, promptPush]);

const showTooltipModal = (key: string) => {
const tooltip = TooltipHelper.getInstance().getTooltip(key);
Expand Down
76 changes: 50 additions & 26 deletions examples/demo/src/services/OneSignalApiService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,35 +75,59 @@ class OneSignalApiService {
subscriptionId: string,
extra: Record<string, unknown>,
): Promise<boolean> {
try {
const body = {
app_id: this._appId,
include_subscription_ids: [subscriptionId],
headings,
contents,
...extra,
};

const response = await fetch('https://onesignal.com/api/v1/notifications', {
method: 'POST',
headers: {
Accept: 'application/vnd.onesignal.v1+json',
'Content-Type': 'application/json',
},
body: JSON.stringify(body),
});

if (!response.ok) {
const text = await response.text();
console.error(`Send notification failed: ${text}`);
const body = {
app_id: this._appId,
include_subscription_ids: [subscriptionId],
headings,
contents,
...extra,
};

const maxAttempts = 3;

// Retry on `invalid_player_ids` to absorb the brief race where the
// subscription has been created locally but is not yet visible to the
// /notifications endpoint.
for (let attempt = 1; attempt <= maxAttempts; attempt++) {
try {
const response = await fetch('https://onesignal.com/api/v1/notifications', {
method: 'POST',
headers: {
Accept: 'application/vnd.onesignal.v1+json',
'Content-Type': 'application/json',
},
body: JSON.stringify(body),
});

if (!response.ok) {
const text = await response.text();
console.error(`Send notification failed: ${text}`);
return false;
}

const data = (await response.json().catch(() => undefined)) as
| { errors?: { invalid_player_ids?: unknown } }
| undefined;
const invalidIds = data?.errors?.invalid_player_ids;
if (Array.isArray(invalidIds) && invalidIds.length > 0) {
if (attempt < maxAttempts) {
await new Promise<void>((resolve) => setTimeout(() => resolve(), 3_000 * attempt));
continue;
}
console.error(
`Send notification failed: invalid_player_ids ${JSON.stringify(invalidIds)}`,
);
return false;
}

return true;
} catch (err) {
console.error(`Send notification error: ${String(err)}`);
return false;
}

return true;
} catch (err) {
console.error(`Send notification error: ${String(err)}`);
return false;
}

return false;
}

async updateLiveActivity(
Expand Down
10 changes: 0 additions & 10 deletions examples/demo/src/utils/maskValue.ts

This file was deleted.

1 change: 0 additions & 1 deletion examples/demo/types/env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,4 @@ declare module '@env' {
export const ONESIGNAL_APP_ID: string;
export const ONESIGNAL_API_KEY: string;
export const ONESIGNAL_ANDROID_CHANNEL_ID: string;
export const E2E_MODE: string;
}
2 changes: 1 addition & 1 deletion ios/RCTOneSignal/RCTOneSignal.mm
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ - (void)initOneSignal:(NSDictionary *)launchOptions {
return;

OneSignalWrapper.sdkType = @"reactnative";
OneSignalWrapper.sdkVersion = @"050405";
OneSignalWrapper.sdkVersion = @"050406";
// initialize the SDK with a nil app ID so cold start click listeners can be
// triggered
[OneSignal initialize:nil withLaunchOptions:launchOptions];
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "react-native-onesignal",
"version": "5.4.5",
"version": "5.4.6",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟣 Pre-existing: examples/demo/ios/Podfile.lock still pins react-native-onesignal (5.4.4) while package.json is being bumped to 5.4.6 β€” already two releases stale. The release workflow's "Refresh demo Podfile.lock" step (.github/workflows/create-release-pr.yml line 190) is gated behind if: inputs.ios_version != '', so any Android-only or wrapper-only release silently skips the pod-install refresh. Recommend dropping that gate (or running pod install manually for this release) so the lock keeps pace with version bumps.

Extended reasoning...

What the bug is

The demo app's examples/demo/ios/Podfile.lock records react-native-onesignal (5.4.4) at line 1449, but package.json in this PR is being bumped to 5.4.6. The lock file was already stale from the 5.4.5 release and this PR widens the gap to two versions.

Code path that triggers it

In .github/workflows/create-release-pr.yml, the step that refreshes the demo lock file is gated by an iOS-only conditional:

189:      - name: Refresh demo Podfile.lock
190:        if: inputs.ios_version != ''
191:        run: |
...
201:          pod install

This PR is a wrapper + Android-SDK-only release (only android/build.gradle bumped to OneSignal 5.9.2, no iOS native pin change), so inputs.ios_version is empty and the entire pod install step is skipped β€” even though the wrapper itself is being versioned.

Why existing code doesn't prevent it

The gate was likely written assuming that only iOS-native version bumps require a Podfile.lock refresh, but the path-based pod entry (:path: "../node_modules/react-native-onesignal" at line 2230) still records the wrapper's declared version (5.4.4 at line 1449). Whenever package.json changes, that pin gets stale until someone re-runs pod install.

Step-by-step proof

  1. Open examples/demo/ios/Podfile.lock, line 1449 β†’ - react-native-onesignal (5.4.4):
  2. Open package.json in this PR's tree, line 3 β†’ "version": "5.4.6"
  3. Trace the release workflow: this PR was produced by create-release-pr.yml with ios_version empty (only Android SDK bumped). On line 190, if: inputs.ios_version != '' evaluates false β†’ the entire refresh step (including pod install at line 201) is skipped.
  4. Net result: package.json (5.4.6) and Podfile.lock (5.4.4) diverge by 2 versions in this commit.

Impact

Limited but real. The pod entry is path-based, so anyone running pod install locally will re-resolve to whatever is in node_modules/react-native-onesignal (i.e. 5.4.6) β€” so working copies self-heal. The harm is to repo hygiene and reproducibility: a clean checkout reading the in-tree lock without re-running pod install will see the wrong wrapper version recorded, and git diff after a local pod install will keep showing churn until someone commits the refresh.

How to fix

Either (a) drop the if: inputs.ios_version != '' conditional on the Refresh step so any version bump triggers pod install, or (b) broaden the condition to fire when inputs.wrapper_version (or any other version input) is set, or (c) refresh the lock manually for this 5.4.6 release as a one-off catch-up. Option (a) is simplest β€” pod install is idempotent and inexpensive β€” and would have caught both the 5.4.5 miss and this one.

This bug is pre_existing: the workflow gate predates this PR. Flagging because this PR is a concrete instance that further widens the divergence.

"description": "React Native OneSignal SDK",
"keywords": [
"android",
Expand Down
Loading