Uh oh!
There was an error while loading. Please reload this page.
feat(expo): support custom logos in native AuthView - #9163
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: c70a4db 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 |
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCustom React Native logo content is supported by Expo Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
@clerk/astro@clerk/backend@clerk/chrome-extension@clerk/clerk-js@clerk/electron@clerk/electron-passkeys@clerk/eslint-plugin@clerk/expo@clerk/expo-google-signin@clerk/expo-passkeys@clerk/express@clerk/fastify@clerk/hono@clerk/localizations@clerk/nextjs@clerk/nuxt@clerk/react@clerk/react-router@clerk/shared@clerk/tanstack-react-start@clerk/testing@clerk/ui@clerk/upgrade@clerk/vuecommit: |
API Changes Report
Summary
@clerk/expoCurrent version: 4.0.3 Subpath |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/ios/ClerkAuthNativeView.swift`:
- Around line 81-87: Update the logoBoundsObservation callback and its
asynchronous main-thread block so queued work reads the current logo view bounds
when it executes, rather than applying the asynchronously captured bounds.size.
Continue validating the view identity and only update logoSize and call
setNeedsHostedViewUpdate when the latest size differs.
🪄 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: 62273dd8-3af2-454f-acf8-27bbd45b925c
📒 Files selected for processing (7)
.changeset/expo-native-custom-logo.mdpackages/expo/android/src/main/java/expo/modules/clerk/ClerkAuthViewModule.ktpackages/expo/ios/ClerkAuthNativeView.swiftpackages/expo/ios/ClerkNativeBridge.swiftpackages/expo/src/native/AuthView.tsxpackages/expo/src/native/AuthView.types.tspackages/expo/src/native/__tests__/AuthView.test.tsx
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/expo/ios/Tests/ClerkAuthNativeViewPaperTests.swift (1)
1-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise both React Native architectures in this test.
ClerkAuthNativeView.swifthas separate Paper and Fabric child-lifecycle paths, but this entire test is excluded whenRCT_NEW_ARCH_ENABLEDis set. Add equivalent Fabric coverage so the new-architecture path cannot regress while the suite remains green.🤖 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/ios/Tests/ClerkAuthNativeViewPaperTests.swift` around lines 1 - 30, Extend ClerkAuthNativeViewPaperTests coverage to also exercise the Fabric child-lifecycle path when RCT_NEW_ARCH_ENABLED is enabled. Add equivalent assertions for inserting, updating, and removing the React child through ClerkAuthNativeView’s Fabric APIs, while preserving the existing Paper test and its host-hierarchy expectations.
🤖 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.
Nitpick comments:
In `@packages/expo/ios/Tests/ClerkAuthNativeViewPaperTests.swift`:
- Around line 1-30: Extend ClerkAuthNativeViewPaperTests coverage to also
exercise the Fabric child-lifecycle path when RCT_NEW_ARCH_ENABLED is enabled.
Add equivalent assertions for inserting, updating, and removing the React child
through ClerkAuthNativeView’s Fabric APIs, while preserving the existing Paper
test and its host-hierarchy expectations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 020949b2-d4fc-4b4b-88fa-ca88393e9722
📒 Files selected for processing (4)
packages/expo/ios/ClerkAuthNativeView.swiftpackages/expo/ios/ClerkExpo.podspecpackages/expo/ios/ClerkNativeBridge.swiftpackages/expo/ios/Tests/ClerkAuthNativeViewPaperTests.swift
wobsoriano
left a comment
There was a problem hiding this comment.
Looks good! The DX is right 👍🏼
I pushed a commit adding Maestro coverage, the detach/reattach flow now asserts a custom logo renders in the native sloth on both platforms
Description
Adds a
logoprop to the Expo nativeAuthView, allowing apps to replace the dashboard-configured logo with arbitrary React Native content on Android and iOS.The React element is mounted as the native view's single child and rehosted inside the Clerk Compose/SwiftUI logo slot while preserving its React Native layout dimensions. Callers retain control over sizing, spacing, and accessibility.
This ports the custom logo-view APIs introduced by clerk/clerk-android#801 and clerk/clerk-ios#520.
Resolves MOBILE-526