refactor(expo): align iOS native components with Expo Modules#8976
Conversation
🦋 Changeset detectedLatest commit: a8aed3c 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe Expo package switches iOS native modules and views to Expo Modules registration, rewires native module and native view TypeScript contracts, and updates client-change event subscription and provider bootstrap code to use the new Expo APIs. ChangesExpo Modules migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@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/vue
commit: |
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
mikepitre
force-pushed
the
mike/expo-ios-expo-modules
branch
from
3e44d6f to
e352b31
Compare
![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 (2)
packages/expo/src/specs/NativeClerkUserProfileView.ts (1)
2-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
NativeSyntheticEventfor the native callback type here too.This has the same contract drift as
NativeClerkAuthView.ts: the local wrapper works forevent.nativeEvent, but it under-types the actual React Native event object and makes these specs inconsistent.♻️ Proposed fix
-import type { ViewProps } from 'react-native'; +import type { NativeSyntheticEvent, ViewProps } from 'react-native'; type ProfileEvent = Readonly<{ type: string }>; -type NativeEvent<T> = Readonly<{ nativeEvent: T }>; interface NativeProps extends ViewProps { isDismissible?: boolean; - onProfileEvent?: (event: NativeEvent<ProfileEvent>) => void; + onProfileEvent?: (event: NativeSyntheticEvent<ProfileEvent>) => void; }Based on learnings, in
packages/expo/src/specs/native event handlers should be typed withNativeSyntheticEventfromreact-native, not a custom wrapper.🤖 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/src/specs/NativeClerkUserProfileView.ts` around lines 2 - 9, The native callback type in NativeClerkUserProfileView is using a custom NativeEvent wrapper instead of the React Native event contract, which makes it inconsistent with the other specs. Update the NativeProps.onProfileEvent signature to use NativeSyntheticEvent from react-native, matching the pattern used in NativeClerkAuthView and keeping the event type aligned with React Native’s actual native callback object.Source: Learnings
packages/expo/src/specs/NativeClerkAuthView.ts (1)
2-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
NativeSyntheticEventfor the native callback type.
requireNativeViewstill exposes a React Native event object here, so the local{ nativeEvent: T }wrapper narrows the contract and drifts from the spec pattern already established in this package.♻️ Proposed fix
-import type { ViewProps } from 'react-native'; +import type { NativeSyntheticEvent, ViewProps } from 'react-native'; type AuthEvent = Readonly<{ type: string }>; -type NativeEvent<T> = Readonly<{ nativeEvent: T }>; interface NativeProps extends ViewProps { mode?: string; isDismissible?: boolean; - onAuthEvent?: (event: NativeEvent<AuthEvent>) => void; + onAuthEvent?: (event: NativeSyntheticEvent<AuthEvent>) => void; }Based on learnings, in
packages/expo/src/specs/native event handlers should be typed withNativeSyntheticEventfromreact-native, not a custom wrapper.🤖 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/src/specs/NativeClerkAuthView.ts` around lines 2 - 10, Update NativeClerkAuthView so the onAuthEvent callback uses NativeSyntheticEvent from react-native instead of the local NativeEvent<T> wrapper, and remove the custom native event type if it is no longer needed. Keep the NativeProps interface aligned with the existing spec pattern in packages/expo/src/specs/ by typing the event parameter as a standard React Native synthetic event.Source: Learnings
🤖 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 @.changeset/quiet-buttons-align.md:
- Line 2: The changeset for `@clerk/expo` is underspecified by using a patch
release for a beta native migration that removes the old iOS bridge/fallback
path. Update the changeset entry in quiet-buttons-align.md to a minor bump so
the release signal matches the impact of the Expo native component migration.
In `@packages/expo/ios/ClerkExpo.podspec`:
- Around line 54-59: The iOS podspec source allowlist is missing the Apple-side
Google Sign-In module, so Expo autolinking can reference a class that never gets
built. Update the ClerkExpo podspec’s s.source_files list to include
ClerkGoogleSignInModule.swift alongside the existing Swift modules so the
ClkerGoogleSignInModule symbol is compiled into the pod again.
---
Nitpick comments:
In `@packages/expo/src/specs/NativeClerkAuthView.ts`:
- Around line 2-10: Update NativeClerkAuthView so the onAuthEvent callback uses
NativeSyntheticEvent from react-native instead of the local NativeEvent<T>
wrapper, and remove the custom native event type if it is no longer needed. Keep
the NativeProps interface aligned with the existing spec pattern in
packages/expo/src/specs/ by typing the event parameter as a standard React
Native synthetic event.
In `@packages/expo/src/specs/NativeClerkUserProfileView.ts`:
- Around line 2-9: The native callback type in NativeClerkUserProfileView is
using a custom NativeEvent wrapper instead of the React Native event contract,
which makes it inconsistent with the other specs. Update the
NativeProps.onProfileEvent signature to use NativeSyntheticEvent from
react-native, matching the pattern used in NativeClerkAuthView and keeping the
event type aligned with React Native’s actual native callback object.
🪄 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: ba780184-842f-4c97-94d2-c56f8b3eafa4
📒 Files selected for processing (28)
.changeset/quiet-buttons-align.mdpackages/expo/app.plugin.jspackages/expo/expo-module.config.jsonpackages/expo/ios/ClerkAuthNativeView.swiftpackages/expo/ios/ClerkAuthViewManager.mpackages/expo/ios/ClerkExpo.podspecpackages/expo/ios/ClerkExpoModule.mpackages/expo/ios/ClerkExpoModule.swiftpackages/expo/ios/ClerkNativeViewHost.swiftpackages/expo/ios/ClerkUserButtonNativeView.swiftpackages/expo/ios/ClerkUserButtonViewManager.mpackages/expo/ios/ClerkUserProfileNativeView.swiftpackages/expo/ios/ClerkUserProfileViewManager.mpackages/expo/src/hooks/__tests__/useNativeClientEvents.test.tspackages/expo/src/hooks/useNativeClientEvents.tspackages/expo/src/plugin/withClerkExpo.tspackages/expo/src/provider/__tests__/ClerkProvider.nativeClientSync.test.tsxpackages/expo/src/provider/nativeClientSync.tsxpackages/expo/src/provider/singleton/__tests__/createClerkInstance.test.tspackages/expo/src/provider/singleton/createClerkInstance.tspackages/expo/src/specs/NativeClerkAuthView.tspackages/expo/src/specs/NativeClerkModule.android.tspackages/expo/src/specs/NativeClerkModule.tspackages/expo/src/specs/NativeClerkModule.web.tspackages/expo/src/specs/NativeClerkUserButtonView.tspackages/expo/src/specs/NativeClerkUserProfileView.tspackages/expo/src/utils/__tests__/native-module.test.tspackages/expo/src/utils/native-module.ts
💤 Files with no reviewable changes (8)
- packages/expo/ios/ClerkUserButtonViewManager.m
- packages/expo/src/provider/tests/ClerkProvider.nativeClientSync.test.tsx
- packages/expo/src/provider/singleton/tests/createClerkInstance.test.ts
- packages/expo/ios/ClerkUserProfileViewManager.m
- packages/expo/ios/ClerkExpoModule.m
- packages/expo/ios/ClerkAuthViewManager.m
- packages/expo/src/hooks/useNativeClientEvents.ts
- packages/expo/src/utils/tests/native-module.test.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8162438d17
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67681fe14e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
2 participants
Summary
Summary by CodeRabbit