Uh oh!
There was an error while loading. Please reload this page.
fix(clerk-js): Use __internal_setActiveInProgress to prevent navigations - #5515
Conversation
🦋 Changeset detectedLatest commit: e03f9a2 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 Git ↗︎
|
There was a problem hiding this comment.
Pull Request Overview
This PR prevents navigations from being triggered during an in-flight call to setActive by introducing and leveraging an internal flag (__internal_setActiveInProgress) within the core Clerk class. Additionally, it fixes an issue with the SSO callback URL for the combined sign-up/sign-in flow.
- Introduces __internal_setActiveInProgress in the Clerk type and class to signal an active setActive call.
- Updates related components (SignUpContinue and SSOCallback) to check this flag before triggering navigations.
- Adjusts the SSO callback URL logic to return a custom SSO callback URL from context when available.
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/types/src/clerk.ts | Adds __internal_setActiveInProgress property with documentation. |
| packages/react/src/isomorphicClerk.ts | Updates type definition to include __internal_setActiveInProgress. |
| packages/clerk-js/src/ui/components/SignUp/SignUpContinue.tsx | Adds check for __internal_setActiveInProgress to prevent premature navigation. |
| packages/clerk-js/src/ui/common/redirects.ts | Updates buildSSOCallbackURL to return a custom URL if provided in context. |
| packages/clerk-js/src/ui/common/tests/redirects.test.ts | Adds test for custom SSO callback URL handling. |
| packages/clerk-js/src/ui/common/SSOCallback.tsx | Skips redirect callback if __internal_setActiveInProgress is true. |
| packages/clerk-js/src/core/clerk.ts | Sets and unsets __internal_setActiveInProgress around setActive operations. |
| .changeset/*.md | Documents the patch-level changes to address navigation and URL issues. |
Files not reviewed (1)
- packages/clerk-js/bundlewatch.config.json: Language not supported
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
brkalow
left a comment
There was a problem hiding this comment.
Approving if we want to get this out to fix the immediate issues, but I would like to see this integrated into our internal router instead. Feels like this is something that will likely crop up again if we don't.
Nice idea.
| handleError(e, [], card.setError); | ||
| timeoutId = setTimeout(() => void navigate('../'), 4000); | ||
| }); | ||
| if (__internal_setActiveInProgress !== true) { |
There was a problem hiding this comment.
❓ why don't we do this in our router instead?
There was a problem hiding this comment.
We can definitely explore something like that, which I feel is more intelligent than this blunt approach. It'd likely use the same flag too, so we're good there. The main reason I went with this instead of scoping this purely to preventing navigations is I'm concerned we have more functionality that needs to be prevented from executing a second time. For example, the PR as is prevents the execution of handleRedirectCallback, rather than simply preventing a navigation from it. handleRedirectCallback has several non-navigation actions that it performs that it would be best if they were prevented completely (stuff like signIn.reload, signIn.create, signUp.create, setActive, etc).
| // If the context contains an SSO callback URL, use it instead of building a new one, as it likely contains the | ||
| // combined flow path. | ||
| if ('ssoCallbackUrl' in ctx && ctx.ssoCallbackUrl) { | ||
| return ctx.ssoCallbackUrl; | ||
| } |
Description
This PR introduces a new flag on the core Clerk class,
__internal_setActiveInProgress, to indicate that a call tosetActiveis in flight. This can be used to guard against performing navigations from components that are remounted by Next.js duringsetActiveas a result of ourinvalidateCacheAction. This fixes issues where routes like/sso-callbackand/sign-up/continuewere remounted duringsetActive, which trigger navigations that can sometimes resolve beforesetActiveis complete, leading the user to the incorrect location.Additionally, this PR fixes an issue where the SSO callback URL was incorrectly set for the combined flow, which prevented users from being sent to
/sign-up/continuewhen attempting to sign up.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change