fix(desktop): stop Clerk virtual-router navigation from hard-reloading the app - #7605
fix(desktop): stop Clerk virtual-router navigation from hard-reloading the app#7605ishaanko wants to merge 2 commits into
Conversation
…g the app Electron desktop OAuth sign-in via T3 Connect dead-ends whenever sign-in needs a second step (first-time sign-up transfer, MFA, password reset): Clerk had no routerPush/routerReplace, so clerk-js fell back to a hard window navigation to a synthetic path like t3code://app/CLERK-ROUTER/VIRTUAL/sign-up#/continue. Electron's protocol handler can't resolve that, tearing down the whole React tree, including the mounted sign-in modal, and dropping the user back to signed-out. Bridge Clerk's navigation into the app's own history instead. A genuine in-app destination goes through the real TanStack Router history as a normal SPA transition. A Clerk virtual-router path (detected by the same marker clerk-js's own internals use to recognize one) is absorbed into an isolated, unmounted history, so it never reaches the app's real router.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
On browser history a "#" is a real URL fragment, not Electron's routing convention. Unwrapping it unconditionally pushed just the fragment instead of the real path. Only Electron uses hash history, so only unwrap there; browser history gets the destination unchanged.
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — Self-contained bug fix that prevents Clerk's internal virtual-router paths from hard-reloading the Electron app. The new utility absorbs Clerk's synthetic navigation paths while forwarding real paths to the app router, with comprehensive tests covering key scenarios. You can add or adjust custom eligibility rules. Learn more. |
|
[claude-fable-5] RESPONDING ON BEHALF OF ISHAAN This one is approved and all checks are green. Let me know if anything else is needed from my side before merge. |
Problem
T3 Connect sign-in on the desktop app dead-ends whenever sign-in needs a second step: a first-time sign-up transfer, an MFA factor, or a password reset. Clerk had no
routerPush/routerReplace, so clerk-js fell back to a hard window navigation to a synthetic path liket3code://app/CLERK-ROUTER/VIRTUAL/sign-up#/continue. Electron's protocol handler can't resolve that. The navigation tears down the whole React tree, including the mounted sign-in modal, and drops the user back to signed-out.Fixes #7512
Fix
Bridge Clerk's navigation into the app's own history instead of leaving it to fall back to
window.location. A genuine in-app destination goes through the real TanStack Router history as a normal SPA transition. A Clerk virtual-router path never corresponds to a real app route, so pushing one into the app's own router-bound history would just move the problem: the app's router would fail to match it and blank the whole UI behind the modal to "Not Found" instead. It's absorbed into a separate, unmounted history instead, detected by the same marker clerk-js's own internals use to recognize a virtual-router path.An earlier version of this fix tried asking the app's router whether it recognized a path, using it as the real/virtual signal. That broke against the app's real route tree: a param route (
/$environmentId/$threadId) fuzzy-matched Clerk's virtual paths as real, silently reintroducing the same bug. Checking Clerk's own marker directly avoids depending on what routes the app happens to have.Testing
vp test run apps/web/src/components/clerk/clerkRouterBridge.test.ts: 4 passed, covering a virtual path with the marker before and after a hash, a real in-app navigation, and a real Electron redirect target that embeds its path after a hash (t3code://app/#/settings, built byauthRedirect.ts) to confirm it lands on the right page instead of the app root.vp run --filter @t3tools/web typecheck: cleanModel: Claude Sonnet 5. Harness: Claude Code.
Note
Medium Risk
Touches authentication navigation on both Electron and web; incorrect path routing could break sign-in redirects or in-app navigation, though behavior is covered by focused unit tests.
Overview
Fixes desktop T3 Connect sign-in dead-ends on multi-step flows (sign-up transfer, MFA, password reset) by supplying
routerPush/routerReplaceto Clerk instead of letting clerk-js hard-navigate to unresolvablet3code://…/CLERK-ROUTER/VIRTUAL/…URLs that tear down the React tree.Adds
createClerkRouterBridge, which routes Clerk’s navigation calls: paths containing Clerk’sCLERK-ROUTER/VIRTUAL/marker go to an isolated in-memory history so the real TanStack Router history stays put (avoiding a Not Found blank behind the modal); real app paths use the app history. On Electron hash history, hash-prefixed destinations from redirects like/#/settings/…are unwrapped so routes don’t double-hash; browser history still keeps literal URL fragments.Wires the bridge into
ElectronClerkProviderandClerkProviderinmain.tsx, with unit tests for virtual paths, real navigation, Electron hash unwrapping, and browser fragments.Reviewed by Cursor Bugbot for commit cf9cb6c. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix Clerk virtual-router navigation from hard-reloading the app
createClerkRouterBridgein clerkRouterBridge.ts that producesrouterPush/routerReplacefunctions for Clerk providers.CLERK-ROUTER/VIRTUAL/marker are absorbed into an isolated in-memory history, preventing them from reaching the app router and triggering a hard reload./#prefix before forwarding to the real history; on browser histories, paths are forwarded unchanged.routerPush/routerReplaceto bothElectronClerkProviderandClerkProvider.Macroscope summarized cf9cb6c.