Skip to content

feat(android): drop the MainActivity permission delegate requirement - #266

Open
matinzd wants to merge 8 commits into
mainfrom
feat/permissions-without-main-activity
Open

feat(android): drop the MainActivity permission delegate requirement#266
matinzd wants to merge 8 commits into
mainfrom
feat/permissions-without-main-activity

Conversation

@matinzd

@matinzd matinzd commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Consumers no longer need to call HealthConnectPermissionDelegate.setPermissionDelegate(this) from MainActivity. Permission and exercise route dialogs are launched from a transparent activity this library declares in its own manifest, which merges into the app automatically.

The obvious alternative — building the intent from the contract and using startActivityForResult — does not work: on Android 14+ the permission contract produces a synthetic intent that only an ActivityResultRegistry can service, so it fails with ActivityNotFoundException. Hosting our own ComponentActivity provides that registry.

HealthConnectPermissionDelegate remains as a deprecated no-op so existing MainActivity code keeps compiling. The Expo ReactActivityLifecycleListener that used to register it is removed, along with the setup steps in the README and docs site.

Testing

Verified on an API 36 emulator with the bare example (no setPermissionDelegate anywhere): initialize succeeds, the real Health Connect consent screen appears, granting returns the permission list to JS, and a repeat request works. Manifest merge confirmed in the app's merged manifest.

🤖 Generated with Claude Code

matinzd and others added 3 commits August 2, 2026 02:11
Permission and exercise route dialogs are now launched from a transparent
activity the library declares in its own manifest, so consumers no longer
need to call HealthConnectPermissionDelegate.setPermissionDelegate(this).

The contracts cannot be launched with a plain startActivityForResult: on
Android 14+ the permission contract produces a synthetic intent that only
an ActivityResultRegistry can service, so hosting our own ComponentActivity
is what makes this work without touching the host activity.

HealthConnectPermissionDelegate is kept as a deprecated no-op, and the Expo
ReactActivityLifecycleListener that used to register it is removed.

Co-Authored-By: Claude Opus 5 <[email protected]>
It existed only to register HealthConnectPermissionDelegate from a
ReactActivityLifecycleListener. Permission dialogs no longer need anything
registered on the host activity, so the module had nothing left to do.

Expo apps now autolink android/ like any other React Native package; the
config plugin in app.plugin.js is unaffected. Verified with a local
expo prebuild, ./gradlew projects and a debug APK dex check.

Co-Authored-By: Claude Opus 5 <[email protected]>
matinzd and others added 4 commits August 2, 2026 02:22
…atinzd/react-native-health-connect into feat/permissions-without-main-activity
They belong to their own PR; only the implementation notes under
.claude/plans/ are specific to this change.

Co-Authored-By: Claude Opus 5 <[email protected]>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Not ready to approve

The new dialog launcher currently has a confirmed race risk around in-flight request guarding (can overwrite the pending request and hang promises) and needs the requested concurrency/validation fixes before it’s safe to merge.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Removes the requirement for consumers to wire HealthConnectPermissionDelegate into MainActivity by moving Health Connect permission/exercise-route dialog launching into a library-declared transparent ComponentActivity, and cleans up the old Expo-native-module path and related docs/CI.

Changes:

  • Introduces HealthConnectPermissionActivity + HealthConnectPermissionLauncher to host Activity Result contracts inside the library (no host MainActivity integration).
  • Deprecates HealthConnectPermissionDelegate into a no-op; updates HealthConnectManager to use the new launcher and adds new exception codes.
  • Removes the android-expo/ Expo module + config, and updates README/docs/examples/CI accordingly.
File summaries
File Description
README.md Removes MainActivity setup instructions; documents the new no-setup behavior and upgrade note.
package.json Stops publishing android-expo/ and expo-module.config.json artifacts.
expo-module.config.json Removed (Expo native module config no longer needed).
example/android/app/src/main/java/com/healthconnectexample/MainActivity.kt Removes now-unnecessary setPermissionDelegate call.
example-expo/README.md Updates Expo example explanation to match “no Expo native module” approach.
docs/docs/get-started.md Mirrors README updates for “no MainActivity changes” and migration notes.
android/src/main/java/dev/matinzd/healthconnect/utils/ExceptionsUtils.kt Adds two new exception types + error codes for new failure modes.
android/src/main/java/dev/matinzd/healthconnect/permissions/HealthConnectPermissionLauncher.kt New launcher that starts the library-owned activity and awaits results.
android/src/main/java/dev/matinzd/healthconnect/permissions/HealthConnectPermissionDelegate.kt Converts delegate to deprecated no-op for backwards compile compatibility.
android/src/main/java/dev/matinzd/healthconnect/permissions/HealthConnectPermissionActivity.kt New transparent activity owning the ActivityResultRegistry and contracts.
android/src/main/java/dev/matinzd/healthconnect/HealthConnectManager.kt Switches permission/route requests to the new launcher; stores provider package name.
android/src/main/AndroidManifest.xml Declares the new internal transparent activity for manifest-merge into host apps.
android-expo/src/main/java/expo/modules/healthconnect/HealthConnectPermissionReactActivityHandler.kt Removed (Expo lifecycle registration no longer needed).
android-expo/src/main/java/expo/modules/healthconnect/HealthConnectPackage.kt Removed (Expo native module removed).
android-expo/src/main/AndroidManifest.xml Removed (Expo module removed).
android-expo/build.gradle Removed (Expo module removed).
.github/workflows/ci.yml Updates CI to validate new Expo/bare linking expectations and that new activity is packaged.
.claude/plans/activity-result-without-main-activity.md Adds design/verification plan documenting rationale and trade-offs for the new approach.
Review details

Suppressed comments (1)

android/src/main/java/dev/matinzd/healthconnect/permissions/HealthConnectPermissionLauncher.kt:58

  • Clearing requestInFlight and setting the pending request should be coordinated with the same lock used to guard the in-flight check; otherwise concurrent calls can still interleave and overwrite pendingRequest before the activity consumes it.
    return try {
      deferred.await()
    } finally {
      requestInFlight = false
    }
  • Files reviewed: 18/18 changed files
  • Comments generated: 2
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment on lines +55 to +65
when (intent.getStringExtra(EXTRA_REQUEST_TYPE)) {
REQUEST_TYPE_PERMISSIONS -> requestPermissions.launch(
intent.getStringArrayListExtra(EXTRA_PERMISSIONS)?.toSet() ?: emptySet()
)

REQUEST_TYPE_EXERCISE_ROUTE -> requestExerciseRoute.launch(
intent.getStringExtra(EXTRA_RECORD_ID) ?: ""
)

else -> finish()
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'd go a step further, validate that the record id is not empty before you even get to this step, and bail out (throw out a log.w telling the dev that there was something wrong and gracefully finish)

Comment on lines +16 to +40
private var requestInFlight = false

suspend fun requestPermissions(
providerPackageName: String, permissions: Set<String>
): Set<String> {
val deferred = CompletableDeferred<Set<String>>()

start(PendingHealthConnectRequest.Permissions(deferred)) {
putExtra(
HealthConnectPermissionActivity.EXTRA_REQUEST_TYPE,
HealthConnectPermissionActivity.REQUEST_TYPE_PERMISSIONS
)
putExtra(
HealthConnectPermissionActivity.EXTRA_PROVIDER_PACKAGE_NAME, providerPackageName
)
putStringArrayListExtra(
HealthConnectPermissionActivity.EXTRA_PERMISSIONS, ArrayList(permissions)
)
}

return try {
deferred.await()
} finally {
requestInFlight = false
}
@TheRogue76

Copy link
Copy Markdown
Collaborator

oooooh let me take a look

@TheRogue76

Copy link
Copy Markdown
Collaborator

Consumers no longer need to call HealthConnectPermissionDelegate.setPermissionDelegate(this) from MainActivity. Permission and exercise route dialogs are launched from a transparent activity this library declares in its own manifest, which merges into the app automatically.

Ehhhhhh, not sure about this bud. Google has been going on a campaign of "Single activity for android apps" for quite some time now. https://developer.android.com/topic/architecture#app_composition
I'll read the rest of the code

Comment on lines +4 to +15

Bare React Native consumers used to be required to edit `MainActivity`:

```kotlin
HealthConnectPermissionDelegate.setPermissionDelegate(this)
```

That existed purely because `ComponentActivity.registerForActivityResult` must be called **before
the activity reaches STARTED**. A native module is constructed far too late to do it itself, so the
library had to borrow the host's activity. Expo projects sidestepped this with a
`ReactActivityLifecycleListener` (`android-expo/.../HealthConnectPermissionReactActivityHandler.kt`);
bare RN has no equivalent hook.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah, so it's this conversation again. Ok i at least understand why you want to do it, but again, i am not 100% sure having a separate activity is the right way to go for it. hmmm

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What was the consensus from the RN core team on your old PR for adding this to core? too much overhead?

@matinzd matinzd Aug 3, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Yep. There wasn't enough bandwidth to move this forward in React Native core at the time.

I think I'll need to start digging into RN core myself and see if I can help get this over the line.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Okay :)

Let's see if this PR lands: react/react-native#57798

@TheRogue76 TheRogue76 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

NGL, i would prefer to keep the MainActivity requirement around for the Bare apps. It's better than altering an applications general architecture (as someone who has had to deal with libraries doing sneaky things with their own activity, those usually suck, they sometimes mess up deep links, and they also sometimes mess up other things)

@matinzd

matinzd commented Aug 3, 2026

Copy link
Copy Markdown
Owner Author

Ehhhhhh, not sure about this bud. Google has been going on a campaign of "Single activity for android apps" for quite some time now. https://developer.android.com/topic/architecture#app_composition
I'll read the rest of the code

Couldn't agree more if there was a way out in bare RN.

@matinzd

matinzd commented Aug 3, 2026

Copy link
Copy Markdown
Owner Author

Depends on: react/react-native#57798

Let's see if it lands in core.

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.

3 participants