Skip to content

chore: [SDK-4728] add no-location demo and switch to env var opt-out#1956

Merged
fadi-george merged 5 commits into
fadi/sdk-4728from
fadi/sdk-4728-cleanup
Jun 9, 2026
Merged

chore: [SDK-4728] add no-location demo and switch to env var opt-out#1956
fadi-george merged 5 commits into
fadi/sdk-4728from
fadi/sdk-4728-cleanup

Conversation

@fadi-george

Copy link
Copy Markdown
Collaborator

Description

One Line Summary

Adds the demo-no-location example app and switches the disable-location opt-out to a single ONESIGNAL_DISABLE_LOCATION environment variable.

Details

Motivation

Stacked on top of #1953. That PR added the ability to exclude OneSignal's native location module. This PR:

  1. Replaces the per-platform knobs introduced in feat: [SDK-4728] support disabling location module #1953 (Android OneSignal_disableLocation gradle property and iOS $OneSignalDisableLocation Podfile global) with a single ONESIGNAL_DISABLE_LOCATION environment variable, read at dependency-resolution time on both platforms. This keeps the opt-out consistent across iOS (CocoaPods) and Android (Gradle), and matches the approach used in the OneSignal Flutter SDK.
  2. Adds a minimal runnable examples/demo-no-location app that proves push + in-app messaging work without linking the native location module, and cleans up cruft that was copied into it (the unused com.demo Android package and the orphaned vine_boom.wav asset with its dangling Xcode references).

Scope

  • Core SDK: android/build.gradle and react-native-onesignal.podspec now read ONESIGNAL_DISABLE_LOCATION; default behavior (location included) is unchanged when the variable is unset.
  • Demo only otherwise; no runtime SDK code paths change.

Testing

Manual testing

Validated with clean reinstalls in examples/demo-no-location:

  • iOS (pod install with the env var, via the Podfile): Podfile.lock resolves react-native-onesignal to OneSignalXCFramework/OneSignal + OneSignalXCFramework/OneSignalInAppMessages only — no OneSignalLocation subspec.
  • Android (./gradlew :app:dependencies with the env var): resolves com.onesignal:core + notifications + in-app-messages (+ transitive otel), with no com.onesignal:location and no aggregate com.onesignal:OneSignal.
  • Control (Android without the env var): the aggregate com.onesignal:OneSignal pulls com.onesignal:location back in, confirming the variable is what excludes the module.

Affected code checklist

  • Public API changes (build-time opt-out mechanism)

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Made with Cursor

@fadi-george fadi-george requested a review from a team as a code owner June 9, 2026 20:48
@fadi-george fadi-george changed the base branch from fadi/sdk-4728-disable-location-module to fadi/sdk-4728 June 9, 2026 20:51

@sherwinski sherwinski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  1. https://github.com/OneSignal/react-native-onesignal/blob/fadi/sdk-4728-cleanup/examples/demo/ios/demo/vine_boom.wav is still on disk even though it's dereferenced in res/raw. We should remove it there too.
  2. To avoid headaches in testing, we should normalize how we parse ONESIGNAL_DISABLE_LOCATION from env:
    a. Android: value.toString().toBoolean() is case-insensitive, so true/TRUE/True all work, but 1 does not.
    b. iOS: ENV['ONESIGNAL_DISABLE_LOCATION'] == 'true' requires case-sensitive exact match, so TRUE and 1 both fail.

fadi-george and others added 2 commits June 9, 2026 15:05
…ATION env var

Replace the Android gradle property (OneSignal_disableLocation) and the
iOS Podfile global ($OneSignalDisableLocation) with a single
ONESIGNAL_DISABLE_LOCATION environment variable read at dependency
resolution time, so the same knob works for both platforms.

Co-authored-by: Cursor <cursoragent@cursor.com>
Remove leftover cruft from the demo (the unused com.demo Android package
and the orphaned vine_boom.wav asset plus its dangling Xcode references),
and switch the demo to the ONESIGNAL_DISABLE_LOCATION environment
variable for both iOS (Podfile) and Android (run script).

Co-authored-by: Cursor <cursoragent@cursor.com>
fadi-george and others added 3 commits June 9, 2026 15:05
Move the no-location environment variable out of the Podfile and onto the package scripts that run CocoaPods resolution, keeping the Podfile conventional while preserving the demo's self-contained setup.

Co-authored-by: Cursor <cursoragent@cursor.com>
Update the e2e workflow version resolver after the Android dependency string moved behind the oneSignalVersion variable.

Co-authored-by: Cursor <cursoragent@cursor.com>
Clarify that GitHub Actions can set ONESIGNAL_DISABLE_LOCATION at the job or step level so CocoaPods and Gradle inherit it during dependency resolution and builds.

Co-authored-by: Cursor <cursoragent@cursor.com>
@fadi-george fadi-george force-pushed the fadi/sdk-4728-cleanup branch from 1f0fc7d to b0a12cb Compare June 9, 2026 22:05
@fadi-george fadi-george merged commit 854e446 into fadi/sdk-4728 Jun 9, 2026
2 checks passed
@fadi-george fadi-george deleted the fadi/sdk-4728-cleanup branch June 9, 2026 22:08
@fadi-george fadi-george restored the fadi/sdk-4728-cleanup branch June 9, 2026 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants