fix(clerk-expo): Resolve module loading issue for optional Sign in with Apple #7171
Conversation
🦋 Changeset detectedLatest commit: d2a2314 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.
|
WalkthroughConverts static imports of Changes
Sequence Diagram(s)sequenceDiagram
participant App as App / Consumer
participant Hook as useSignInWithApple
participant Modules as Runtime Modules
Note over Hook,Modules: New behavior — imports deferred until hook call
App->>Hook: invoke signInWithApple()
Hook->>Modules: dynamic import('expo-apple-authentication')\nand import('expo-crypto')
alt import succeeds
Hook->>Modules: AppleAuthentication.isAvailableAsync()
alt available
Hook->>Modules: perform cryptographic steps (Crypto)
Hook->>App: return credential / success
else not available
Hook->>App: return availability error
end
else import fails
Hook->>App: throw/return error indicating modules must be installed
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 (1)
packages/expo/src/hooks/useSignInWithApple.ios.ts (1)
76-81: Consider logging the import error for debugging.The caught error parameter is unused. While the error message is clear and actionable, logging the underlying error could help diagnose module loading failures (e.g., version mismatches, installation issues).
} catch (error) { + console.error('Failed to load Apple authentication modules:', error); return errorThrower.throw(
📜 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 (3)
.changeset/fix-expo-apple-auth-optional.md(1 hunks)packages/expo/package.json(1 hunks)packages/expo/src/hooks/useSignInWithApple.ios.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
.changeset/**
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/fix-expo-apple-auth-optional.md
**/*.{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/useSignInWithApple.ios.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/useSignInWithApple.ios.tspackages/expo/package.json
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/expo/src/hooks/useSignInWithApple.ios.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/useSignInWithApple.ios.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/useSignInWithApple.ios.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/useSignInWithApple.ios.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
⏰ 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: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
packages/expo/package.json (1)
3-3: LGTM! Appropriate patch version bump.The patch-level version increment is appropriate for this bug fix that makes optional dependencies truly optional at bundle time.
.changeset/fix-expo-apple-auth-optional.md (1)
1-5: LGTM! Changeset accurately describes the fix.The changeset correctly documents the patch-level change and provides a clear explanation of the problem and solution.
packages/expo/src/hooks/useSignInWithApple.ios.ts (1)
83-154: LGTM! Dynamic import approach successfully resolves the bundling issue.The implementation correctly defers module loading until the hook is actually invoked, making expo-apple-authentication and expo-crypto truly optional at bundle time. The existing authentication flow logic remains sound, and the dynamically imported modules are used correctly throughout.
| // Dynamically import expo-apple-authentication only when needed | ||
| let AppleAuthentication; | ||
| let Crypto; | ||
|
|
||
| try { | ||
| [AppleAuthentication, Crypto] = await Promise.all([import('expo-apple-authentication'), import('expo-crypto')]); | ||
| } catch (error) { | ||
| return errorThrower.throw( | ||
| 'expo-apple-authentication is required to use Sign in with Apple. ' + | ||
| 'Please install it by running: npx expo install expo-apple-authentication expo-crypto', | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add explicit type annotations for dynamically imported modules.
The variables AppleAuthentication and Crypto lack explicit type annotations, which violates the TypeScript coding guidelines. Dynamic imports return module namespace objects that have specific types.
Apply this diff to add explicit type annotations:
// Dynamically import expo-apple-authentication only when needed
-let AppleAuthentication;
-let Crypto;
+let AppleAuthentication: typeof import('expo-apple-authentication');
+let Crypto: typeof import('expo-crypto');
try {
[AppleAuthentication, Crypto] = await Promise.all([import('expo-apple-authentication'), import('expo-crypto')]);As per coding guidelines: "Always define explicit return types for functions, especially public APIs" and "Avoid any type - prefer unknown when type is uncertain, then narrow with type guards."
📝 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.
| // Dynamically import expo-apple-authentication only when needed | |
| let AppleAuthentication; | |
| let Crypto; | |
| try { | |
| [AppleAuthentication, Crypto] = await Promise.all([import('expo-apple-authentication'), import('expo-crypto')]); | |
| } catch (error) { | |
| return errorThrower.throw( | |
| 'expo-apple-authentication is required to use Sign in with Apple. ' + | |
| 'Please install it by running: npx expo install expo-apple-authentication expo-crypto', | |
| ); | |
| } | |
| // Dynamically import expo-apple-authentication only when needed | |
| let AppleAuthentication: typeof import('expo-apple-authentication'); | |
| let Crypto: typeof import('expo-crypto'); | |
| try { | |
| [AppleAuthentication, Crypto] = await Promise.all([import('expo-apple-authentication'), import('expo-crypto')]); | |
| } catch (error) { | |
| return errorThrower.throw( | |
| 'expo-apple-authentication is required to use Sign in with Apple. ' + | |
| 'Please install it by running: npx expo install expo-apple-authentication expo-crypto', | |
| ); | |
| } |
🤖 Prompt for AI Agents
In packages/expo/src/hooks/useSignInWithApple.ios.ts around lines 70 to 82, the
dynamically imported variables AppleAuthentication and Crypto lack explicit
TypeScript annotations; add explicit types such as declaring
AppleAuthentication: typeof import('expo-apple-authentication') | unknown and
Crypto: typeof import('expo-crypto') | unknown (or use unknown first and then
narrow/cast with type guards) to avoid implicit any, then narrow before use (or
import the module types and annotate directly) so the code follows the guideline
of explicit types and avoids any.
There was a problem hiding this comment.
Coderrabbit, trix are for kids... These type annotations are not allowed in our codebase according to the linting rules:
import() type annotations are forbidden.eslint@typescript-eslint/consistent-type-imports
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@typescript-eslint/consistent-type-imports dynamic import typing patterns
💡 Result:
Short summary + patterns and how the rule affects them:
- Rule basics
- @typescript-eslint/consistent-type-imports enforces using type-only imports (or not) and has options prefer, fixStyle (inline-type-imports | separate-type-imports) and disallowTypeAnnotations. [1]
- Preferred patterns for dynamic imports (typing runtime import() results)
- Value import (runtime): use const mod = await import('./x'); then use mod.foo (no type-only import involved).
- Type for module shape (recommended, separate-style):
- import type { Foo } from './x'; type T = Foo; — works with the rule when prefer: 'type-imports' + fixStyle:'separate-type-imports'. [1]
- Inline type imports (TypeScript ≥4.5) — single import can mix values and type names:
- import { func, type TypeName } from './x'; — controlled by fixStyle:'inline-type-imports'. [1][3]
- Typing the dynamic import expression (type-only import() vs typeof import()):
- type OnlyFromImportType = import('./x').Foo; // this is an inline type import (import() in a type position)
- type ModuleType = typeof import('./x'); // gets the runtime module type (useful for describing default/exports)
Note: consistent-type-imports has option disallowTypeAnnotations which, when true, forbids type positions using import(...) syntax — so import('./x').T may be flagged. If you want to allow import() types, set disallowTypeAnnotations: false. [2]
- Practical patterns depending on rule config
- If prefer: 'type-imports' + disallowTypeAnnotations: true (default):
- Use import type { T } from './x' or inline type modifiers (import { type T } ...) rather than type import('./x').T. [1][2]
- If you need the module's runtime type (e.g., to type a wrapper around dynamic import):
- Use type Mod = typeof import('./x'); const m = await import('./x') as Promise; or cast the awaited result: const m = (await import('./x')) as unknown as Mod.
- Or keep value-level import and derive types with import type { Foo } from './x' and use Foo for typing values returned (if exports are types/values).
- Recommendations
- Prefer import type / import { type X } for named types (obeying fixStyle choice).
- If you must use import('./x').T, set disallowTypeAnnotations: false in the rule (but be aware many projects prefer explicit import type forms). [1][2]
- For typing dynamic import results, favor typeof import('./x') when you need the module shape at the type level; otherwise import type { T } is clearer.
Sources
[1] @typescript-eslint/consistent-type-imports docs.
[2] Rule details / options (node docs and rule README).
[3] TypeScript import { type ... } syntax discussion (Stack Overflow).
@chriscanin, you're absolutely right—my apologies for the incorrect suggestion!
Given your ESLint rule configuration with disallowTypeAnnotations: true, the typeof import() syntax is indeed forbidden. With dynamic imports designed to make the module optional at bundle time, you're facing a tradeoff between type safety and the primary goal.
Your current approach (implicit any for the dynamically imported variables) is actually pragmatic here. The alternatives would be:
-
Use
unknownand type assertions - More verbose but maintains some type safety:let AppleAuthentication: unknown; let Crypto: unknown; // Later, use with type assertions const isAvailable = await (AppleAuthentication as any).isAvailableAsync();
-
Accept the implicit
any- Simpler and acceptable given:- The modules are used immediately after import
- The try-catch provides runtime safety
- Type errors would surface quickly during development
- The primary goal (optional bundling) is preserved
Given the constraints, I'd recommend keeping your current implementation as-is. The implicit any is a reasonable tradeoff when the linting rules prevent proper typing of dynamic imports.
chriscanin
changed the title
@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: |
Co-authored-by: Dylan Staley <88163+dstaley@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/expo/src/hooks/useSignInWithApple.ios.ts (1)
70-82: Solid dynamic import implementation for optional dependencies.The dynamic import approach effectively resolves the bundling issue by loading
expo-apple-authenticationandexpo-cryptoonly at runtime when the hook is invoked. UsingPromise.allfor parallel imports is good for performance, and the error handling provides clear guidance to developers.The implicit
anytypes are an acceptable tradeoff given your linting constraints (as discussed in the previous review).
Optional improvement: Clarify that both packages are required.
The error message could be slightly more explicit about both dependencies being required:
return errorThrower.throw( - 'expo-apple-authentication is required to use Sign in with Apple. ' + - 'Please install it by running: npx expo install expo-apple-authentication expo-crypto', + 'expo-apple-authentication and expo-crypto are required to use Sign in with Apple. ' + + 'Please install them by running: npx expo install expo-apple-authentication expo-crypto', );
📜 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/useSignInWithApple.ios.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/useSignInWithApple.ios.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/useSignInWithApple.ios.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/expo/src/hooks/useSignInWithApple.ios.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/useSignInWithApple.ios.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/useSignInWithApple.ios.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/useSignInWithApple.ios.ts
⏰ 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 (3)
packages/expo/src/hooks/useSignInWithApple.ios.ts (3)
17-68: LGTM: Excellent documentation and setup.The JSDoc comments are comprehensive and provide a clear usage example. The function signature and initial setup (loading checks) are clean and well-structured.
84-139: LGTM: Authentication flow correctly handles all scenarios.The implementation properly:
- Checks device availability before attempting authentication
- Generates a cryptographically secure nonce using
Crypto.randomUUID()- Requests appropriate scopes (FULL_NAME, EMAIL)
- Validates the identity token
- Handles the transfer flow for new users with optional metadata
- Returns the correct session for existing users
The dynamic imports integrate seamlessly into the authentication flow.
140-154: LGTM: Robust error handling for user cancellations and errors.The error handling correctly distinguishes between:
- User-initiated cancellations (
ERR_REQUEST_CANCELED) → graceful return with null session- Actual errors → re-thrown for upstream handling
The type guard pattern for checking the error shape is defensive and appropriate.
Description
This update was made to resolve a module loading issue with users who are not using the optional Sign In With Apple hook.
In useSignInWithApple.ios.ts we had:
import * as AppleAuthentication from 'expo-apple-authentication';
This does not work because when xcode, or eas (fastlane) go to bundle the application, we expect expo-apple-authentication to be available. To make these modules optional, we now dynamically import expo-apple-authentication only when needed.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit