feat(expo): add experimental useSSO with new hooks - #9103
Conversation
🦋 Changeset detectedLatest commit: e7a9719 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.
|
📝 WalkthroughWalkthroughIntroduces an experimental Expo Changes
Estimated code review effort: 4 (Complex) | ~40 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-google-signin
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
API Changes Report
Summary
@clerk/expoCurrent version: 4.1.0 Subpath
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/expo/src/hooks/useSSO.experimental.ts (2)
14-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPublic API types lack JSDoc.
StartSSOFlowParamsandStartSSOFlowReturnTypeare exported and re-exported throughexperimental.ts, making them part of the generated reference docs surface. Neither has JSDoc describing fields likeredirectUrl,authSessionOptions,createdSessionId, orauthSessionResult.As per path instructions, "If a PR adds or changes public/reference-facing API surface area, check whether the corresponding JSDoc is present, accurate, and aligned with the implementation." Since this is new reference-facing surface, the Docs team may need to review the addition.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/expo/src/hooks/useSSO.experimental.ts` around lines 14 - 34, The exported public API types in useSSO.experimental.ts are missing JSDoc, so add doc comments for StartSSOFlowParams and StartSSOFlowReturnType and their documented fields to match the reference-facing surface. Make sure the descriptions cover the optional inputs like redirectUrl, unsafeMetadata, authSessionOptions, and the returned values such as createdSessionId and authSessionResult, and keep the docs aligned with the behavior of useSSO so the generated reference remains accurate.Source: Path instructions
58-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
useSSOis missing an explicit return type and JSDoc.As per coding guidelines, "Always define explicit return types for functions, especially public APIs" and "Document functions with JSDoc comments including
@param,@returns,@throws, and@exampletags."useSSOis the primary exported hook for this feature and has neither.♻️ Suggested fix
+export interface UseSSOReturn { + startSSOFlow: (startSSOFlowParams: StartSSOFlowParams) => Promise<StartSSOFlowReturnType>; +} + +/** + * Hook to initiate an SSO (OAuth or Enterprise SSO) flow in Expo apps. + * `@example` + * const { startSSOFlow } = useSSO(); + */ -export function useSSO() { +export function useSSO(): UseSSOReturn {Since this is also a reference-facing export via
experimental.ts, the added JSDoc will likely surface in generated docs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/expo/src/hooks/useSSO.experimental.ts` around lines 58 - 63, The exported useSSO hook lacks an explicit return type and JSDoc, so update the useSSO declaration to name its return type explicitly and add a docblock above it. Document the hook with `@param` for any inputs it exposes through startSSOFlow, `@returns` for the hook API it provides, `@throws` for expected failures in the SSO flow, and `@example` for typical usage. Keep the documentation attached to useSSO so it also benefits the experimental export surface.Sources: Coding guidelines, Path instructions
packages/expo/src/hooks/ssoDependencies.ts (1)
16-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo direct test coverage for the actual
require()failure path.All tests in
useSSO.experimental.test.tsfully mockloadSSODependencies, so this file's real try/catch branch (and its install-guidance error message) is never executed by the test suite.♻️ Suggested test addition
// packages/expo/src/hooks/__tests__/ssoDependencies.test.ts vi.mock('expo-auth-session', () => { throw new Error('Cannot find module'); }); test('throws install guidance when optional deps are missing', () => { expect(() => loadSSODependencies()).toThrow(/npx expo install expo-auth-session expo-web-browser/); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/expo/src/hooks/ssoDependencies.ts` around lines 16 - 27, Add direct test coverage for the real failure path in loadSSODependencies, since useSSO.experimental.test.ts only mocks it and never exercises the try/catch. Create a test around the actual loadSSODependencies function in ssoDependencies.test.ts that makes require('expo-auth-session') or require('expo-web-browser') throw, then assert the thrown message includes the install guidance text. This should validate the catch block’s behavior and the error message for missing optional SSO dependencies.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/expo/src/hooks/ssoDependencies.ts`:
- Around line 16-27: Add direct test coverage for the real failure path in
loadSSODependencies, since useSSO.experimental.test.ts only mocks it and never
exercises the try/catch. Create a test around the actual loadSSODependencies
function in ssoDependencies.test.ts that makes require('expo-auth-session') or
require('expo-web-browser') throw, then assert the thrown message includes the
install guidance text. This should validate the catch block’s behavior and the
error message for missing optional SSO dependencies.
In `@packages/expo/src/hooks/useSSO.experimental.ts`:
- Around line 14-34: The exported public API types in useSSO.experimental.ts are
missing JSDoc, so add doc comments for StartSSOFlowParams and
StartSSOFlowReturnType and their documented fields to match the reference-facing
surface. Make sure the descriptions cover the optional inputs like redirectUrl,
unsafeMetadata, authSessionOptions, and the returned values such as
createdSessionId and authSessionResult, and keep the docs aligned with the
behavior of useSSO so the generated reference remains accurate.
- Around line 58-63: The exported useSSO hook lacks an explicit return type and
JSDoc, so update the useSSO declaration to name its return type explicitly and
add a docblock above it. Document the hook with `@param` for any inputs it exposes
through startSSOFlow, `@returns` for the hook API it provides, `@throws` for
expected failures in the SSO flow, and `@example` for typical usage. Keep the
documentation attached to useSSO so it also benefits the experimental export
surface.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 67a83ff0-5090-473c-9bf4-b389a5c17703
📒 Files selected for processing (4)
packages/expo/src/experimental.tspackages/expo/src/hooks/__tests__/useSSO.experimental.test.tspackages/expo/src/hooks/ssoDependencies.tspackages/expo/src/hooks/useSSO.experimental.ts
…erimental-to-use-new-hooks
wobsoriano
left a comment
wobsoriano
left a comment
There was a problem hiding this comment.
Looking good with minor changes!
Co-authored-by: Robert Soriano <sorianorobertc@gmail.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/expo/src/hooks/useSSO.experimental.ts (2)
118-120: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject successful callbacks without a rotating token nonce.
The
?? ''fallback turns an invalid callback intoreload({ rotatingTokenNonce: '' }), causing an unnecessary API call and an opaque failure. Validate the nonce before reloading and tell developers to restart the SSO flow or verify the callback configuration.Proposed fix
const params = new URL(authSessionResult.url).searchParams; -const rotatingTokenNonce = params.get('rotating_token_nonce') ?? ''; +const rotatingTokenNonce = params.get('rotating_token_nonce'); +if (!rotatingTokenNonce) { + return errorThrower.throw( + 'SSO callback is missing rotating_token_nonce. Restart the SSO flow and verify the redirect URL configuration.', + ); +} await client.signIn.reload({ rotatingTokenNonce });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/expo/src/hooks/useSSO.experimental.ts` around lines 118 - 120, Update the callback handling around rotatingTokenNonce in the SSO flow to reject successful callbacks when the URL lacks a rotating token nonce, rather than defaulting to an empty string. Before calling client.signIn.reload, validate the nonce and surface an actionable error instructing developers to restart the SSO flow or verify the callback configuration.
14-34: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd explicit typing and JSDoc for the public hook API.
useSSOis exported without a return annotation, and the new public types/API lack the required JSDoc (@param,@returns,@throws, and@example). Add a named hook return type and document the returnedstartSSOFlowcontract.As per coding guidelines, public TypeScript APIs require explicit return types and comprehensive JSDoc. Based on learnings, exported/public functions in this repository should retain explicit return annotations.
Also applies to: 58-63
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/expo/src/hooks/useSSO.experimental.ts` around lines 14 - 34, Update the exported useSSO hook with an explicit named return type, and add comprehensive JSDoc for the public hook and returned startSSOFlow contract. Document parameters, return values, possible throws, and include an example, using the existing StartSSOFlowParams and StartSSOFlowReturnType symbols rather than duplicating types.Sources: Coding guidelines, Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/expo/src/hooks/useSSO.experimental.ts`:
- Around line 118-120: Update the callback handling around rotatingTokenNonce in
the SSO flow to reject successful callbacks when the URL lacks a rotating token
nonce, rather than defaulting to an empty string. Before calling
client.signIn.reload, validate the nonce and surface an actionable error
instructing developers to restart the SSO flow or verify the callback
configuration.
- Around line 14-34: Update the exported useSSO hook with an explicit named
return type, and add comprehensive JSDoc for the public hook and returned
startSSOFlow contract. Document parameters, return values, possible throws, and
include an example, using the existing StartSSOFlowParams and
StartSSOFlowReturnType symbols rather than duplicating types.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: fec0cc2c-6143-4657-b376-40053ffb4cbf
📒 Files selected for processing (1)
packages/expo/src/hooks/useSSO.experimental.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/expo/src/hooks/__tests__/ssoDependencies.test.ts`:
- Around line 21-31: Ensure the module-loader spy created in the
loadSSODependencies test is always restored, including when the assertion fails.
Wrap the expect assertion in a try/finally block and call loadSpy.mockRestore()
in the finally clause.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 8a93c7f4-af7d-4aa2-8e3e-3989d30175a5
📒 Files selected for processing (4)
.changeset/bright-taxis-sing.mdpackages/expo/src/hooks/__tests__/ssoDependencies.test.tspackages/expo/src/hooks/__tests__/useSSO.experimental.test.tspackages/expo/src/hooks/useSSO.experimental.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/expo/src/hooks/tests/useSSO.experimental.test.ts
| test('throws install guidance when an optional dependency cannot be loaded', () => { | ||
| const loadSpy = vi.spyOn(moduleWithLoad, '_load').mockImplementation((request, parent, isMain) => { | ||
| if (request === 'expo-auth-session') { | ||
| throw new Error('Cannot find module expo-auth-session'); | ||
| } | ||
|
|
||
| return originalModuleLoad(request, parent, isMain); | ||
| }); | ||
|
|
||
| expect(() => loadSSODependencies()).toThrow(/npx expo install expo-auth-session expo-web-browser/); | ||
| loadSpy.mockRestore(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Always restore the module-loader spy.
If Line 30 fails, Line 31 is skipped and _load remains mocked for subsequent tests. Use try/finally around the assertion.
Proposed fix
- expect(() => loadSSODependencies()).toThrow(/npx expo install expo-auth-session expo-web-browser/);
- loadSpy.mockRestore();
+ try {
+ expect(() => loadSSODependencies()).toThrow(/npx expo install expo-auth-session expo-web-browser/);
+ } finally {
+ loadSpy.mockRestore();
+ }📝 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.
| test('throws install guidance when an optional dependency cannot be loaded', () => { | |
| const loadSpy = vi.spyOn(moduleWithLoad, '_load').mockImplementation((request, parent, isMain) => { | |
| if (request === 'expo-auth-session') { | |
| throw new Error('Cannot find module expo-auth-session'); | |
| } | |
| return originalModuleLoad(request, parent, isMain); | |
| }); | |
| expect(() => loadSSODependencies()).toThrow(/npx expo install expo-auth-session expo-web-browser/); | |
| loadSpy.mockRestore(); | |
| test('throws install guidance when an optional dependency cannot be loaded', () => { | |
| const loadSpy = vi.spyOn(moduleWithLoad, '_load').mockImplementation((request, parent, isMain) => { | |
| if (request === 'expo-auth-session') { | |
| throw new Error('Cannot find module expo-auth-session'); | |
| } | |
| return originalModuleLoad(request, parent, isMain); | |
| }); | |
| try { | |
| expect(() => loadSSODependencies()).toThrow(/npx expo install expo-auth-session expo-web-browser/); | |
| } finally { | |
| loadSpy.mockRestore(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/expo/src/hooks/__tests__/ssoDependencies.test.ts` around lines 21 -
31, Ensure the module-loader spy created in the loadSSODependencies test is
always restored, including when the assertion fails. Wrap the expect assertion
in a try/finally block and call loadSpy.mockRestore() in the finally clause.
2 participants
Summary
useSSOexport for@clerk/expo/experimentalthat uses the newuseClerk,useSignIn, anduseSignUphooks.Linear: MOBILE-582
Testing
corepack pnpm --filter @clerk/expo exec vitest run src/hooks/__tests__/useSSO.experimental.test.ts src/hooks/__tests__/useSSO.test.tscorepack pnpm --filter @clerk/expo exec eslint src/experimental.ts src/hooks/useSSO.experimental.ts src/hooks/ssoDependencies.ts src/hooks/__tests__/useSSO.experimental.test.tscorepack pnpm --filter @clerk/expo format:checkhttps://pkg.pr.new/@clerk/expo@9103: Google SSO from the native development build returned to the app and signed in successfully.Notes
build:declarationswas blocked by workspace dependency resolution / pnpm non-interactive module purge behavior in this shell, before reaching this change.