feat(clerk-expo): Add native Apple Sign-In support for iOS #7053
Conversation
- Introduced `useAppleSignIn` hook for native Apple Sign-In on iOS using expo-apple-authentication. - Implemented flow to handle sign-in and sign-up using Clerk's backend. - Added type definitions for parameters and return types of the hook. - Updated `pnpm-lock.yaml` to include expo-apple-authentication as a dependency. - Modified strategies.ts to include 'oauth_token_apple' strategy type.
…orresponding tests
🦋 Changeset detectedLatest commit: bed16e5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
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.
|
WalkthroughAdds native iOS Apple Sign-In to the Expo package via a new Changes
Sequence Diagram(s)sequenceDiagram
participant App as Expo App
participant Hook as useAppleSignIn
participant AppleAuth as expo-apple-authentication
participant Crypto as expo-crypto
participant Clerk as Clerk SDK
App->>Hook: startAppleSignInFlow()
Hook->>Hook: Validate platform (iOS)
Hook->>Crypto: Generate nonce
Hook->>AppleAuth: requestAsync (FULL_NAME, EMAIL)
alt User Cancels
AppleAuth-->>Hook: ERR_REQUEST_CANCELED
Hook-->>App: { createdSessionId: null }
else Apple Auth Success
AppleAuth-->>Hook: identityToken
Hook->>Clerk: signIn with oauth_token_apple
alt Existing User
Clerk-->>Hook: signIn verified
Hook->>Clerk: setActive
Hook-->>App: { createdSessionId, signIn }
else New User (Transfer)
Clerk-->>Hook: signIn.firstFactorVerification.status = 'transferable'
Hook->>Clerk: signUp with transfer
Clerk-->>Hook: signUp verified
Hook->>Clerk: setActive
Hook-->>App: { createdSessionId, signIn, signUp }
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (5)**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
integration/**📄 CodeRabbit inference engine (.cursor/rules/global.mdc)
Files:
integration/**/*📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Files:
**/*.{js,ts,tsx,jsx}📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 1
🧹 Nitpick comments (2)
packages/expo/src/hooks/useAppleSignIn.ts (2)
86-87: Consider using cryptographically secure nonce generation.The current nonce generation using
Math.random()is not cryptographically secure. While this may be acceptable if Clerk's backend validates the identity token independently, consider using a more secure random source.Consider using expo-crypto for secure random generation:
+import * as Crypto from 'expo-crypto'; + // Generate a nonce for the Apple Sign-In request (required by Clerk) -const nonce = Math.random().toString(36).substring(2, 15) + Math.random().toString(36).substring(2, 15); +const nonce = Crypto.randomUUID();Note: This would require adding expo-crypto as a dependency. Verify with the Clerk backend team whether the current approach meets security requirements.
105-109: Consider removing type assertions once types are fully propagated.The
as anycasts on lines 107 and 109 bypass TypeScript's type checking. While this is acceptable during the rollout of new types, consider:
- Add a comment explaining why the casts are temporarily needed:
// Create a SignIn with the Apple ID token strategy +// Note: Type assertions needed until @clerk/clerk-react types are updated await signIn.create({ strategy: 'oauth_token_apple' as any, token: identityToken, } as any);
- Alternatively, if the types in
@clerk/typesare now properly defined, verify whether these casts can be removed entirely by ensuring@clerk/clerk-reacthas the updated types.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.changeset/brave-apples-sign.md(1 hunks)packages/expo/package.json(3 hunks)packages/expo/src/hooks/__tests__/useAppleSignIn.test.ts(1 hunks)packages/expo/src/hooks/index.ts(1 hunks)packages/expo/src/hooks/useAppleSignIn.ts(1 hunks)packages/types/src/signInCommon.ts(2 hunks)packages/types/src/signUpCommon.ts(2 hunks)packages/types/src/strategies.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/types/src/strategies.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.tspackages/expo/src/hooks/index.tspackages/expo/src/hooks/useAppleSignIn.tspackages/types/src/signUpCommon.tspackages/types/src/signInCommon.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/types/src/strategies.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.tspackages/expo/src/hooks/index.tspackages/expo/src/hooks/useAppleSignIn.tspackages/expo/package.jsonpackages/types/src/signUpCommon.tspackages/types/src/signInCommon.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/types/src/strategies.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.tspackages/expo/src/hooks/index.tspackages/expo/src/hooks/useAppleSignIn.tspackages/types/src/signUpCommon.tspackages/types/src/signInCommon.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/types/src/strategies.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.tspackages/expo/src/hooks/index.tspackages/expo/src/hooks/useAppleSignIn.tspackages/types/src/signUpCommon.tspackages/types/src/signInCommon.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/types/src/strategies.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.tspackages/expo/src/hooks/index.tspackages/expo/src/hooks/useAppleSignIn.tspackages/types/src/signUpCommon.tspackages/types/src/signInCommon.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/types/src/strategies.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.tspackages/expo/src/hooks/index.tspackages/expo/src/hooks/useAppleSignIn.tspackages/types/src/signUpCommon.tspackages/types/src/signInCommon.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/expo/src/hooks/__tests__/useAppleSignIn.test.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/expo/src/hooks/__tests__/useAppleSignIn.test.ts
packages/**/index.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use tree-shaking friendly exports
Files:
packages/expo/src/hooks/index.ts
**/index.ts
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Use index.ts files for clean imports but avoid deep barrel exports
Avoid barrel files (index.ts re-exports) as they can cause circular dependencies
Files:
packages/expo/src/hooks/index.ts
packages/*/package.json
📄 CodeRabbit inference engine (.cursor/rules/global.mdc)
All publishable packages should be placed under the packages/ directory
packages/*/package.json: All publishable packages must be located in the 'packages/' directory.
All packages must be published under the @clerk namespace on npm.
Semantic versioning must be used across all packages.
Files:
packages/expo/package.json
.changeset/**
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/brave-apples-sign.md
🧬 Code graph analysis (4)
packages/expo/src/hooks/__tests__/useAppleSignIn.test.ts (1)
packages/expo/src/hooks/useAppleSignIn.ts (1)
useAppleSignIn(56-156)
packages/expo/src/hooks/useAppleSignIn.ts (3)
packages/types/src/clerk.ts (1)
SetActive(1268-1268)packages/types/src/signIn.ts (1)
SignInResource(34-91)packages/types/src/signUp.ts (1)
SignUpResource(40-121)
packages/types/src/signUpCommon.ts (1)
packages/types/src/strategies.ts (1)
AppleIdTokenStrategy(5-5)
packages/types/src/signInCommon.ts (1)
packages/types/src/strategies.ts (1)
AppleIdTokenStrategy(5-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (21)
packages/expo/package.json (1)
97-97: LGTM! Dependency configuration follows established patterns.The expo-apple-authentication package is correctly configured across devDependencies, peerDependencies, and peerDependenciesMeta, following the same pattern as other optional Expo packages like expo-local-authentication and expo-secure-store.
Also applies to: 106-106, 119-121
.changeset/brave-apples-sign.md (1)
1-6: LGTM! Changeset properly documents the new feature.The version bumps are appropriate: minor for @clerk/clerk-expo (new feature) and patch for @clerk/types (type additions). The description clearly states the feature and requirements.
packages/types/src/signUpCommon.ts (1)
12-12: LGTM! Type extension properly supports Apple ID token-based signup.The AppleIdTokenStrategy is correctly imported and added to the SignUpCreateParams strategy union, enabling the new Apple Sign-In flow during user registration.
Also applies to: 93-93
packages/expo/src/hooks/index.ts (1)
14-14: LGTM! Export follows established barrel pattern.The useAppleSignIn hook is correctly exported alongside other Expo-specific hooks.
packages/types/src/strategies.ts (1)
5-5: LGTM! Strategy type follows naming conventions.The AppleIdTokenStrategy type is correctly defined with the 'oauth_token_apple' literal, following the established pattern for token-based strategies.
packages/types/src/signInCommon.ts (1)
43-43: LGTM! Type extension correctly supports Apple ID token-based sign-in.The AppleIdTokenStrategy is properly imported and added to SignInCreateParams as a discriminated union variant with the required token field, following the same pattern as GoogleOneTapStrategy.
Also applies to: 141-144
packages/expo/src/hooks/__tests__/useAppleSignIn.test.ts (6)
6-39: LGTM! Test mocks are properly configured.The test setup correctly mocks all dependencies (@clerk/clerk-react, expo-apple-authentication, react-native) using vitest's hoisting pattern, ensuring isolated and reliable tests.
41-76: LGTM! Test fixtures and lifecycle hooks are well-structured.The mock objects correctly represent Clerk's SignIn and SignUp resources, and the beforeEach/afterEach hooks properly initialize and clean up the test state.
79-84: LGTM! Basic hook structure test is appropriate.The test verifies the hook returns the expected API surface.
86-157: LGTM! Sign-in flow tests comprehensively cover main scenarios.The tests effectively verify:
- Successful sign-in for existing users with proper token exchange
- Transfer flow for new users requiring sign-up
- Graceful handling of user cancellation
All assertions validate the correct behavior and return values.
159-179: LGTM! Error handling tests validate proper error conditions.The tests correctly verify that appropriate errors are thrown when:
- Apple Authentication is unavailable on the device
- No identity token is received from Apple
181-195: LGTM! Early return test validates defensive loading check.The test correctly verifies that when Clerk isn't fully loaded, the hook returns early without attempting Apple Authentication, preventing potential runtime errors.
packages/expo/src/hooks/useAppleSignIn.ts (9)
19-55: LGTM! Excellent documentation with practical example.The JSDoc provides comprehensive documentation including a realistic usage example, platform requirements, and peer dependency information.
56-58: LGTM! Hook initialization correctly uses Clerk hooks.Both useSignIn and useSignUp are properly called to obtain the necessary resources and loading states.
64-68: LGTM! Platform check with helpful error message.The iOS-only restriction is correctly enforced with a clear error message that guides developers to use the alternative OAuth flow on other platforms.
70-77: LGTM! Proper defensive check for Clerk loading state.The early return when Clerk isn't loaded prevents potential runtime errors and returns a consistent structure for callers to handle.
79-83: LGTM! Device capability check is appropriate.The availability check ensures Apple Authentication is supported before proceeding, with a clear error message for unsupported devices.
98-103: LGTM! Identity token validation is appropriate.The code correctly validates that an identity token was received from Apple before proceeding.
111-127: LGTM! Transfer flow correctly handles new user sign-up.The code properly detects when a user doesn't exist (transferable status) and creates a SignUp with the transfer flag, preserving any unsafe metadata provided by the caller.
129-150: LGTM! Error handling gracefully manages cancellation and propagates other errors.The code correctly:
- Returns the sign-in session ID for existing users
- Handles user cancellation gracefully by returning null without throwing
- Propagates other errors for proper error handling upstream
153-155: LGTM! Hook return follows React conventions.The hook cleanly returns an object with the startAppleSignInFlow function.
What!? I will fix this... I don't think thats correct. |
Ahhh... This is everyone elses changes since my branch was created. Only two of my packages will be updated. |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/expo/src/hooks/useAppleSignIn.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/expo/src/hooks/useAppleSignIn.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/expo/src/hooks/useAppleSignIn.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/expo/src/hooks/useAppleSignIn.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/expo/src/hooks/useAppleSignIn.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/expo/src/hooks/useAppleSignIn.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/expo/src/hooks/useAppleSignIn.ts
🧬 Code graph analysis (1)
packages/expo/src/hooks/useAppleSignIn.ts (3)
packages/types/src/clerk.ts (1)
SetActive(1268-1268)packages/types/src/signIn.ts (1)
SignInResource(34-91)packages/types/src/signUp.ts (1)
SignUpResource(40-121)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (7)
packages/expo/src/hooks/useAppleSignIn.ts (7)
2-2: Past review issue resolved.The missing
SignUpUnsafeMetadataimport flagged in the previous review has been correctly added.
19-55: Excellent documentation.The JSDoc is comprehensive, includes a practical example, and clearly documents platform restrictions and peer dependencies.
64-68: Good platform enforcement.The iOS-only check with a descriptive error message guiding users to the web OAuth alternative is well-implemented.
113-129: Transfer flow logic is well-implemented.The conditional handling of the
transferablestatus correctly routes new users through the SignUp flow while preserving optional metadata.
138-152: Robust error handling.The distinction between user cancellation (
ERR_REQUEST_CANCELED) and other errors is appropriate. Returning gracefully on cancellation allows the UI to handle the flow without treating it as an error.
155-157: Clean hook interface.The return structure follows React hook conventions and provides a clear API surface.
108-111: The review comment is based on flawed assumptions about type availability.The
AppleIdTokenStrategytype does exist in@clerk/types, but it's not included in thestrategyparameter union ofSignInFutureCreateParams. Thestrategyfield acceptsOAuthStrategy | 'saml' | 'enterprise_sso' | PasskeyStrategy, andAppleIdTokenStrategy('oauth_token_apple') is not part of this union.Using
as AppleIdTokenStrategy(as suggested) would not eliminate the type assertion problem—TypeScript would still reject it sinceAppleIdTokenStrategyis not an accepted strategy type. The currentas anyapproach is a reasonable temporary workaround until the type definition inSignInFutureCreateParamsis updated to includeAppleIdTokenStrategyin its union.Likely an incorrect or invalid review comment.
chriscanin
changed the title
chriscanin
changed the title
chriscanin
changed the title
…eneration in useAppleSignIn hook - Added expo-crypto as a dependency in package.json. - Updated useAppleSignIn hook to use Crypto.randomUUID() for generating a cryptographically secure nonce instead of Math.random(). - Mocked expo-crypto in useAppleSignIn tests to return a fixed UUID for consistent testing. - Updated tests to verify that randomUUID is called and the correct nonce is used in the sign-in flow.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/expo/src/hooks/useAppleSignIn.ts (1)
138-152: Consider usingunknownfor caught errors.The error handling logic is correct, but the error parameter uses
anywhich goes against TypeScript best practices.Apply this diff to improve type safety:
- } catch (error: any) { + } catch (error: unknown) { // Handle Apple Authentication errors - if (error?.code === 'ERR_REQUEST_CANCELED') { + if (error && typeof error === 'object' && 'code' in error && error.code === 'ERR_REQUEST_CANCELED') { // User canceled the sign-in flowAlternatively, define a type guard:
function isAppleAuthError(error: unknown): error is { code: string } { return typeof error === 'object' && error !== null && 'code' in error; } // Then use: if (isAppleAuthError(error) && error.code === 'ERR_REQUEST_CANCELED') {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/expo/package.json(2 hunks)packages/expo/src/hooks/__tests__/useAppleSignIn.test.ts(1 hunks)packages/expo/src/hooks/useAppleSignIn.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/expo/package.json
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/expo/src/hooks/useAppleSignIn.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/expo/src/hooks/useAppleSignIn.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/expo/src/hooks/useAppleSignIn.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/expo/src/hooks/useAppleSignIn.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/expo/src/hooks/useAppleSignIn.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/expo/src/hooks/useAppleSignIn.tspackages/expo/src/hooks/__tests__/useAppleSignIn.test.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/expo/src/hooks/__tests__/useAppleSignIn.test.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/expo/src/hooks/__tests__/useAppleSignIn.test.ts
🧬 Code graph analysis (2)
packages/expo/src/hooks/useAppleSignIn.ts (3)
packages/types/src/clerk.ts (1)
SetActive(1268-1268)packages/types/src/signIn.ts (1)
SignInResource(34-91)packages/types/src/signUp.ts (1)
SignUpResource(40-121)
packages/expo/src/hooks/__tests__/useAppleSignIn.test.ts (1)
packages/expo/src/hooks/useAppleSignIn.ts (1)
useAppleSignIn(57-158)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (8)
packages/expo/src/hooks/__tests__/useAppleSignIn.test.ts (3)
1-46: LGTM! Well-structured test setup.The mock setup is comprehensive and properly uses
vi.hoisted()for hoisting. All dependencies are appropriately mocked with realistic API surfaces.
48-84: LGTM! Proper test lifecycle management.The test fixtures and lifecycle hooks are well-organized. Mocks are properly reset before each test and restored after, ensuring test isolation.
86-205: LGTM! Comprehensive test coverage.The test suite thoroughly covers all critical paths:
- ✅ Happy path for existing users with session creation
- ✅ Transfer flow for new user sign-up
- ✅ Graceful handling of user cancellation
- ✅ Error cases (unavailable authentication, missing token)
- ✅ Early return when Clerk is not loaded
All assertions properly verify function calls, parameters, and return values.
packages/expo/src/hooks/useAppleSignIn.ts (5)
1-18: LGTM! Clean imports and type definitions.The type definitions are well-structured and properly exported. The import of
SignUpUnsafeMetadatafrom a previous review has been correctly addressed.
20-56: LGTM! Excellent documentation.The JSDoc is comprehensive and follows best practices. The example is clear and demonstrates the complete flow including session activation.
57-78: LGTM! Proper hook initialization and guards.The hook correctly uses Clerk hooks at the top level and implements appropriate guards for platform compatibility and loaded state. The error message for non-iOS platforms helpfully suggests an alternative approach.
80-137: LGTM! Authentication flow correctly implemented.The flow properly handles:
- Device capability checking
- Cryptographically secure nonce generation (addressed from previous review)
- Identity token validation
- Transfer flow for new users vs. existing user sign-in
The
as anytype assertions on lines 109 and 111 are acceptable given the inline comment explaining they're temporary until the newoauth_token_applestrategy type propagates through the type system.
155-158: LGTM! Clean hook interface.The hook properly returns an object with the
startAppleSignInFlowfunction, following React hook conventions.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
integration/models/application.ts (1)
108-114: This issue was already flagged in a previous review.The exit code logic and missing return type annotation have already been identified in a previous review comment. Please address the feedback provided there.
🧹 Nitpick comments (1)
integration/models/application.ts (1)
116-120: Consider extracting magic numbers to named constants.The timeout values are hardcoded. While the inline comments are helpful, extracting these to named constants would improve maintainability and make them easier to adjust if needed.
+ const DEV_SERVER_STARTUP_TIMEOUT_MS = 60000; + const DEV_SERVER_POLL_INTERVAL_MS = 1000; + - // Use a finite timeout to avoid hanging forever - const maxWaitTimeMs = 60000; // 60 seconds - const pollIntervalMs = 1000; - const maxAttempts = Math.floor(maxWaitTimeMs / pollIntervalMs); + // Use a finite timeout to avoid hanging forever + const maxAttempts = Math.floor(DEV_SERVER_STARTUP_TIMEOUT_MS / DEV_SERVER_POLL_INTERVAL_MS);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
integration/models/application.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
integration/models/application.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
integration/models/application.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
integration/models/application.ts
integration/**
📄 CodeRabbit inference engine (.cursor/rules/global.mdc)
Framework integration templates and E2E tests should be placed under the integration/ directory
Files:
integration/models/application.ts
integration/**/*
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
End-to-end tests and integration templates must be located in the 'integration/' directory.
Files:
integration/models/application.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
integration/models/application.ts
🧬 Code graph analysis (1)
integration/models/application.ts (1)
integration/models/applicationConfig.ts (1)
name(133-135)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (27)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Static analysis
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
integration/models/application.ts (1)
121-146: Good error handling structure for server startup.The finite timeout-based startup with detailed error logging and log dumping on failure provides good observability for debugging startup issues. The structure correctly handles both detached and non-detached modes.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
integration/models/application.ts (2)
88-126: Extract duplicated log-dumping logic and use proper error types.The log-dumping code is repeated multiple times (early stdout, early stderr, late stdout, late stderr, and again on failure below) with untyped catch blocks using
e.messagedirectly, violating DRY and TypeScript coding guidelines.Extract into a reusable helper:
+ const dumpLogFile = async (filePath: string, label: string): Promise<void> => { + try { + const content = await fs.readFile(filePath, 'utf-8'); + console.log(`[${name}:dev] ===== ${label} (${content.length} bytes) =====`); + console.log(content || '(empty)'); + console.log(`[${name}:dev] ===== END ${label} =====`); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + console.log(`[${name}:dev] Could not read ${label}: ${message}`); + } + };Then replace the repeated blocks:
- // Dump logs early to help diagnose startup issues - try { - const stdoutContent = await fs.readFile(stdoutFilePath, 'utf-8'); - console.log(`[${name}:dev] ===== EARLY STDOUT @5s (${stdoutContent.length} bytes) =====`); - console.log(stdoutContent || '(empty)'); - console.log(`[${name}:dev] ===== END EARLY STDOUT =====`); - } catch (e) { - console.log(`[${name}:dev] No stdout yet: ${e.message}`); - } - - try { - const stderrContent = await fs.readFile(stderrFilePath, 'utf-8'); - console.log(`[${name}:dev] ===== EARLY STDERR @5s (${stderrContent.length} bytes) =====`); - console.log(stderrContent || '(empty)'); - console.log(`[${name}:dev] ===== END EARLY STDERR =====`); - } catch (e) { - console.log(`[${name}:dev] No stderr yet: ${e.message}`); - } + await dumpLogFile(stdoutFilePath, 'EARLY STDOUT @5s'); + await dumpLogFile(stderrFilePath, 'EARLY STDERR @5s');Apply similar changes to the late dumps (lines 109-125) and failure dumps (lines 149-164).
Based on coding guidelines.
129-135: Fix exit code logic and add explicit return type.Two critical issues:
Exit code logic: The condition
!!proc.exitCode && proc.exitCode !== 0only detects non-zero exits. If the dev server exits with code 0 before being ready, this returnsfalseand continues waiting until timeout—treating a premature successful exit as if the process is still running.Missing return type: Violates TypeScript coding guidelines requiring explicit return types for functions.
Apply this diff:
- const shouldExit = () => { - const hasExited = !!proc.exitCode && proc.exitCode !== 0; + const shouldExit = (): boolean => { + const hasExited = proc.exitCode != null; if (hasExited) { console.log(`[${name}:dev] Process ${proc.pid} exited with code ${proc.exitCode}`); } return hasExited; };This treats any process exit (zero or non-zero) before server readiness as an error condition.
Based on coding guidelines.
🧹 Nitpick comments (1)
integration/models/application.ts (1)
137-143: Consider adding a clarifying comment for the timeout strategy.The finite timeout configuration is clear, but a brief comment explaining the strategy would improve maintainability.
+ // Wait up to 60 seconds for server to become ready, checking every second const maxWaitTimeMs = 60000; // 60 seconds const pollIntervalMs = 1000; const maxAttempts = Math.floor(maxWaitTimeMs / pollIntervalMs);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
integration/models/application.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
integration/models/application.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
integration/models/application.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
integration/models/application.ts
integration/**
📄 CodeRabbit inference engine (.cursor/rules/global.mdc)
Framework integration templates and E2E tests should be placed under the integration/ directory
Files:
integration/models/application.ts
integration/**/*
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
End-to-end tests and integration templates must be located in the 'integration/' directory.
Files:
integration/models/application.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
integration/models/application.ts
🧬 Code graph analysis (1)
integration/models/application.ts (1)
integration/models/applicationConfig.ts (1)
name(133-135)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Static analysis
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/expo/src/hooks/useAppleSignIn.ts (1)
7-9: CRITICAL: Missing import for SignUpUnsafeMetadata.
SignUpUnsafeMetadatais used in the type definition but is not imported. This will cause a TypeScript compilation error.Apply this diff to add the missing import:
-import type { SetActive, SignInResource, SignUpResource } from '@clerk/types'; +import type { SetActive, SignInResource, SignUpResource, SignUpUnsafeMetadata } from '@clerk/types';
🧹 Nitpick comments (1)
packages/expo/src/hooks/useAppleSignIn.ts (1)
141-155: Consider a type guard for error handling.The error type narrowing is functional but could be more maintainable with a type guard.
// Add before the function type AppleAuthError = { code: string; message?: string; }; function isAppleAuthError(error: unknown): error is AppleAuthError { return typeof error === 'object' && error !== null && 'code' in error; } // Then in the catch block: if (isAppleAuthError(error) && error.code === 'ERR_REQUEST_CANCELED') { // User canceled the sign-in flow return { createdSessionId: null, setActive, signIn, signUp, }; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/expo/src/hooks/useAppleSignIn.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/expo/src/hooks/useAppleSignIn.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/expo/src/hooks/useAppleSignIn.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/expo/src/hooks/useAppleSignIn.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/expo/src/hooks/useAppleSignIn.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/expo/src/hooks/useAppleSignIn.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/expo/src/hooks/useAppleSignIn.ts
🧬 Code graph analysis (1)
packages/expo/src/hooks/useAppleSignIn.ts (3)
packages/types/src/clerk.ts (1)
SetActive(1268-1268)packages/types/src/signIn.ts (1)
SignInResource(34-91)packages/types/src/signUp.ts (1)
SignUpResource(40-121)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
- GitHub Check: Static analysis
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (4)
packages/expo/src/hooks/useAppleSignIn.ts (4)
18-54: Excellent documentation.The JSDoc is comprehensive, includes a working example, and clearly states platform restrictions and dependencies.
69-76: Consider the early return behavior.When Clerk hasn't loaded, the function returns potentially undefined
signIn,signUp, andsetActivevalues. While type-safe (these are optional in the return type), callers should be aware they may receive undefined references rather than initialized objects.
78-86: Good lazy loading pattern.Dynamically importing
expo-apple-authenticationprevents issues on non-iOS platforms and web. The availability check is appropriate.
87-107: Secure nonce generation and proper token extraction.Using
Crypto.randomUUID()ensures cryptographically secure nonce generation. The identity token validation is appropriate.
| // Create a SignIn with the Apple ID token strategy | ||
| // Note: Type assertions needed until @clerk/clerk-react propagates the new oauth_token_apple strategy type | ||
| await signIn.create({ | ||
| strategy: 'oauth_token_apple' as any, | ||
| token: identityToken, | ||
| } as any); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Improve type safety for the new oauth_token_apple strategy.
The double as any assertions bypass all type checking. While the comment explains this is temporary, consider creating a more precise type assertion or augmenting the module locally.
Consider this alternative that maintains some type safety:
- // Create a SignIn with the Apple ID token strategy
- // Note: Type assertions needed until @clerk/clerk-react propagates the new oauth_token_apple strategy type
- await signIn.create({
- strategy: 'oauth_token_apple' as any,
- token: identityToken,
- } as any);
+ // Create a SignIn with the Apple ID token strategy
+ // Note: Type assertion needed until @clerk/clerk-react propagates the new oauth_token_apple strategy type
+ await signIn.create({
+ strategy: 'oauth_token_apple',
+ token: identityToken,
+ } as Parameters<typeof signIn.create>[0]);This reduces the scope of the type assertion while still allowing the new strategy.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/expo/src/hooks/useAppleSignIn.ts around lines 109 to 115, the code
uses broad "as any" casts for the signIn.create call which disables type
checking; replace the double any with a minimal, precise type assertion or local
type augmentation: define a narrow interface/type that extends the existing
Clerk sign-in params to include strategy: 'oauth_token_apple' and token: string
(or optional fields as needed), then cast only the object literal to that new
type when calling signIn.create; alternatively add a module augmentation for
@clerk/clerk-react to add the 'oauth_token_apple' strategy to the existing type
definitions and remove the any casts entirely.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/expo/src/hooks/useAppleSignIn.ts(1 hunks)packages/expo/src/hooks/useAppleSignIn.web.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/expo/src/hooks/useAppleSignIn.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/expo/src/hooks/useAppleSignIn.web.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/expo/src/hooks/useAppleSignIn.web.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/expo/src/hooks/useAppleSignIn.web.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/expo/src/hooks/useAppleSignIn.web.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/expo/src/hooks/useAppleSignIn.web.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/expo/src/hooks/useAppleSignIn.web.ts
🧬 Code graph analysis (1)
packages/expo/src/hooks/useAppleSignIn.web.ts (4)
packages/expo/src/hooks/useAppleSignIn.ts (3)
StartAppleSignInFlowParams(8-10)StartAppleSignInFlowReturnType(12-17)useAppleSignIn(56-159)packages/types/src/clerk.ts (1)
SetActive(1268-1268)packages/types/src/signIn.ts (1)
SignInResource(34-91)packages/types/src/signUp.ts (1)
SignUpResource(40-121)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Static analysis
- GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
packages/expo/src/hooks/useAppleSignIn.web.ts (2)
5-14: LGTM: Type definitions are well-structured.The type definitions correctly mirror the native implementation's API surface, ensuring consistency across platforms. The use of optional properties and nullable types appropriately models the return scenarios.
16-51: LGTM: Excellent documentation with actionable guidance.The JSDoc provides comprehensive guidance with a practical example showing developers the correct OAuth-based alternative for web platforms. The platform tag and error path are clearly documented.
…include parameters for startAppleSignInFlow
…logging for CI diagnostics
…ax attempts fix(longRunningApplication): remove redundant logging during initialization
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
integration/templates/expo-web/metro.config.js (1)
37-45: Remove duplicateconfig.watchFoldersassignment.Lines 37 and 40 both assign
config.watchFolders = [clerkMonorepoPath]. This issue was previously flagged and marked as addressed in commit 676621b, but the duplicate still exists. Remove line 37 to eliminate the redundancy.Apply this diff:
// Only customize Metro config when running from monorepo if (clerkMonorepoPath) { console.log('[Metro Config] Applying monorepo customizations'); - config.watchFolders = [clerkMonorepoPath]; // Disable file watching to prevent infinite reload loops in integration tests config.watchFolders = [clerkMonorepoPath]; config.watcher = {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
integration/templates/expo-web/metro.config.js(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
integration/templates/expo-web/metro.config.js
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
integration/templates/expo-web/metro.config.js
integration/**
📄 CodeRabbit inference engine (.cursor/rules/global.mdc)
Framework integration templates and E2E tests should be placed under the integration/ directory
Files:
integration/templates/expo-web/metro.config.js
integration/**/*
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
End-to-end tests and integration templates must be located in the 'integration/' directory.
Files:
integration/templates/expo-web/metro.config.js
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
integration/templates/expo-web/metro.config.js
🪛 Biome (2.1.2)
integration/templates/expo-web/metro.config.js
[error] 8-8: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (5)
integration/templates/expo-web/metro.config.js (5)
27-29: LGTM! Support for link: protocol added.The addition of
link:protocol handling is consistent with the existingfile:protocol handling and properly supports pnpm workspaces.
47-57: LGTM! Node module resolution properly configured.The prioritization of local
node_modulesover monoreponode_modulesand the explicit package mappings are correctly configured to handle the monorepo setup.
59-86: LGTM! Comprehensive blockList prevents duplicate React versions.The blockList implementation is thorough and well-structured. Proper path escaping, word boundaries, and coverage of all potential duplicate locations (monorepo root, pnpm store, package node_modules) ensure React/React-Native version conflicts are prevented.
88-118: LGTM! Custom resolver correctly handles subpath exports.The custom resolver implementation properly handles
@clerkpackage subpath exports (e.g.,@clerk/clerk-react/internal) by:
- Matching the pattern and extracting package name and subpath
- Attempting resolution via the subpath directory's
package.jsonmain field- Gracefully falling back to default resolution when subpath directories don't exist
- Preserving the original resolver if it exists
The broad catch block on line 107 is acceptable here since the fallback behavior is appropriate for any resolution failure.
121-121: LGTM! Direct export is appropriate.Exporting
configdirectly (instead of spreading it) is the correct approach since the config object is mutated in place throughout the file.
| startAppleSignInFlowParams?: StartAppleSignInFlowParams, | ||
| ): Promise<StartAppleSignInFlowReturnType> { | ||
| // Check platform compatibility | ||
| if (Platform.OS !== 'ios') { |
There was a problem hiding this comment.
Does it make sense to rename this file to useAppleSignIn.ios.ts and make useAppleSignIn.ts throw an error when it's imported?
There was a problem hiding this comment.
I think that would make sense, and I believe that would eliminate the need for the useAppleSignIn.web.ts stub. I'll give that a try.
| try { | ||
| // Generate a cryptographically secure nonce for the Apple Sign-In request (required by Clerk) | ||
| // Lazy load expo-crypto to avoid import issues on web | ||
| const Crypto = await import('expo-crypto'); |
There was a problem hiding this comment.
stuff like this is why I think it might be better to write this as a platform-specific file
| * | ||
| * @returns An object containing the `startAppleSignInFlow` function | ||
| */ | ||
| export function useAppleSignIn() { |
There was a problem hiding this comment.
what do you think about renaming this to useAppleAuthentication? Would be easier to make it work for dedicated sign-ups (outside of the sign-in to sign-up transfer that it already supports).
There was a problem hiding this comment.
Actually after reading more about this feature, I think the best name is likely useSignInWithApple since "Sign in with Apple" is the official name of the platform-level feature.
| } | ||
|
|
||
| // Create a SignIn with the Apple ID token strategy | ||
| // Note: Type assertions needed until @clerk/clerk-react propagates the new oauth_token_apple strategy type |
There was a problem hiding this comment.
Since you updated the types package we don't need the as any cast anymore right?
3 participants
Description
useAppleSignInhook for native Apple Sign-In on iOS using expo-apple-authentication.Fixes: https://linear.app/clerk/project/native-apple-login-flow-in-expo-f94b4842afd2
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Docs PR: Chris/mobile 279 expo sign in with apple clerk-docs#2732
Expo Quickstart PR: Chris/mobile 279 expo sign in with apple clerk-expo-quickstart#15
Type of change
Summary by CodeRabbit
New Features
Improvements
Chores