fix(expo): dismiss browser after SSO/OAuth flow to prevent stuck sessions on re-login#8126
fix(expo): dismiss browser after SSO/OAuth flow to prevent stuck sessions on re-login#8126chriscanin wants to merge 3 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: f6e817d 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis change updates the 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/expo/src/hooks/useOAuth.ts`:
- Around line 80-91: The TypeScript error is caused by destructuring url from
authSessionResult which is a discriminated union; update the logic in
useOAuth.ts around the authSessionResult returned by
WebBrowserModule.openAuthSessionAsync (and the variable authSessionResult) to
first check if authSessionResult.type === 'success' (or !== 'success' to
early-return/handle non-success) before accessing authSessionResult.url,
mirroring the pattern used in useSSO.ts; ensure you only read .url inside the
success branch and handle the other variants (cancel, dismiss, etc.) explicitly
so the compiler no longer sees an unsafe property access.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: ab97e8b2-1aa8-4a0d-9186-79f19ac3ab1b
📒 Files selected for processing (3)
.changeset/fix-sso-browser-dismissal.mdpackages/expo/src/hooks/useOAuth.tspackages/expo/src/hooks/useSSO.ts
Access authSessionResult.type and authSessionResult.url directly with proper discriminated union narrowing instead of destructuring, matching the pattern used in useSSO.ts.
|
!snapshot |
|
@brkalow this PR is possibly having issues discovered in final tests, could be a local environment issue, I am working on it. I have added a "Do not merge" tag for the moment. |
Summary
Fixes browser remaining open in the background after a successful OAuth/SSO flow on some platforms (reported by community user Erwin). When the browser isn't explicitly dismissed, subsequent sign-in attempts fail silently or the browser appears frozen.
openAuthSessionAsyncin atry/finallyin bothuseSSOanduseOAuthWebBrowserModule.dismissBrowser()unconditionally after the auth session completes, regardless of outcomeSymptoms fixed
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit