fix(expo): recreate Android recomposer after reattach#9143
Conversation
|
@D3OXY is attempting to deploy a commit to the Clerk Production Team on Vercel. A member of the Team first needs to authorize it. |
🦋 Changeset detectedLatest commit: fbfa349 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughThe Android Compose native view host now recreates recomposition after window reattachment, disposes composition state during detachment, and cancels stale recomposer resources. A Changesets entry records a patch release for ChangesExpo Compose lifecycle
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/expo/android/src/main/java/expo/modules/clerk/ClerkComposeNativeViewHost.kt (1)
27-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer importing
kotlinx.coroutines.Job/SupervisorJobinstead of fully-qualifying inline.Fully-qualified references (
kotlinx.coroutines.Job,kotlinx.coroutines.SupervisorJob) are repeated across the field declaration andstartRecomposer(); importing them would improve readability.Also applies to: 46-46, 54-54, 62-63, 65-65
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/expo/android/src/main/java/expo/modules/clerk/ClerkComposeNativeViewHost.kt` at line 27, Replace the fully qualified kotlinx.coroutines.Job and kotlinx.coroutines.SupervisorJob references in ClerkComposeNativeViewHost, including recomposerJob and startRecomposer(), with imports at the top of the file. Preserve the existing coroutine behavior and declarations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/expo/android/src/main/java/expo/modules/clerk/ClerkComposeNativeViewHost.kt`:
- Around line 53-67: Update startRecomposer so recomposerJob references the Job
returned by the coroutine launched to run
newRecomposer.runRecomposeAndApplyChanges(), rather than the standalone
SupervisorJob in the scope context. Preserve the existing scope and recomposer
setup, while ensuring the isActive guard permits recovery after the
recomposition coroutine completes or fails.
- Around line 53-67: Update ClerkAuthNativeView and ClerkUserProfileNativeView
so onAttachedToWindow() invokes setupView(), matching ClerkUserButtonNativeView.
Preserve the existing OnViewDidUpdateProps behavior and ensure reattaching
without prop changes recreates the Compose content after the recomposer
restarts.
---
Nitpick comments:
In
`@packages/expo/android/src/main/java/expo/modules/clerk/ClerkComposeNativeViewHost.kt`:
- Line 27: Replace the fully qualified kotlinx.coroutines.Job and
kotlinx.coroutines.SupervisorJob references in ClerkComposeNativeViewHost,
including recomposerJob and startRecomposer(), with imports at the top of the
file. Preserve the existing coroutine behavior and declarations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: c8505747-f658-4f76-b111-5055c5f794dd
📒 Files selected for processing (2)
.changeset/fuzzy-lions-recompose.mdpackages/expo/android/src/main/java/expo/modules/clerk/ClerkComposeNativeViewHost.kt
| private fun startRecomposer() { | ||
| if (activity == null || recomposerJob?.isActive == true) return | ||
|
|
||
| // Navigation can temporarily detach and later reattach the same native host. | ||
| // Always give a reattached ComposeView a live parent composition context. | ||
| val recomposerContext = AndroidUiDispatcher.Main | ||
| val newRecomposer = Recomposer(recomposerContext) | ||
| recomposer = newRecomposer | ||
| composeView.setParentCompositionContext(newRecomposer) | ||
| val scope = CoroutineScope(recomposerContext + kotlinx.coroutines.SupervisorJob()) | ||
| recomposerJob = scope.coroutineContext[kotlinx.coroutines.Job] | ||
| scope.launch { | ||
| newRecomposer.runRecomposeAndApplyChanges() | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
recomposerJob tracks the wrong job — isActive guard can silently skip recovery.
recomposerJob is set to scope.coroutineContext[Job], which is the root SupervisorJob(), not the coroutine launched at Line 64-66. A SupervisorJob() created this way stays isActive == true even after its child coroutine (runRecomposeAndApplyChanges()) completes or throws, because supervisor semantics isolate child failures from the parent and the parent job doesn't auto-complete when its lone child finishes.
Concretely: if the recomposition coroutine ever fails or exits, recomposerJob?.isActive will still report true on the next onAttachedToWindow(), so Line 54's guard causes startRecomposer() to skip creating a fresh recomposer — reintroducing the exact "stale, non-functional recomposer" symptom this PR is fixing, just triggered by a recomposition failure instead of a cancel/detach.
Track the actually-launched job instead:
🐛 Proposed fix
val scope = CoroutineScope(recomposerContext + kotlinx.coroutines.SupervisorJob())
- recomposerJob = scope.coroutineContext[kotlinx.coroutines.Job]
- scope.launch {
+ recomposerJob = scope.launch {
newRecomposer.runRecomposeAndApplyChanges()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private fun startRecomposer() { | |
| if (activity == null || recomposerJob?.isActive == true) return | |
| // Navigation can temporarily detach and later reattach the same native host. | |
| // Always give a reattached ComposeView a live parent composition context. | |
| val recomposerContext = AndroidUiDispatcher.Main | |
| val newRecomposer = Recomposer(recomposerContext) | |
| recomposer = newRecomposer | |
| composeView.setParentCompositionContext(newRecomposer) | |
| val scope = CoroutineScope(recomposerContext + kotlinx.coroutines.SupervisorJob()) | |
| recomposerJob = scope.coroutineContext[kotlinx.coroutines.Job] | |
| scope.launch { | |
| newRecomposer.runRecomposeAndApplyChanges() | |
| } | |
| } | |
| private fun startRecomposer() { | |
| if (activity == null || recomposerJob?.isActive == true) return | |
| // Navigation can temporarily detach and later reattach the same native host. | |
| // Always give a reattached ComposeView a live parent composition context. | |
| val recomposerContext = AndroidUiDispatcher.Main | |
| val newRecomposer = Recomposer(recomposerContext) | |
| recomposer = newRecomposer | |
| composeView.setParentCompositionContext(newRecomposer) | |
| val scope = CoroutineScope(recomposerContext + kotlinx.coroutines.SupervisorJob()) | |
| recomposerJob = scope.launch { | |
| newRecomposer.runRecomposeAndApplyChanges() | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/expo/android/src/main/java/expo/modules/clerk/ClerkComposeNativeViewHost.kt`
around lines 53 - 67, Update startRecomposer so recomposerJob references the Job
returned by the coroutine launched to run
newRecomposer.runRecomposeAndApplyChanges(), rather than the standalone
SupervisorJob in the scope context. Preserve the existing scope and recomposer
setup, while ensuring the isActive guard permits recovery after the
recomposition coroutine completes or fails.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all subclasses of ClerkComposeNativeViewHost and check their onAttachedToWindow overrides
rg -n --type=kt 'ClerkComposeNativeViewHost' -g '*.kt'
ast-grep run --pattern 'override fun onAttachedToWindow() {
$$$
}' --lang kotlin packages/expo/android/src/main/java/expo/modules/clerkRepository: clerk/javascript
Length of output: 1065
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files and search for subclasses/reattach patterns.
git ls-files 'packages/expo/android/src/main/java/expo/modules/clerk/*.kt'
echo
echo '--- base class ---'
sed -n '1,220p' packages/expo/android/src/main/java/expo/modules/clerk/ClerkComposeNativeViewHost.kt
echo
echo '--- user button module ---'
sed -n '1,220p' packages/expo/android/src/main/java/expo/modules/clerk/ClerkUserButtonViewModule.kt
echo
echo '--- subclass search ---'
rg -n 'class\s+\w+\s*:\s*ClerkComposeNativeViewHost|:.*ClerkComposeNativeViewHost' packages/expo/android/src/main/java/expo/modules/clerk
echo
echo '--- setupView search ---'
rg -n 'setupView\(\)|onAttachedToWindow\(\)' packages/expo/android/src/main/java/expo/modules/clerkRepository: clerk/javascript
Length of output: 6981
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for any other references to the base host and its attach/setup behavior.
rg -n --glob '*.kt' 'ClerkComposeNativeViewHost|setupView\(\)|onAttachedToWindow\(\)' packages/expo/android/src/main/javaRepository: clerk/javascript
Length of output: 1908
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- ClerkAuthViewModule.kt ---'
sed -n '1,180p' packages/expo/android/src/main/java/expo/modules/clerk/ClerkAuthViewModule.kt
echo
echo '--- ClerkUserProfileViewModule.kt ---'
sed -n '1,180p' packages/expo/android/src/main/java/expo/modules/clerk/ClerkUserProfileViewModule.ktRepository: clerk/javascript
Length of output: 5821
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the exact lifecycle hooks for the remaining subclasses.
ast-grep outline packages/expo/android/src/main/java/expo/modules/clerk/ClerkAuthViewModule.kt --view expanded
echo
ast-grep outline packages/expo/android/src/main/java/expo/modules/clerk/ClerkUserProfileViewModule.kt --view expandedRepository: clerk/javascript
Length of output: 1803
Re-run setupView() on reattach for the other native views. ClerkAuthNativeView and ClerkUserProfileNativeView still only call setupView() from OnViewDidUpdateProps; only ClerkUserButtonNativeView does it in onAttachedToWindow(). If either view is detached and reattached without a prop change, the recomposer restarts but the Compose content does not.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/expo/android/src/main/java/expo/modules/clerk/ClerkComposeNativeViewHost.kt`
around lines 53 - 67, Update ClerkAuthNativeView and ClerkUserProfileNativeView
so onAttachedToWindow() invokes setupView(), matching ClerkUserButtonNativeView.
Preserve the existing OnViewDidUpdateProps behavior and ensure reattaching
without prop changes recreates the Compose content after the recomposer
restarts.
1 participant
Description
Fixes #9142
Android native Clerk views currently create a parent
Recomposeronce, then permanently cancel it whenever the view temporarily detaches from its window. React Navigation can detach and reattach the same header view, so the reattachedComposeViewkeeps a canceled parent composition context.UserButtonthen shows a stale/fallback avatar and no longer handles taps.This change moves recomposer creation into the attach lifecycle. On detach it disposes the active composition, cancels and clears the recomposer state, then creates a fresh parent recomposer before subclasses call
setContent()on the next attach.The same lifecycle fix applies to all Android prebuilt views hosted by
ClerkComposeNativeViewHost. iOS is unchanged.Validation
pnpm --filter @clerk/expo... buildpnpm --filter @clerk/expo test— 92 tests passedpnpm --filter @clerk/expo lint— no errorspnpm --filter @clerk/expo format:checkBUILD SUCCESSFULChecklist
pnpm testruns as expected; targeted Expo tests pass.pnpm buildruns as expected; filtered Expo dependency build passes.Type of change
Summary by CodeRabbit