refactor(clerk-js): Consolidate resource events into single event - #7980

Closed
bratsos wants to merge 2 commits into
mainfrom
alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen
Closed

refactor(clerk-js): Consolidate resource events into single event#7980
bratsos wants to merge 2 commits into
mainfrom
alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen

Conversation

@bratsos

@bratsosbratsos commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

Description

Resource state changes were spread across three separate events: resource:update, resource:error, and resource:fetch. Each one triggered its own signal write in State, which meant React could re-render between them. After an API call completed, React would briefly see the updated resource while fetchStatus was still 'fetching', because the resource and fetch-status updates arrived independently.

Even after consolidating into one event, a second source of inconsistency remained: fromJSON emits a resource-only event mid-flight (during an API call), which updated the resource signal before runAsyncResourceTask emitted the completion event with fetchStatus: 'idle'.

A third source came from finalize(), which used runAsyncResourceTask and set fetchStatus: 'fetching' on an already-complete resource even though it's just calling setActive, a session-level operation.

How we fixed it

Three changes, each addressing one source of inconsistency:

  1. Consolidated three events into one.resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

  2. Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

  3. Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

Demo

(showcasing the finalize() fix)

BeforeAfter
resource-events-before.mov
resource-events-after.mov
Sample code used for the demo
exportdefaultfunctionSignInPage(){const{ signIn, errors, fetchStatus }=useSignIn()constrouter=useRouter()const[email,setEmail]=useState('')const[password,setPassword]=useState('')const[toasts,setToasts]=useState<Toast[]>([])consttoastIdRef=useRef(0)useEffect(()=>{if(fetchStatus==='fetching'){setToasts(prev=>[...prev,{id: ++toastIdRef.current,message: 'Verifying your credentials...',type: 'info'}])}if(fetchStatus==='idle'){setToasts(prev=>prev.filter(t=>t.type!=='info'))}},[fetchStatus])useEffect(()=>{if(signIn?.status==='complete'){setToasts(prev=>[...prev,{id: ++toastIdRef.current,message: 'Welcome back! Redirecting...',type: 'success'}])}},[signIn?.status])functiondismissToast(id: number){setToasts(prev=>prev.filter(t=>t.id!==id))}asyncfunctionhandleSubmit(e: React.FormEvent){e.preventDefault()if(!signIn)returnsetToasts([])toastIdRef.current=0awaitsignIn.password({identifier: email, password })if(signIn.status==='complete'){awaitsignIn.finalize({navigate: async({ session, decorateUrl })=>{if(session?.currentTask){router.push(decorateUrl(`/sign-in/tasks/${session.currentTask.key}`))return}router.push(decorateUrl('/'))},})}}return(<divclassName="flex min-h-screen items-center justify-center bg-zinc-50 dark:bg-black"><ToastContainertoasts={toasts}onDismiss={dismissToast}/><divclassName="w-full max-w-sm rounded-lg border border-zinc-200 bg-white p-8 shadow-sm dark:border-zinc-800 dark:bg-zinc-950"><h1className="mb-6 text-center text-2xl font-semibold text-zinc-900 dark:text-zinc-50">Signin</h1><formonSubmit={handleSubmit}className="flex flex-col gap-4"><divclassName="flex flex-col gap-1"><labelhtmlFor="email"className="text-sm font-medium text-zinc-700 dark:text-zinc-300">Email</label><inputid="email"type="email"value={email}onChange={(e)=>setEmail(e.target.value)}placeholder="you@example.com"className="rounded-md border border-zinc-300 px-3 py-2 text-sm text-zinc-900 placeholder:text-zinc-400 focus:border-zinc-500 focus:outline-none focus:ring-1 focus:ring-zinc-500 dark:border-zinc-700 dark:bg-zinc-900 dark:text-zinc-50"/>{errors?.fields?.identifier&&(<pclassName="text-sm text-red-600">{errors.fields.identifier.message}</p>)}</div><divclassName="flex flex-col gap-1"><labelhtmlFor="password"className="text-sm font-medium text-zinc-700 dark:text-zinc-300">Password</label><inputid="password"type="password"value={password}onChange={(e)=>setPassword(e.target.value)}placeholder="Password"className="rounded-md border border-zinc-300 px-3 py-2 text-sm text-zinc-900 placeholder:text-zinc-400 focus:border-zinc-500 focus:outline-none focus:ring-1 focus:ring-zinc-500 dark:border-zinc-700 dark:bg-zinc-900 dark:text-zinc-50"/>{errors?.fields?.password&&(<pclassName="text-sm text-red-600">{errors.fields.password.message}</p>)}</div>{errors?.global?.map((err,i)=>(<pkey={i}className="text-sm text-red-600">{err.longMessage}</p>))}<buttontype="submit"disabled={fetchStatus ==='fetching'}className="mt-2 rounded-md bg-zinc-900 px-4 py-2 text-sm font-medium text-white transition-colors hover:bg-zinc-800 disabled:opacity-50 dark:bg-zinc-50 dark:text-zinc-900 dark:hover:bg-zinc-200">{fetchStatus ==='fetching' ? 'Signing in...' : 'Sign in'}</button></form><pclassName="mt-4 text-center text-sm text-zinc-500">Don&apos;thaveanaccount?{' '}<Linkhref="/sign-up"className="font-medium text-zinc-900 hover:underline dark:text-zinc-50">Signup</Link></p></div></div>)}

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Summary by CodeRabbit

  • Bug Fixes

    • Fixed an issue where completion status could briefly show as complete during sign-in/sign-up while requests were still processing.
  • Improvements

    • Consolidated resource state handling and added batching to reduce unnecessary UI updates and ensure more consistent authentication status updates.

@changeset-bot

changeset-botBot commented Mar 4, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5bd33d4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
NameType
@clerk/clerk-jsPatch
@clerk/chrome-extensionPatch
@clerk/expoPatch

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

@vercel

vercelBot commented Mar 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
clerk-js-sandboxReadyReadyPreview, CommentMay 1, 2026 1:31am

Request Review

@nikosdouvlis

Copy link
Copy Markdown
Member

!snapshot

@clerk-cookie

Copy link
Copy Markdown
Collaborator

Hey @nikosdouvlis - the snapshot version command generated the following package versions:

PackageVersion
@clerk/agent-toolkit0.3.2-snapshot.v20260305105512
@clerk/astro3.0.2-snapshot.v20260305105512
@clerk/backend3.0.2-snapshot.v20260305105512
@clerk/chrome-extension3.0.2-snapshot.v20260305105512
@clerk/clerk-js6.0.1-snapshot.v20260305105512
@clerk/dev-cli0.1.1-snapshot.v20260305105512
@clerk/expo3.0.2-snapshot.v20260305105512
@clerk/expo-passkeys1.0.2-snapshot.v20260305105512
@clerk/express2.0.2-snapshot.v20260305105512
@clerk/fastify3.0.2-snapshot.v20260305105512
@clerk/hono0.0.4-snapshot.v20260305105512
@clerk/localizations4.0.1-snapshot.v20260305105512
@clerk/msw0.0.2-snapshot.v20260305105512
@clerk/nextjs7.0.2-snapshot.v20260305105512
@clerk/nuxt2.0.2-snapshot.v20260305105512
@clerk/react6.0.2-snapshot.v20260305105512
@clerk/react-router3.0.2-snapshot.v20260305105512
@clerk/shared4.0.1-snapshot.v20260305105512
@clerk/tanstack-react-start1.0.2-snapshot.v20260305105512
@clerk/testing2.0.2-snapshot.v20260305105512
@clerk/ui1.0.2-snapshot.v20260305105512
@clerk/upgrade2.0.2-snapshot.v20260305105512
@clerk/vue2.0.2-snapshot.v20260305105512

Tip: Use the snippet copy button below to quickly install the required packages.
@clerk/agent-toolkit

npm i @clerk/agent-toolkit@0.3.2-snapshot.v20260305105512 --save-exact

@clerk/astro

npm i @clerk/astro@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/backend

npm i @clerk/backend@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/chrome-extension

npm i @clerk/chrome-extension@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/clerk-js

npm i @clerk/clerk-js@6.0.1-snapshot.v20260305105512 --save-exact

@clerk/dev-cli

npm i @clerk/dev-cli@0.1.1-snapshot.v20260305105512 --save-exact

@clerk/expo

npm i @clerk/expo@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/expo-passkeys

npm i @clerk/expo-passkeys@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/express

npm i @clerk/express@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/fastify

npm i @clerk/fastify@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/hono

npm i @clerk/hono@0.0.4-snapshot.v20260305105512 --save-exact

@clerk/localizations

npm i @clerk/localizations@4.0.1-snapshot.v20260305105512 --save-exact

@clerk/msw

npm i @clerk/msw@0.0.2-snapshot.v20260305105512 --save-exact

@clerk/nextjs

npm i @clerk/nextjs@7.0.2-snapshot.v20260305105512 --save-exact

@clerk/nuxt

npm i @clerk/nuxt@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/react

npm i @clerk/react@6.0.2-snapshot.v20260305105512 --save-exact

@clerk/react-router

npm i @clerk/react-router@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/shared

npm i @clerk/shared@4.0.1-snapshot.v20260305105512 --save-exact

@clerk/tanstack-react-start

npm i @clerk/tanstack-react-start@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/testing

npm i @clerk/testing@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/ui

npm i @clerk/ui@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/upgrade

npm i @clerk/upgrade@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/vue

npm i @clerk/vue@2.0.2-snapshot.v20260305105512 --save-exact

@bratsos
bratsosforce-pushed the alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen branch from baab30c to 59c6aa4CompareMarch 5, 2026 18:31
@bratsos
bratsos marked this pull request as ready for review March 5, 2026 18:33
@coderabbitai

coderabbitaiBot commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 7ec07b01-a4f4-4323-a5d8-1ebe21587f3f

📥 Commits

Reviewing files that changed from the base of the PR and between 59c6aa4 and cb580f1.

📒 Files selected for processing (13)
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/__tests__/state.test.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/Waitlist.test.ts
  • packages/clerk-js/src/core/state.ts
  • packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts
  • packages/clerk-js/src/utils/runAsyncResourceTask.ts
🚧 Files skipped from review as they are similar to previous changes (7)
  • packages/clerk-js/src/core/resources/tests/SignIn.test.ts
  • packages/clerk-js/src/core/resources/tests/Waitlist.test.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/resources/SignUp.ts

📝 Walkthrough

Walkthrough

This PR consolidates resource state handling in the clerk/clerk-js package by replacing separate ResourceUpdate/ResourceError/ResourceFetch events with a single ResourceStateChange event whose payload includes resource, optional error, and optional fetchStatus. State handling was consolidated into a single onResourceStateChange handler that batches signal updates. Resources and client code were updated to emit the new event shape. runAsyncResourceTask was simplified to emit state-change events. A new signInFetchSignal export and additional tests covering batching and lifecycle edge cases were added.

Possibly related PRs

  • clerk/javascript PR 6549: initial fetch-status tracking that this change extends into a unified ResourceStateChange flow and signal batching.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title accurately and concisely summarizes the primary change: consolidating three separate resource events into a single unified event, which is the core refactoring goal of this PR.
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/clerk-js/src/core/resources/SignIn.ts (1)

1293-1312: ⚠️ Potential issue | 🟠 Major

finalize() can reject unexpectedly and mark the flow discardable before success

Line [1296] throws outside the try, so this method can reject instead of returning { error }. Also, setting #canBeDiscarded before setActive succeeds can prematurely allow null-resource replacement on failure.

Proposed fix
 async finalize(params?: SignInFutureFinalizeParams): Promise<{ error: ClerkError | null }> {
const { navigate } = params || {};
-- if (!this.#resource.createdSessionId) {- throw new Error('Cannot finalize sign-in without a created session.');- }
try {
+ if (!this.#resource.createdSessionId) {+ throw new Error('Cannot finalize sign-in without a created session.');+ }+
// Reload the client if the created session is not in the client's sessions. This can happen during modal SSO
// flows where the in-memory client does not have the created session.
if (SignIn.clerk.client && !SignIn.clerk.client.sessions.some(s => s.id === this.#resource.createdSessionId)) {
await SignIn.clerk.client.reload();
}
- this.#canBeDiscarded = true;
await SignIn.clerk.setActive({ session: this.#resource.createdSessionId, navigate });
+ this.#canBeDiscarded = true;
return { error: null };
} catch (err) {
return { error: err as ClerkError };
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/clerk-js/src/core/resources/SignIn.ts` around lines 1293 - 1312, The
finalize method currently throws if `#resource.createdSessionId` is missing and
sets `#canBeDiscarded` before setActive succeeds, so change finalize to return {
error } instead of throwing and only mark `#canBeDiscarded` true after
SignIn.clerk.setActive completes successfully: move the createdSessionId guard
and any throws into the try/catch (or convert them to an early return of {
error: new ClerkError(...) }) and relocate the assignment of
this.#canBeDiscarded to immediately after await SignIn.clerk.setActive(...)
succeeds; adjust error handling to return the caught error as ClerkError.
Reference finalize, this.#resource.createdSessionId, this.#canBeDiscarded,
SignIn.clerk.client.reload, and SignIn.clerk.setActive.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/clerk-js/src/core/resources/SignIn.ts`:
- Around line 1293-1312: The finalize method currently throws if
`#resource.createdSessionId` is missing and sets `#canBeDiscarded` before setActive
succeeds, so change finalize to return { error } instead of throwing and only
mark `#canBeDiscarded` true after SignIn.clerk.setActive completes successfully:
move the createdSessionId guard and any throws into the try/catch (or convert
them to an early return of { error: new ClerkError(...) }) and relocate the
assignment of this.#canBeDiscarded to immediately after await
SignIn.clerk.setActive(...) succeeds; adjust error handling to return the caught
error as ClerkError. Reference finalize, this.#resource.createdSessionId,
this.#canBeDiscarded, SignIn.clerk.client.reload, and SignIn.clerk.setActive.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: d47a6777-69a9-4390-a604-fcc13018fee0

📥 Commits

Reviewing files that changed from the base of the PR and between e13fc29 and 59c6aa4.

📒 Files selected for processing (13)
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/__tests__/state.test.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/Waitlist.test.ts
  • packages/clerk-js/src/core/state.ts
  • packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts
  • packages/clerk-js/src/utils/runAsyncResourceTask.ts

@bratsos

Copy link
Copy Markdown
ContributorAuthor

!snapshot

@bratsosbratsos closed this Mar 6, 2026
@bratsosbratsos reopened this Mar 6, 2026
@pkg-pr-new

pkg-pr-newBot commented Mar 6, 2026

Copy link
Copy Markdown

Open in StackBlitz

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@7980

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@7980

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@7980

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@7980

@clerk/dev-cli

npm i https://pkg.pr.new/@clerk/dev-cli@7980

@clerk/expo

npm i https://pkg.pr.new/@clerk/expo@7980

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@7980

@clerk/express

npm i https://pkg.pr.new/@clerk/express@7980

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@7980

@clerk/hono

npm i https://pkg.pr.new/@clerk/hono@7980

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@7980

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@7980

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@7980

@clerk/react

npm i https://pkg.pr.new/@clerk/react@7980

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@7980

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@7980

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@7980

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@7980

@clerk/ui

npm i https://pkg.pr.new/@clerk/ui@7980

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@7980

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@7980

commit: 5bd33d4

@dstaley

Copy link
Copy Markdown
Member

This looks great! I just have a few clarifying questions before I approve:

Consolidated three events into one. resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

I'm pretty sure I understand this, but just to state it again to make sure I understand: with this change we're now able to batch the resource/error/fetchStatus signal updates into a single update when those happen in a single call site. Meaning one event emit results in one signal update, but multiple events aren't batched in any way (which is totally fine, just making sure I understand). So, for example:

eventBus.emit('resource:state-change', { resource, error, fetchStatus })

triggers one signal update. I think I'm fine with this, but it does mean we no longer have the ability to update just the resource, or just the fetch status. As evidenced by this PR that's probably fine? But still worth calling out.

Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

This one I'm the most unsure of, because there's times when the resource identity isn't stable. However I can't think of an instance in which the resource identity will change during a method that's set fetchStatus to fetching so this is probably fine? Plus I think this is something we can easily revert if we discover that we do need this.

Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

The original intent here was to put the resource into a fetching status since we were doing something that should disable input (waiting for the navigation to be performed). I think it's fine to remove for now, and we can add back if we feel the need or see people doing things like isFinalizing to render specific UI during finalize().

@bratsos

bratsos commented Mar 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review!

This looks great! I just have a few clarifying questions before I approve:

Consolidated three events into one. resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

I'm pretty sure I understand this, but just to state it again to make sure I understand: with this change we're now able to batch the resource/error/fetchStatus signal updates into a single update when those happen in a single call site. Meaning one event emit results in one signal update, but multiple events aren't batched in any way (which is totally fine, just making sure I understand). So, for example:

eventBus.emit('resource:state-change', { resource, error, fetchStatus })

triggers one signal update. I think I'm fine with this, but it does mean we no longer have the ability to update just the resource, or just the fetch status. As evidenced by this PR that's probably fine? But still worth calling out.

Yeah exactly. One emit -> one handler -> one flush. Multiple emits are still independent.
Regarding losing the ability to update, the handler checks 'error' in payload and 'fetchStatus' in payload before writing each signal, so for example eventBus.emit('resource:state-change', { resource }) only writes the resource signal.

Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

This one I'm the most unsure of, because there's times when the resource identity isn't stable. However I can't think of an instance in which the resource identity will change during a method that's set fetchStatus to fetching so this is probably fine? Plus I think this is something we can easily revert if we discover that we do need this.

Yes that's correct, I think the only thing that would create a new instance mid-flight is a client refresh, which creates a new SignIn(null) but that would be blocked by shouldIgnoreNullUpdate, so it should be safe (and easy to revert if we find other cases).

Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

The original intent here was to put the resource into a fetching status since we were doing something that should disable input (waiting for the navigation to be performed). I think it's fine to remove for now, and we can add back if we feel the need or see people doing things like isFinalizing to render specific UI during finalize().

Makes sense. If we need that in the future, we could add a dedicated isFinalizing flag than reusing fetchStatusto keep semantics stay clean (fetchStatus = API call in progress, isFinalizing = session activation in progress). But agreed, no need to add it right now.

@bratsos
bratsosforce-pushed the alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen branch from 59c6aa4 to cb580f1CompareMarch 11, 2026 17:45
@github-actions

Copy link
Copy Markdown
Contributor

Hello 👋

We currently close PRs after 60 days of inactivity. It's been 50 days since the last update here. If we missed this PR, please reply here. Otherwise, we'll close this PR in 10 days.

Thanks for being a part of the Clerk community! 🙏

@github-actions

Copy link
Copy Markdown
Contributor

Hello 👋

We currently close PRs after 60 days of inactivity. It's been 50 days since the last update here. If we missed this PR, please reply here. Otherwise, we'll close this PR in 10 days.

Thanks for being a part of the Clerk community! 🙏

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@bratsos@nikosdouvlis@clerk-cookie@dstaley@jacekradko
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

refactor(clerk-js): Consolidate resource events into single event - #7980

Closed
bratsos wants to merge 2 commits into
mainfrom
alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen
Closed

refactor(clerk-js): Consolidate resource events into single event#7980
bratsos wants to merge 2 commits into
mainfrom
alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen

Conversation

@bratsos

@bratsosbratsos commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

Description

Resource state changes were spread across three separate events: resource:update, resource:error, and resource:fetch. Each one triggered its own signal write in State, which meant React could re-render between them. After an API call completed, React would briefly see the updated resource while fetchStatus was still 'fetching', because the resource and fetch-status updates arrived independently.

Even after consolidating into one event, a second source of inconsistency remained: fromJSON emits a resource-only event mid-flight (during an API call), which updated the resource signal before runAsyncResourceTask emitted the completion event with fetchStatus: 'idle'.

A third source came from finalize(), which used runAsyncResourceTask and set fetchStatus: 'fetching' on an already-complete resource even though it's just calling setActive, a session-level operation.

How we fixed it

Three changes, each addressing one source of inconsistency:

  1. Consolidated three events into one.resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

  2. Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

  3. Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

Demo

(showcasing the finalize() fix)

BeforeAfter
resource-events-before.mov
resource-events-after.mov
Sample code used for the demo
exportdefaultfunctionSignInPage(){const{ signIn, errors, fetchStatus }=useSignIn()constrouter=useRouter()const[email,setEmail]=useState('')const[password,setPassword]=useState('')const[toasts,setToasts]=useState<Toast[]>([])consttoastIdRef=useRef(0)useEffect(()=>{if(fetchStatus==='fetching'){setToasts(prev=>[...prev,{id: ++toastIdRef.current,message: 'Verifying your credentials...',type: 'info'}])}if(fetchStatus==='idle'){setToasts(prev=>prev.filter(t=>t.type!=='info'))}},[fetchStatus])useEffect(()=>{if(signIn?.status==='complete'){setToasts(prev=>[...prev,{id: ++toastIdRef.current,message: 'Welcome back! Redirecting...',type: 'success'}])}},[signIn?.status])functiondismissToast(id: number){setToasts(prev=>prev.filter(t=>t.id!==id))}asyncfunctionhandleSubmit(e: React.FormEvent){e.preventDefault()if(!signIn)returnsetToasts([])toastIdRef.current=0awaitsignIn.password({identifier: email, password })if(signIn.status==='complete'){awaitsignIn.finalize({navigate: async({ session, decorateUrl })=>{if(session?.currentTask){router.push(decorateUrl(`/sign-in/tasks/${session.currentTask.key}`))return}router.push(decorateUrl('/'))},})}}return(<divclassName="flex min-h-screen items-center justify-center bg-zinc-50 dark:bg-black"><ToastContainertoasts={toasts}onDismiss={dismissToast}/><divclassName="w-full max-w-sm rounded-lg border border-zinc-200 bg-white p-8 shadow-sm dark:border-zinc-800 dark:bg-zinc-950"><h1className="mb-6 text-center text-2xl font-semibold text-zinc-900 dark:text-zinc-50">Signin</h1><formonSubmit={handleSubmit}className="flex flex-col gap-4"><divclassName="flex flex-col gap-1"><labelhtmlFor="email"className="text-sm font-medium text-zinc-700 dark:text-zinc-300">Email</label><inputid="email"type="email"value={email}onChange={(e)=>setEmail(e.target.value)}placeholder="you@example.com"className="rounded-md border border-zinc-300 px-3 py-2 text-sm text-zinc-900 placeholder:text-zinc-400 focus:border-zinc-500 focus:outline-none focus:ring-1 focus:ring-zinc-500 dark:border-zinc-700 dark:bg-zinc-900 dark:text-zinc-50"/>{errors?.fields?.identifier&&(<pclassName="text-sm text-red-600">{errors.fields.identifier.message}</p>)}</div><divclassName="flex flex-col gap-1"><labelhtmlFor="password"className="text-sm font-medium text-zinc-700 dark:text-zinc-300">Password</label><inputid="password"type="password"value={password}onChange={(e)=>setPassword(e.target.value)}placeholder="Password"className="rounded-md border border-zinc-300 px-3 py-2 text-sm text-zinc-900 placeholder:text-zinc-400 focus:border-zinc-500 focus:outline-none focus:ring-1 focus:ring-zinc-500 dark:border-zinc-700 dark:bg-zinc-900 dark:text-zinc-50"/>{errors?.fields?.password&&(<pclassName="text-sm text-red-600">{errors.fields.password.message}</p>)}</div>{errors?.global?.map((err,i)=>(<pkey={i}className="text-sm text-red-600">{err.longMessage}</p>))}<buttontype="submit"disabled={fetchStatus ==='fetching'}className="mt-2 rounded-md bg-zinc-900 px-4 py-2 text-sm font-medium text-white transition-colors hover:bg-zinc-800 disabled:opacity-50 dark:bg-zinc-50 dark:text-zinc-900 dark:hover:bg-zinc-200">{fetchStatus ==='fetching' ? 'Signing in...' : 'Sign in'}</button></form><pclassName="mt-4 text-center text-sm text-zinc-500">Don&apos;thaveanaccount?{' '}<Linkhref="/sign-up"className="font-medium text-zinc-900 hover:underline dark:text-zinc-50">Signup</Link></p></div></div>)}

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Summary by CodeRabbit

  • Bug Fixes

    • Fixed an issue where completion status could briefly show as complete during sign-in/sign-up while requests were still processing.
  • Improvements

    • Consolidated resource state handling and added batching to reduce unnecessary UI updates and ensure more consistent authentication status updates.

@changeset-bot

changeset-botBot commented Mar 4, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5bd33d4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
NameType
@clerk/clerk-jsPatch
@clerk/chrome-extensionPatch
@clerk/expoPatch

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

@vercel

vercelBot commented Mar 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
clerk-js-sandboxReadyReadyPreview, CommentMay 1, 2026 1:31am

Request Review

@nikosdouvlis

Copy link
Copy Markdown
Member

!snapshot

@clerk-cookie

Copy link
Copy Markdown
Collaborator

Hey @nikosdouvlis - the snapshot version command generated the following package versions:

PackageVersion
@clerk/agent-toolkit0.3.2-snapshot.v20260305105512
@clerk/astro3.0.2-snapshot.v20260305105512
@clerk/backend3.0.2-snapshot.v20260305105512
@clerk/chrome-extension3.0.2-snapshot.v20260305105512
@clerk/clerk-js6.0.1-snapshot.v20260305105512
@clerk/dev-cli0.1.1-snapshot.v20260305105512
@clerk/expo3.0.2-snapshot.v20260305105512
@clerk/expo-passkeys1.0.2-snapshot.v20260305105512
@clerk/express2.0.2-snapshot.v20260305105512
@clerk/fastify3.0.2-snapshot.v20260305105512
@clerk/hono0.0.4-snapshot.v20260305105512
@clerk/localizations4.0.1-snapshot.v20260305105512
@clerk/msw0.0.2-snapshot.v20260305105512
@clerk/nextjs7.0.2-snapshot.v20260305105512
@clerk/nuxt2.0.2-snapshot.v20260305105512
@clerk/react6.0.2-snapshot.v20260305105512
@clerk/react-router3.0.2-snapshot.v20260305105512
@clerk/shared4.0.1-snapshot.v20260305105512
@clerk/tanstack-react-start1.0.2-snapshot.v20260305105512
@clerk/testing2.0.2-snapshot.v20260305105512
@clerk/ui1.0.2-snapshot.v20260305105512
@clerk/upgrade2.0.2-snapshot.v20260305105512
@clerk/vue2.0.2-snapshot.v20260305105512

Tip: Use the snippet copy button below to quickly install the required packages.
@clerk/agent-toolkit

npm i @clerk/agent-toolkit@0.3.2-snapshot.v20260305105512 --save-exact

@clerk/astro

npm i @clerk/astro@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/backend

npm i @clerk/backend@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/chrome-extension

npm i @clerk/chrome-extension@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/clerk-js

npm i @clerk/clerk-js@6.0.1-snapshot.v20260305105512 --save-exact

@clerk/dev-cli

npm i @clerk/dev-cli@0.1.1-snapshot.v20260305105512 --save-exact

@clerk/expo

npm i @clerk/expo@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/expo-passkeys

npm i @clerk/expo-passkeys@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/express

npm i @clerk/express@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/fastify

npm i @clerk/fastify@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/hono

npm i @clerk/hono@0.0.4-snapshot.v20260305105512 --save-exact

@clerk/localizations

npm i @clerk/localizations@4.0.1-snapshot.v20260305105512 --save-exact

@clerk/msw

npm i @clerk/msw@0.0.2-snapshot.v20260305105512 --save-exact

@clerk/nextjs

npm i @clerk/nextjs@7.0.2-snapshot.v20260305105512 --save-exact

@clerk/nuxt

npm i @clerk/nuxt@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/react

npm i @clerk/react@6.0.2-snapshot.v20260305105512 --save-exact

@clerk/react-router

npm i @clerk/react-router@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/shared

npm i @clerk/shared@4.0.1-snapshot.v20260305105512 --save-exact

@clerk/tanstack-react-start

npm i @clerk/tanstack-react-start@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/testing

npm i @clerk/testing@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/ui

npm i @clerk/ui@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/upgrade

npm i @clerk/upgrade@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/vue

npm i @clerk/vue@2.0.2-snapshot.v20260305105512 --save-exact

@bratsos
bratsosforce-pushed the alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen branch from baab30c to 59c6aa4CompareMarch 5, 2026 18:31
@bratsos
bratsos marked this pull request as ready for review March 5, 2026 18:33
@coderabbitai

coderabbitaiBot commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 7ec07b01-a4f4-4323-a5d8-1ebe21587f3f

📥 Commits

Reviewing files that changed from the base of the PR and between 59c6aa4 and cb580f1.

📒 Files selected for processing (13)
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/__tests__/state.test.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/Waitlist.test.ts
  • packages/clerk-js/src/core/state.ts
  • packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts
  • packages/clerk-js/src/utils/runAsyncResourceTask.ts
🚧 Files skipped from review as they are similar to previous changes (7)
  • packages/clerk-js/src/core/resources/tests/SignIn.test.ts
  • packages/clerk-js/src/core/resources/tests/Waitlist.test.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/resources/SignUp.ts

📝 Walkthrough

Walkthrough

This PR consolidates resource state handling in the clerk/clerk-js package by replacing separate ResourceUpdate/ResourceError/ResourceFetch events with a single ResourceStateChange event whose payload includes resource, optional error, and optional fetchStatus. State handling was consolidated into a single onResourceStateChange handler that batches signal updates. Resources and client code were updated to emit the new event shape. runAsyncResourceTask was simplified to emit state-change events. A new signInFetchSignal export and additional tests covering batching and lifecycle edge cases were added.

Possibly related PRs

  • clerk/javascript PR 6549: initial fetch-status tracking that this change extends into a unified ResourceStateChange flow and signal batching.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title accurately and concisely summarizes the primary change: consolidating three separate resource events into a single unified event, which is the core refactoring goal of this PR.
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/clerk-js/src/core/resources/SignIn.ts (1)

1293-1312: ⚠️ Potential issue | 🟠 Major

finalize() can reject unexpectedly and mark the flow discardable before success

Line [1296] throws outside the try, so this method can reject instead of returning { error }. Also, setting #canBeDiscarded before setActive succeeds can prematurely allow null-resource replacement on failure.

Proposed fix
 async finalize(params?: SignInFutureFinalizeParams): Promise<{ error: ClerkError | null }> {
const { navigate } = params || {};
-- if (!this.#resource.createdSessionId) {- throw new Error('Cannot finalize sign-in without a created session.');- }
try {
+ if (!this.#resource.createdSessionId) {+ throw new Error('Cannot finalize sign-in without a created session.');+ }+
// Reload the client if the created session is not in the client's sessions. This can happen during modal SSO
// flows where the in-memory client does not have the created session.
if (SignIn.clerk.client && !SignIn.clerk.client.sessions.some(s => s.id === this.#resource.createdSessionId)) {
await SignIn.clerk.client.reload();
}
- this.#canBeDiscarded = true;
await SignIn.clerk.setActive({ session: this.#resource.createdSessionId, navigate });
+ this.#canBeDiscarded = true;
return { error: null };
} catch (err) {
return { error: err as ClerkError };
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/clerk-js/src/core/resources/SignIn.ts` around lines 1293 - 1312, The
finalize method currently throws if `#resource.createdSessionId` is missing and
sets `#canBeDiscarded` before setActive succeeds, so change finalize to return {
error } instead of throwing and only mark `#canBeDiscarded` true after
SignIn.clerk.setActive completes successfully: move the createdSessionId guard
and any throws into the try/catch (or convert them to an early return of {
error: new ClerkError(...) }) and relocate the assignment of
this.#canBeDiscarded to immediately after await SignIn.clerk.setActive(...)
succeeds; adjust error handling to return the caught error as ClerkError.
Reference finalize, this.#resource.createdSessionId, this.#canBeDiscarded,
SignIn.clerk.client.reload, and SignIn.clerk.setActive.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/clerk-js/src/core/resources/SignIn.ts`:
- Around line 1293-1312: The finalize method currently throws if
`#resource.createdSessionId` is missing and sets `#canBeDiscarded` before setActive
succeeds, so change finalize to return { error } instead of throwing and only
mark `#canBeDiscarded` true after SignIn.clerk.setActive completes successfully:
move the createdSessionId guard and any throws into the try/catch (or convert
them to an early return of { error: new ClerkError(...) }) and relocate the
assignment of this.#canBeDiscarded to immediately after await
SignIn.clerk.setActive(...) succeeds; adjust error handling to return the caught
error as ClerkError. Reference finalize, this.#resource.createdSessionId,
this.#canBeDiscarded, SignIn.clerk.client.reload, and SignIn.clerk.setActive.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: d47a6777-69a9-4390-a604-fcc13018fee0

📥 Commits

Reviewing files that changed from the base of the PR and between e13fc29 and 59c6aa4.

📒 Files selected for processing (13)
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/__tests__/state.test.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/Waitlist.test.ts
  • packages/clerk-js/src/core/state.ts
  • packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts
  • packages/clerk-js/src/utils/runAsyncResourceTask.ts

@bratsos

Copy link
Copy Markdown
ContributorAuthor

!snapshot

@bratsosbratsos closed this Mar 6, 2026
@bratsosbratsos reopened this Mar 6, 2026
@pkg-pr-new

pkg-pr-newBot commented Mar 6, 2026

Copy link
Copy Markdown

Open in StackBlitz

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@7980

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@7980

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@7980

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@7980

@clerk/dev-cli

npm i https://pkg.pr.new/@clerk/dev-cli@7980

@clerk/expo

npm i https://pkg.pr.new/@clerk/expo@7980

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@7980

@clerk/express

npm i https://pkg.pr.new/@clerk/express@7980

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@7980

@clerk/hono

npm i https://pkg.pr.new/@clerk/hono@7980

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@7980

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@7980

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@7980

@clerk/react

npm i https://pkg.pr.new/@clerk/react@7980

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@7980

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@7980

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@7980

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@7980

@clerk/ui

npm i https://pkg.pr.new/@clerk/ui@7980

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@7980

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@7980

commit: 5bd33d4

@dstaley

Copy link
Copy Markdown
Member

This looks great! I just have a few clarifying questions before I approve:

Consolidated three events into one. resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

I'm pretty sure I understand this, but just to state it again to make sure I understand: with this change we're now able to batch the resource/error/fetchStatus signal updates into a single update when those happen in a single call site. Meaning one event emit results in one signal update, but multiple events aren't batched in any way (which is totally fine, just making sure I understand). So, for example:

eventBus.emit('resource:state-change', { resource, error, fetchStatus })

triggers one signal update. I think I'm fine with this, but it does mean we no longer have the ability to update just the resource, or just the fetch status. As evidenced by this PR that's probably fine? But still worth calling out.

Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

This one I'm the most unsure of, because there's times when the resource identity isn't stable. However I can't think of an instance in which the resource identity will change during a method that's set fetchStatus to fetching so this is probably fine? Plus I think this is something we can easily revert if we discover that we do need this.

Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

The original intent here was to put the resource into a fetching status since we were doing something that should disable input (waiting for the navigation to be performed). I think it's fine to remove for now, and we can add back if we feel the need or see people doing things like isFinalizing to render specific UI during finalize().

@bratsos

bratsos commented Mar 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review!

This looks great! I just have a few clarifying questions before I approve:

Consolidated three events into one. resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

I'm pretty sure I understand this, but just to state it again to make sure I understand: with this change we're now able to batch the resource/error/fetchStatus signal updates into a single update when those happen in a single call site. Meaning one event emit results in one signal update, but multiple events aren't batched in any way (which is totally fine, just making sure I understand). So, for example:

eventBus.emit('resource:state-change', { resource, error, fetchStatus })

triggers one signal update. I think I'm fine with this, but it does mean we no longer have the ability to update just the resource, or just the fetch status. As evidenced by this PR that's probably fine? But still worth calling out.

Yeah exactly. One emit -> one handler -> one flush. Multiple emits are still independent.
Regarding losing the ability to update, the handler checks 'error' in payload and 'fetchStatus' in payload before writing each signal, so for example eventBus.emit('resource:state-change', { resource }) only writes the resource signal.

Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

This one I'm the most unsure of, because there's times when the resource identity isn't stable. However I can't think of an instance in which the resource identity will change during a method that's set fetchStatus to fetching so this is probably fine? Plus I think this is something we can easily revert if we discover that we do need this.

Yes that's correct, I think the only thing that would create a new instance mid-flight is a client refresh, which creates a new SignIn(null) but that would be blocked by shouldIgnoreNullUpdate, so it should be safe (and easy to revert if we find other cases).

Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

The original intent here was to put the resource into a fetching status since we were doing something that should disable input (waiting for the navigation to be performed). I think it's fine to remove for now, and we can add back if we feel the need or see people doing things like isFinalizing to render specific UI during finalize().

Makes sense. If we need that in the future, we could add a dedicated isFinalizing flag than reusing fetchStatusto keep semantics stay clean (fetchStatus = API call in progress, isFinalizing = session activation in progress). But agreed, no need to add it right now.

@bratsos
bratsosforce-pushed the alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen branch from 59c6aa4 to cb580f1CompareMarch 11, 2026 17:45
@github-actions

Copy link
Copy Markdown
Contributor

Hello 👋

We currently close PRs after 60 days of inactivity. It's been 50 days since the last update here. If we missed this PR, please reply here. Otherwise, we'll close this PR in 10 days.

Thanks for being a part of the Clerk community! 🙏

@github-actions

Copy link
Copy Markdown
Contributor

Hello 👋

We currently close PRs after 60 days of inactivity. It's been 50 days since the last update here. If we missed this PR, please reply here. Otherwise, we'll close this PR in 10 days.

Thanks for being a part of the Clerk community! 🙏

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@bratsos@nikosdouvlis@clerk-cookie@dstaley@jacekradko
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

refactor(clerk-js): Consolidate resource events into single event - #7980

Closed
bratsos wants to merge 2 commits into
mainfrom
alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen
Closed

refactor(clerk-js): Consolidate resource events into single event#7980
bratsos wants to merge 2 commits into
mainfrom
alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen

Conversation

@bratsos

@bratsosbratsos commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

Description

Resource state changes were spread across three separate events: resource:update, resource:error, and resource:fetch. Each one triggered its own signal write in State, which meant React could re-render between them. After an API call completed, React would briefly see the updated resource while fetchStatus was still 'fetching', because the resource and fetch-status updates arrived independently.

Even after consolidating into one event, a second source of inconsistency remained: fromJSON emits a resource-only event mid-flight (during an API call), which updated the resource signal before runAsyncResourceTask emitted the completion event with fetchStatus: 'idle'.

A third source came from finalize(), which used runAsyncResourceTask and set fetchStatus: 'fetching' on an already-complete resource even though it's just calling setActive, a session-level operation.

How we fixed it

Three changes, each addressing one source of inconsistency:

  1. Consolidated three events into one.resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

  2. Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

  3. Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

Demo

(showcasing the finalize() fix)

BeforeAfter
resource-events-before.mov
resource-events-after.mov
Sample code used for the demo
exportdefaultfunctionSignInPage(){const{ signIn, errors, fetchStatus }=useSignIn()constrouter=useRouter()const[email,setEmail]=useState('')const[password,setPassword]=useState('')const[toasts,setToasts]=useState<Toast[]>([])consttoastIdRef=useRef(0)useEffect(()=>{if(fetchStatus==='fetching'){setToasts(prev=>[...prev,{id: ++toastIdRef.current,message: 'Verifying your credentials...',type: 'info'}])}if(fetchStatus==='idle'){setToasts(prev=>prev.filter(t=>t.type!=='info'))}},[fetchStatus])useEffect(()=>{if(signIn?.status==='complete'){setToasts(prev=>[...prev,{id: ++toastIdRef.current,message: 'Welcome back! Redirecting...',type: 'success'}])}},[signIn?.status])functiondismissToast(id: number){setToasts(prev=>prev.filter(t=>t.id!==id))}asyncfunctionhandleSubmit(e: React.FormEvent){e.preventDefault()if(!signIn)returnsetToasts([])toastIdRef.current=0awaitsignIn.password({identifier: email, password })if(signIn.status==='complete'){awaitsignIn.finalize({navigate: async({ session, decorateUrl })=>{if(session?.currentTask){router.push(decorateUrl(`/sign-in/tasks/${session.currentTask.key}`))return}router.push(decorateUrl('/'))},})}}return(<divclassName="flex min-h-screen items-center justify-center bg-zinc-50 dark:bg-black"><ToastContainertoasts={toasts}onDismiss={dismissToast}/><divclassName="w-full max-w-sm rounded-lg border border-zinc-200 bg-white p-8 shadow-sm dark:border-zinc-800 dark:bg-zinc-950"><h1className="mb-6 text-center text-2xl font-semibold text-zinc-900 dark:text-zinc-50">Signin</h1><formonSubmit={handleSubmit}className="flex flex-col gap-4"><divclassName="flex flex-col gap-1"><labelhtmlFor="email"className="text-sm font-medium text-zinc-700 dark:text-zinc-300">Email</label><inputid="email"type="email"value={email}onChange={(e)=>setEmail(e.target.value)}placeholder="you@example.com"className="rounded-md border border-zinc-300 px-3 py-2 text-sm text-zinc-900 placeholder:text-zinc-400 focus:border-zinc-500 focus:outline-none focus:ring-1 focus:ring-zinc-500 dark:border-zinc-700 dark:bg-zinc-900 dark:text-zinc-50"/>{errors?.fields?.identifier&&(<pclassName="text-sm text-red-600">{errors.fields.identifier.message}</p>)}</div><divclassName="flex flex-col gap-1"><labelhtmlFor="password"className="text-sm font-medium text-zinc-700 dark:text-zinc-300">Password</label><inputid="password"type="password"value={password}onChange={(e)=>setPassword(e.target.value)}placeholder="Password"className="rounded-md border border-zinc-300 px-3 py-2 text-sm text-zinc-900 placeholder:text-zinc-400 focus:border-zinc-500 focus:outline-none focus:ring-1 focus:ring-zinc-500 dark:border-zinc-700 dark:bg-zinc-900 dark:text-zinc-50"/>{errors?.fields?.password&&(<pclassName="text-sm text-red-600">{errors.fields.password.message}</p>)}</div>{errors?.global?.map((err,i)=>(<pkey={i}className="text-sm text-red-600">{err.longMessage}</p>))}<buttontype="submit"disabled={fetchStatus ==='fetching'}className="mt-2 rounded-md bg-zinc-900 px-4 py-2 text-sm font-medium text-white transition-colors hover:bg-zinc-800 disabled:opacity-50 dark:bg-zinc-50 dark:text-zinc-900 dark:hover:bg-zinc-200">{fetchStatus ==='fetching' ? 'Signing in...' : 'Sign in'}</button></form><pclassName="mt-4 text-center text-sm text-zinc-500">Don&apos;thaveanaccount?{' '}<Linkhref="/sign-up"className="font-medium text-zinc-900 hover:underline dark:text-zinc-50">Signup</Link></p></div></div>)}

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Summary by CodeRabbit

  • Bug Fixes

    • Fixed an issue where completion status could briefly show as complete during sign-in/sign-up while requests were still processing.
  • Improvements

    • Consolidated resource state handling and added batching to reduce unnecessary UI updates and ensure more consistent authentication status updates.

@changeset-bot

changeset-botBot commented Mar 4, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5bd33d4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
NameType
@clerk/clerk-jsPatch
@clerk/chrome-extensionPatch
@clerk/expoPatch

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

@vercel

vercelBot commented Mar 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
clerk-js-sandboxReadyReadyPreview, CommentMay 1, 2026 1:31am

Request Review

@nikosdouvlis

Copy link
Copy Markdown
Member

!snapshot

@clerk-cookie

Copy link
Copy Markdown
Collaborator

Hey @nikosdouvlis - the snapshot version command generated the following package versions:

PackageVersion
@clerk/agent-toolkit0.3.2-snapshot.v20260305105512
@clerk/astro3.0.2-snapshot.v20260305105512
@clerk/backend3.0.2-snapshot.v20260305105512
@clerk/chrome-extension3.0.2-snapshot.v20260305105512
@clerk/clerk-js6.0.1-snapshot.v20260305105512
@clerk/dev-cli0.1.1-snapshot.v20260305105512
@clerk/expo3.0.2-snapshot.v20260305105512
@clerk/expo-passkeys1.0.2-snapshot.v20260305105512
@clerk/express2.0.2-snapshot.v20260305105512
@clerk/fastify3.0.2-snapshot.v20260305105512
@clerk/hono0.0.4-snapshot.v20260305105512
@clerk/localizations4.0.1-snapshot.v20260305105512
@clerk/msw0.0.2-snapshot.v20260305105512
@clerk/nextjs7.0.2-snapshot.v20260305105512
@clerk/nuxt2.0.2-snapshot.v20260305105512
@clerk/react6.0.2-snapshot.v20260305105512
@clerk/react-router3.0.2-snapshot.v20260305105512
@clerk/shared4.0.1-snapshot.v20260305105512
@clerk/tanstack-react-start1.0.2-snapshot.v20260305105512
@clerk/testing2.0.2-snapshot.v20260305105512
@clerk/ui1.0.2-snapshot.v20260305105512
@clerk/upgrade2.0.2-snapshot.v20260305105512
@clerk/vue2.0.2-snapshot.v20260305105512

Tip: Use the snippet copy button below to quickly install the required packages.
@clerk/agent-toolkit

npm i @clerk/agent-toolkit@0.3.2-snapshot.v20260305105512 --save-exact

@clerk/astro

npm i @clerk/astro@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/backend

npm i @clerk/backend@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/chrome-extension

npm i @clerk/chrome-extension@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/clerk-js

npm i @clerk/clerk-js@6.0.1-snapshot.v20260305105512 --save-exact

@clerk/dev-cli

npm i @clerk/dev-cli@0.1.1-snapshot.v20260305105512 --save-exact

@clerk/expo

npm i @clerk/expo@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/expo-passkeys

npm i @clerk/expo-passkeys@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/express

npm i @clerk/express@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/fastify

npm i @clerk/fastify@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/hono

npm i @clerk/hono@0.0.4-snapshot.v20260305105512 --save-exact

@clerk/localizations

npm i @clerk/localizations@4.0.1-snapshot.v20260305105512 --save-exact

@clerk/msw

npm i @clerk/msw@0.0.2-snapshot.v20260305105512 --save-exact

@clerk/nextjs

npm i @clerk/nextjs@7.0.2-snapshot.v20260305105512 --save-exact

@clerk/nuxt

npm i @clerk/nuxt@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/react

npm i @clerk/react@6.0.2-snapshot.v20260305105512 --save-exact

@clerk/react-router

npm i @clerk/react-router@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/shared

npm i @clerk/shared@4.0.1-snapshot.v20260305105512 --save-exact

@clerk/tanstack-react-start

npm i @clerk/tanstack-react-start@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/testing

npm i @clerk/testing@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/ui

npm i @clerk/ui@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/upgrade

npm i @clerk/upgrade@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/vue

npm i @clerk/vue@2.0.2-snapshot.v20260305105512 --save-exact

@bratsos
bratsosforce-pushed the alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen branch from baab30c to 59c6aa4CompareMarch 5, 2026 18:31
@bratsos
bratsos marked this pull request as ready for review March 5, 2026 18:33
@coderabbitai

coderabbitaiBot commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 7ec07b01-a4f4-4323-a5d8-1ebe21587f3f

📥 Commits

Reviewing files that changed from the base of the PR and between 59c6aa4 and cb580f1.

📒 Files selected for processing (13)
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/__tests__/state.test.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/Waitlist.test.ts
  • packages/clerk-js/src/core/state.ts
  • packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts
  • packages/clerk-js/src/utils/runAsyncResourceTask.ts
🚧 Files skipped from review as they are similar to previous changes (7)
  • packages/clerk-js/src/core/resources/tests/SignIn.test.ts
  • packages/clerk-js/src/core/resources/tests/Waitlist.test.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/resources/SignUp.ts

📝 Walkthrough

Walkthrough

This PR consolidates resource state handling in the clerk/clerk-js package by replacing separate ResourceUpdate/ResourceError/ResourceFetch events with a single ResourceStateChange event whose payload includes resource, optional error, and optional fetchStatus. State handling was consolidated into a single onResourceStateChange handler that batches signal updates. Resources and client code were updated to emit the new event shape. runAsyncResourceTask was simplified to emit state-change events. A new signInFetchSignal export and additional tests covering batching and lifecycle edge cases were added.

Possibly related PRs

  • clerk/javascript PR 6549: initial fetch-status tracking that this change extends into a unified ResourceStateChange flow and signal batching.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title accurately and concisely summarizes the primary change: consolidating three separate resource events into a single unified event, which is the core refactoring goal of this PR.
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/clerk-js/src/core/resources/SignIn.ts (1)

1293-1312: ⚠️ Potential issue | 🟠 Major

finalize() can reject unexpectedly and mark the flow discardable before success

Line [1296] throws outside the try, so this method can reject instead of returning { error }. Also, setting #canBeDiscarded before setActive succeeds can prematurely allow null-resource replacement on failure.

Proposed fix
 async finalize(params?: SignInFutureFinalizeParams): Promise<{ error: ClerkError | null }> {
const { navigate } = params || {};
-- if (!this.#resource.createdSessionId) {- throw new Error('Cannot finalize sign-in without a created session.');- }
try {
+ if (!this.#resource.createdSessionId) {+ throw new Error('Cannot finalize sign-in without a created session.');+ }+
// Reload the client if the created session is not in the client's sessions. This can happen during modal SSO
// flows where the in-memory client does not have the created session.
if (SignIn.clerk.client && !SignIn.clerk.client.sessions.some(s => s.id === this.#resource.createdSessionId)) {
await SignIn.clerk.client.reload();
}
- this.#canBeDiscarded = true;
await SignIn.clerk.setActive({ session: this.#resource.createdSessionId, navigate });
+ this.#canBeDiscarded = true;
return { error: null };
} catch (err) {
return { error: err as ClerkError };
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/clerk-js/src/core/resources/SignIn.ts` around lines 1293 - 1312, The
finalize method currently throws if `#resource.createdSessionId` is missing and
sets `#canBeDiscarded` before setActive succeeds, so change finalize to return {
error } instead of throwing and only mark `#canBeDiscarded` true after
SignIn.clerk.setActive completes successfully: move the createdSessionId guard
and any throws into the try/catch (or convert them to an early return of {
error: new ClerkError(...) }) and relocate the assignment of
this.#canBeDiscarded to immediately after await SignIn.clerk.setActive(...)
succeeds; adjust error handling to return the caught error as ClerkError.
Reference finalize, this.#resource.createdSessionId, this.#canBeDiscarded,
SignIn.clerk.client.reload, and SignIn.clerk.setActive.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/clerk-js/src/core/resources/SignIn.ts`:
- Around line 1293-1312: The finalize method currently throws if
`#resource.createdSessionId` is missing and sets `#canBeDiscarded` before setActive
succeeds, so change finalize to return { error } instead of throwing and only
mark `#canBeDiscarded` true after SignIn.clerk.setActive completes successfully:
move the createdSessionId guard and any throws into the try/catch (or convert
them to an early return of { error: new ClerkError(...) }) and relocate the
assignment of this.#canBeDiscarded to immediately after await
SignIn.clerk.setActive(...) succeeds; adjust error handling to return the caught
error as ClerkError. Reference finalize, this.#resource.createdSessionId,
this.#canBeDiscarded, SignIn.clerk.client.reload, and SignIn.clerk.setActive.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: d47a6777-69a9-4390-a604-fcc13018fee0

📥 Commits

Reviewing files that changed from the base of the PR and between e13fc29 and 59c6aa4.

📒 Files selected for processing (13)
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/__tests__/state.test.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/Waitlist.test.ts
  • packages/clerk-js/src/core/state.ts
  • packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts
  • packages/clerk-js/src/utils/runAsyncResourceTask.ts

@bratsos

Copy link
Copy Markdown
ContributorAuthor

!snapshot

@bratsosbratsos closed this Mar 6, 2026
@bratsosbratsos reopened this Mar 6, 2026
@pkg-pr-new

pkg-pr-newBot commented Mar 6, 2026

Copy link
Copy Markdown

Open in StackBlitz

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@7980

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@7980

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@7980

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@7980

@clerk/dev-cli

npm i https://pkg.pr.new/@clerk/dev-cli@7980

@clerk/expo

npm i https://pkg.pr.new/@clerk/expo@7980

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@7980

@clerk/express

npm i https://pkg.pr.new/@clerk/express@7980

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@7980

@clerk/hono

npm i https://pkg.pr.new/@clerk/hono@7980

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@7980

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@7980

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@7980

@clerk/react

npm i https://pkg.pr.new/@clerk/react@7980

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@7980

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@7980

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@7980

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@7980

@clerk/ui

npm i https://pkg.pr.new/@clerk/ui@7980

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@7980

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@7980

commit: 5bd33d4

@dstaley

Copy link
Copy Markdown
Member

This looks great! I just have a few clarifying questions before I approve:

Consolidated three events into one. resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

I'm pretty sure I understand this, but just to state it again to make sure I understand: with this change we're now able to batch the resource/error/fetchStatus signal updates into a single update when those happen in a single call site. Meaning one event emit results in one signal update, but multiple events aren't batched in any way (which is totally fine, just making sure I understand). So, for example:

eventBus.emit('resource:state-change', { resource, error, fetchStatus })

triggers one signal update. I think I'm fine with this, but it does mean we no longer have the ability to update just the resource, or just the fetch status. As evidenced by this PR that's probably fine? But still worth calling out.

Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

This one I'm the most unsure of, because there's times when the resource identity isn't stable. However I can't think of an instance in which the resource identity will change during a method that's set fetchStatus to fetching so this is probably fine? Plus I think this is something we can easily revert if we discover that we do need this.

Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

The original intent here was to put the resource into a fetching status since we were doing something that should disable input (waiting for the navigation to be performed). I think it's fine to remove for now, and we can add back if we feel the need or see people doing things like isFinalizing to render specific UI during finalize().

@bratsos

bratsos commented Mar 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review!

This looks great! I just have a few clarifying questions before I approve:

Consolidated three events into one. resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

I'm pretty sure I understand this, but just to state it again to make sure I understand: with this change we're now able to batch the resource/error/fetchStatus signal updates into a single update when those happen in a single call site. Meaning one event emit results in one signal update, but multiple events aren't batched in any way (which is totally fine, just making sure I understand). So, for example:

eventBus.emit('resource:state-change', { resource, error, fetchStatus })

triggers one signal update. I think I'm fine with this, but it does mean we no longer have the ability to update just the resource, or just the fetch status. As evidenced by this PR that's probably fine? But still worth calling out.

Yeah exactly. One emit -> one handler -> one flush. Multiple emits are still independent.
Regarding losing the ability to update, the handler checks 'error' in payload and 'fetchStatus' in payload before writing each signal, so for example eventBus.emit('resource:state-change', { resource }) only writes the resource signal.

Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

This one I'm the most unsure of, because there's times when the resource identity isn't stable. However I can't think of an instance in which the resource identity will change during a method that's set fetchStatus to fetching so this is probably fine? Plus I think this is something we can easily revert if we discover that we do need this.

Yes that's correct, I think the only thing that would create a new instance mid-flight is a client refresh, which creates a new SignIn(null) but that would be blocked by shouldIgnoreNullUpdate, so it should be safe (and easy to revert if we find other cases).

Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

The original intent here was to put the resource into a fetching status since we were doing something that should disable input (waiting for the navigation to be performed). I think it's fine to remove for now, and we can add back if we feel the need or see people doing things like isFinalizing to render specific UI during finalize().

Makes sense. If we need that in the future, we could add a dedicated isFinalizing flag than reusing fetchStatusto keep semantics stay clean (fetchStatus = API call in progress, isFinalizing = session activation in progress). But agreed, no need to add it right now.

@bratsos
bratsosforce-pushed the alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen branch from 59c6aa4 to cb580f1CompareMarch 11, 2026 17:45
@github-actions

Copy link
Copy Markdown
Contributor

Hello 👋

We currently close PRs after 60 days of inactivity. It's been 50 days since the last update here. If we missed this PR, please reply here. Otherwise, we'll close this PR in 10 days.

Thanks for being a part of the Clerk community! 🙏

@github-actions

Copy link
Copy Markdown
Contributor

Hello 👋

We currently close PRs after 60 days of inactivity. It's been 50 days since the last update here. If we missed this PR, please reply here. Otherwise, we'll close this PR in 10 days.

Thanks for being a part of the Clerk community! 🙏

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@bratsos@nikosdouvlis@clerk-cookie@dstaley@jacekradko
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

refactor(clerk-js): Consolidate resource events into single event - #7980

Closed
bratsos wants to merge 2 commits into
mainfrom
alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen
Closed

refactor(clerk-js): Consolidate resource events into single event#7980
bratsos wants to merge 2 commits into
mainfrom
alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen

Conversation

@bratsos

@bratsosbratsos commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

Description

Resource state changes were spread across three separate events: resource:update, resource:error, and resource:fetch. Each one triggered its own signal write in State, which meant React could re-render between them. After an API call completed, React would briefly see the updated resource while fetchStatus was still 'fetching', because the resource and fetch-status updates arrived independently.

Even after consolidating into one event, a second source of inconsistency remained: fromJSON emits a resource-only event mid-flight (during an API call), which updated the resource signal before runAsyncResourceTask emitted the completion event with fetchStatus: 'idle'.

A third source came from finalize(), which used runAsyncResourceTask and set fetchStatus: 'fetching' on an already-complete resource even though it's just calling setActive, a session-level operation.

How we fixed it

Three changes, each addressing one source of inconsistency:

  1. Consolidated three events into one.resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

  2. Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

  3. Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

Demo

(showcasing the finalize() fix)

BeforeAfter
resource-events-before.mov
resource-events-after.mov
Sample code used for the demo
exportdefaultfunctionSignInPage(){const{ signIn, errors, fetchStatus }=useSignIn()constrouter=useRouter()const[email,setEmail]=useState('')const[password,setPassword]=useState('')const[toasts,setToasts]=useState<Toast[]>([])consttoastIdRef=useRef(0)useEffect(()=>{if(fetchStatus==='fetching'){setToasts(prev=>[...prev,{id: ++toastIdRef.current,message: 'Verifying your credentials...',type: 'info'}])}if(fetchStatus==='idle'){setToasts(prev=>prev.filter(t=>t.type!=='info'))}},[fetchStatus])useEffect(()=>{if(signIn?.status==='complete'){setToasts(prev=>[...prev,{id: ++toastIdRef.current,message: 'Welcome back! Redirecting...',type: 'success'}])}},[signIn?.status])functiondismissToast(id: number){setToasts(prev=>prev.filter(t=>t.id!==id))}asyncfunctionhandleSubmit(e: React.FormEvent){e.preventDefault()if(!signIn)returnsetToasts([])toastIdRef.current=0awaitsignIn.password({identifier: email, password })if(signIn.status==='complete'){awaitsignIn.finalize({navigate: async({ session, decorateUrl })=>{if(session?.currentTask){router.push(decorateUrl(`/sign-in/tasks/${session.currentTask.key}`))return}router.push(decorateUrl('/'))},})}}return(<divclassName="flex min-h-screen items-center justify-center bg-zinc-50 dark:bg-black"><ToastContainertoasts={toasts}onDismiss={dismissToast}/><divclassName="w-full max-w-sm rounded-lg border border-zinc-200 bg-white p-8 shadow-sm dark:border-zinc-800 dark:bg-zinc-950"><h1className="mb-6 text-center text-2xl font-semibold text-zinc-900 dark:text-zinc-50">Signin</h1><formonSubmit={handleSubmit}className="flex flex-col gap-4"><divclassName="flex flex-col gap-1"><labelhtmlFor="email"className="text-sm font-medium text-zinc-700 dark:text-zinc-300">Email</label><inputid="email"type="email"value={email}onChange={(e)=>setEmail(e.target.value)}placeholder="you@example.com"className="rounded-md border border-zinc-300 px-3 py-2 text-sm text-zinc-900 placeholder:text-zinc-400 focus:border-zinc-500 focus:outline-none focus:ring-1 focus:ring-zinc-500 dark:border-zinc-700 dark:bg-zinc-900 dark:text-zinc-50"/>{errors?.fields?.identifier&&(<pclassName="text-sm text-red-600">{errors.fields.identifier.message}</p>)}</div><divclassName="flex flex-col gap-1"><labelhtmlFor="password"className="text-sm font-medium text-zinc-700 dark:text-zinc-300">Password</label><inputid="password"type="password"value={password}onChange={(e)=>setPassword(e.target.value)}placeholder="Password"className="rounded-md border border-zinc-300 px-3 py-2 text-sm text-zinc-900 placeholder:text-zinc-400 focus:border-zinc-500 focus:outline-none focus:ring-1 focus:ring-zinc-500 dark:border-zinc-700 dark:bg-zinc-900 dark:text-zinc-50"/>{errors?.fields?.password&&(<pclassName="text-sm text-red-600">{errors.fields.password.message}</p>)}</div>{errors?.global?.map((err,i)=>(<pkey={i}className="text-sm text-red-600">{err.longMessage}</p>))}<buttontype="submit"disabled={fetchStatus ==='fetching'}className="mt-2 rounded-md bg-zinc-900 px-4 py-2 text-sm font-medium text-white transition-colors hover:bg-zinc-800 disabled:opacity-50 dark:bg-zinc-50 dark:text-zinc-900 dark:hover:bg-zinc-200">{fetchStatus ==='fetching' ? 'Signing in...' : 'Sign in'}</button></form><pclassName="mt-4 text-center text-sm text-zinc-500">Don&apos;thaveanaccount?{' '}<Linkhref="/sign-up"className="font-medium text-zinc-900 hover:underline dark:text-zinc-50">Signup</Link></p></div></div>)}

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Summary by CodeRabbit

  • Bug Fixes

    • Fixed an issue where completion status could briefly show as complete during sign-in/sign-up while requests were still processing.
  • Improvements

    • Consolidated resource state handling and added batching to reduce unnecessary UI updates and ensure more consistent authentication status updates.

@changeset-bot

changeset-botBot commented Mar 4, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5bd33d4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
NameType
@clerk/clerk-jsPatch
@clerk/chrome-extensionPatch
@clerk/expoPatch

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

@vercel

vercelBot commented Mar 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
clerk-js-sandboxReadyReadyPreview, CommentMay 1, 2026 1:31am

Request Review

@nikosdouvlis

Copy link
Copy Markdown
Member

!snapshot

@clerk-cookie

Copy link
Copy Markdown
Collaborator

Hey @nikosdouvlis - the snapshot version command generated the following package versions:

PackageVersion
@clerk/agent-toolkit0.3.2-snapshot.v20260305105512
@clerk/astro3.0.2-snapshot.v20260305105512
@clerk/backend3.0.2-snapshot.v20260305105512
@clerk/chrome-extension3.0.2-snapshot.v20260305105512
@clerk/clerk-js6.0.1-snapshot.v20260305105512
@clerk/dev-cli0.1.1-snapshot.v20260305105512
@clerk/expo3.0.2-snapshot.v20260305105512
@clerk/expo-passkeys1.0.2-snapshot.v20260305105512
@clerk/express2.0.2-snapshot.v20260305105512
@clerk/fastify3.0.2-snapshot.v20260305105512
@clerk/hono0.0.4-snapshot.v20260305105512
@clerk/localizations4.0.1-snapshot.v20260305105512
@clerk/msw0.0.2-snapshot.v20260305105512
@clerk/nextjs7.0.2-snapshot.v20260305105512
@clerk/nuxt2.0.2-snapshot.v20260305105512
@clerk/react6.0.2-snapshot.v20260305105512
@clerk/react-router3.0.2-snapshot.v20260305105512
@clerk/shared4.0.1-snapshot.v20260305105512
@clerk/tanstack-react-start1.0.2-snapshot.v20260305105512
@clerk/testing2.0.2-snapshot.v20260305105512
@clerk/ui1.0.2-snapshot.v20260305105512
@clerk/upgrade2.0.2-snapshot.v20260305105512
@clerk/vue2.0.2-snapshot.v20260305105512

Tip: Use the snippet copy button below to quickly install the required packages.
@clerk/agent-toolkit

npm i @clerk/agent-toolkit@0.3.2-snapshot.v20260305105512 --save-exact

@clerk/astro

npm i @clerk/astro@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/backend

npm i @clerk/backend@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/chrome-extension

npm i @clerk/chrome-extension@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/clerk-js

npm i @clerk/clerk-js@6.0.1-snapshot.v20260305105512 --save-exact

@clerk/dev-cli

npm i @clerk/dev-cli@0.1.1-snapshot.v20260305105512 --save-exact

@clerk/expo

npm i @clerk/expo@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/expo-passkeys

npm i @clerk/expo-passkeys@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/express

npm i @clerk/express@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/fastify

npm i @clerk/fastify@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/hono

npm i @clerk/hono@0.0.4-snapshot.v20260305105512 --save-exact

@clerk/localizations

npm i @clerk/localizations@4.0.1-snapshot.v20260305105512 --save-exact

@clerk/msw

npm i @clerk/msw@0.0.2-snapshot.v20260305105512 --save-exact

@clerk/nextjs

npm i @clerk/nextjs@7.0.2-snapshot.v20260305105512 --save-exact

@clerk/nuxt

npm i @clerk/nuxt@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/react

npm i @clerk/react@6.0.2-snapshot.v20260305105512 --save-exact

@clerk/react-router

npm i @clerk/react-router@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/shared

npm i @clerk/shared@4.0.1-snapshot.v20260305105512 --save-exact

@clerk/tanstack-react-start

npm i @clerk/tanstack-react-start@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/testing

npm i @clerk/testing@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/ui

npm i @clerk/ui@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/upgrade

npm i @clerk/upgrade@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/vue

npm i @clerk/vue@2.0.2-snapshot.v20260305105512 --save-exact

@bratsos
bratsosforce-pushed the alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen branch from baab30c to 59c6aa4CompareMarch 5, 2026 18:31
@bratsos
bratsos marked this pull request as ready for review March 5, 2026 18:33
@coderabbitai

coderabbitaiBot commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 7ec07b01-a4f4-4323-a5d8-1ebe21587f3f

📥 Commits

Reviewing files that changed from the base of the PR and between 59c6aa4 and cb580f1.

📒 Files selected for processing (13)
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/__tests__/state.test.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/Waitlist.test.ts
  • packages/clerk-js/src/core/state.ts
  • packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts
  • packages/clerk-js/src/utils/runAsyncResourceTask.ts
🚧 Files skipped from review as they are similar to previous changes (7)
  • packages/clerk-js/src/core/resources/tests/SignIn.test.ts
  • packages/clerk-js/src/core/resources/tests/Waitlist.test.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/resources/SignUp.ts

📝 Walkthrough

Walkthrough

This PR consolidates resource state handling in the clerk/clerk-js package by replacing separate ResourceUpdate/ResourceError/ResourceFetch events with a single ResourceStateChange event whose payload includes resource, optional error, and optional fetchStatus. State handling was consolidated into a single onResourceStateChange handler that batches signal updates. Resources and client code were updated to emit the new event shape. runAsyncResourceTask was simplified to emit state-change events. A new signInFetchSignal export and additional tests covering batching and lifecycle edge cases were added.

Possibly related PRs

  • clerk/javascript PR 6549: initial fetch-status tracking that this change extends into a unified ResourceStateChange flow and signal batching.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title accurately and concisely summarizes the primary change: consolidating three separate resource events into a single unified event, which is the core refactoring goal of this PR.
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/clerk-js/src/core/resources/SignIn.ts (1)

1293-1312: ⚠️ Potential issue | 🟠 Major

finalize() can reject unexpectedly and mark the flow discardable before success

Line [1296] throws outside the try, so this method can reject instead of returning { error }. Also, setting #canBeDiscarded before setActive succeeds can prematurely allow null-resource replacement on failure.

Proposed fix
 async finalize(params?: SignInFutureFinalizeParams): Promise<{ error: ClerkError | null }> {
const { navigate } = params || {};
-- if (!this.#resource.createdSessionId) {- throw new Error('Cannot finalize sign-in without a created session.');- }
try {
+ if (!this.#resource.createdSessionId) {+ throw new Error('Cannot finalize sign-in without a created session.');+ }+
// Reload the client if the created session is not in the client's sessions. This can happen during modal SSO
// flows where the in-memory client does not have the created session.
if (SignIn.clerk.client && !SignIn.clerk.client.sessions.some(s => s.id === this.#resource.createdSessionId)) {
await SignIn.clerk.client.reload();
}
- this.#canBeDiscarded = true;
await SignIn.clerk.setActive({ session: this.#resource.createdSessionId, navigate });
+ this.#canBeDiscarded = true;
return { error: null };
} catch (err) {
return { error: err as ClerkError };
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/clerk-js/src/core/resources/SignIn.ts` around lines 1293 - 1312, The
finalize method currently throws if `#resource.createdSessionId` is missing and
sets `#canBeDiscarded` before setActive succeeds, so change finalize to return {
error } instead of throwing and only mark `#canBeDiscarded` true after
SignIn.clerk.setActive completes successfully: move the createdSessionId guard
and any throws into the try/catch (or convert them to an early return of {
error: new ClerkError(...) }) and relocate the assignment of
this.#canBeDiscarded to immediately after await SignIn.clerk.setActive(...)
succeeds; adjust error handling to return the caught error as ClerkError.
Reference finalize, this.#resource.createdSessionId, this.#canBeDiscarded,
SignIn.clerk.client.reload, and SignIn.clerk.setActive.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/clerk-js/src/core/resources/SignIn.ts`:
- Around line 1293-1312: The finalize method currently throws if
`#resource.createdSessionId` is missing and sets `#canBeDiscarded` before setActive
succeeds, so change finalize to return { error } instead of throwing and only
mark `#canBeDiscarded` true after SignIn.clerk.setActive completes successfully:
move the createdSessionId guard and any throws into the try/catch (or convert
them to an early return of { error: new ClerkError(...) }) and relocate the
assignment of this.#canBeDiscarded to immediately after await
SignIn.clerk.setActive(...) succeeds; adjust error handling to return the caught
error as ClerkError. Reference finalize, this.#resource.createdSessionId,
this.#canBeDiscarded, SignIn.clerk.client.reload, and SignIn.clerk.setActive.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: d47a6777-69a9-4390-a604-fcc13018fee0

📥 Commits

Reviewing files that changed from the base of the PR and between e13fc29 and 59c6aa4.

📒 Files selected for processing (13)
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/__tests__/state.test.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/Waitlist.test.ts
  • packages/clerk-js/src/core/state.ts
  • packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts
  • packages/clerk-js/src/utils/runAsyncResourceTask.ts

@bratsos

Copy link
Copy Markdown
ContributorAuthor

!snapshot

@bratsosbratsos closed this Mar 6, 2026
@bratsosbratsos reopened this Mar 6, 2026
@pkg-pr-new

pkg-pr-newBot commented Mar 6, 2026

Copy link
Copy Markdown

Open in StackBlitz

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@7980

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@7980

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@7980

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@7980

@clerk/dev-cli

npm i https://pkg.pr.new/@clerk/dev-cli@7980

@clerk/expo

npm i https://pkg.pr.new/@clerk/expo@7980

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@7980

@clerk/express

npm i https://pkg.pr.new/@clerk/express@7980

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@7980

@clerk/hono

npm i https://pkg.pr.new/@clerk/hono@7980

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@7980

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@7980

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@7980

@clerk/react

npm i https://pkg.pr.new/@clerk/react@7980

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@7980

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@7980

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@7980

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@7980

@clerk/ui

npm i https://pkg.pr.new/@clerk/ui@7980

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@7980

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@7980

commit: 5bd33d4

@dstaley

Copy link
Copy Markdown
Member

This looks great! I just have a few clarifying questions before I approve:

Consolidated three events into one. resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

I'm pretty sure I understand this, but just to state it again to make sure I understand: with this change we're now able to batch the resource/error/fetchStatus signal updates into a single update when those happen in a single call site. Meaning one event emit results in one signal update, but multiple events aren't batched in any way (which is totally fine, just making sure I understand). So, for example:

eventBus.emit('resource:state-change', { resource, error, fetchStatus })

triggers one signal update. I think I'm fine with this, but it does mean we no longer have the ability to update just the resource, or just the fetch status. As evidenced by this PR that's probably fine? But still worth calling out.

Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

This one I'm the most unsure of, because there's times when the resource identity isn't stable. However I can't think of an instance in which the resource identity will change during a method that's set fetchStatus to fetching so this is probably fine? Plus I think this is something we can easily revert if we discover that we do need this.

Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

The original intent here was to put the resource into a fetching status since we were doing something that should disable input (waiting for the navigation to be performed). I think it's fine to remove for now, and we can add back if we feel the need or see people doing things like isFinalizing to render specific UI during finalize().

@bratsos

bratsos commented Mar 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review!

This looks great! I just have a few clarifying questions before I approve:

Consolidated three events into one. resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

I'm pretty sure I understand this, but just to state it again to make sure I understand: with this change we're now able to batch the resource/error/fetchStatus signal updates into a single update when those happen in a single call site. Meaning one event emit results in one signal update, but multiple events aren't batched in any way (which is totally fine, just making sure I understand). So, for example:

eventBus.emit('resource:state-change', { resource, error, fetchStatus })

triggers one signal update. I think I'm fine with this, but it does mean we no longer have the ability to update just the resource, or just the fetch status. As evidenced by this PR that's probably fine? But still worth calling out.

Yeah exactly. One emit -> one handler -> one flush. Multiple emits are still independent.
Regarding losing the ability to update, the handler checks 'error' in payload and 'fetchStatus' in payload before writing each signal, so for example eventBus.emit('resource:state-change', { resource }) only writes the resource signal.

Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

This one I'm the most unsure of, because there's times when the resource identity isn't stable. However I can't think of an instance in which the resource identity will change during a method that's set fetchStatus to fetching so this is probably fine? Plus I think this is something we can easily revert if we discover that we do need this.

Yes that's correct, I think the only thing that would create a new instance mid-flight is a client refresh, which creates a new SignIn(null) but that would be blocked by shouldIgnoreNullUpdate, so it should be safe (and easy to revert if we find other cases).

Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

The original intent here was to put the resource into a fetching status since we were doing something that should disable input (waiting for the navigation to be performed). I think it's fine to remove for now, and we can add back if we feel the need or see people doing things like isFinalizing to render specific UI during finalize().

Makes sense. If we need that in the future, we could add a dedicated isFinalizing flag than reusing fetchStatusto keep semantics stay clean (fetchStatus = API call in progress, isFinalizing = session activation in progress). But agreed, no need to add it right now.

@bratsos
bratsosforce-pushed the alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen branch from 59c6aa4 to cb580f1CompareMarch 11, 2026 17:45
@github-actions

Copy link
Copy Markdown
Contributor

Hello 👋

We currently close PRs after 60 days of inactivity. It's been 50 days since the last update here. If we missed this PR, please reply here. Otherwise, we'll close this PR in 10 days.

Thanks for being a part of the Clerk community! 🙏

@github-actions

Copy link
Copy Markdown
Contributor

Hello 👋

We currently close PRs after 60 days of inactivity. It's been 50 days since the last update here. If we missed this PR, please reply here. Otherwise, we'll close this PR in 10 days.

Thanks for being a part of the Clerk community! 🙏

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@bratsos@nikosdouvlis@clerk-cookie@dstaley@jacekradko
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

refactor(clerk-js): Consolidate resource events into single event - #7980

Closed
bratsos wants to merge 2 commits into
mainfrom
alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen
Closed

refactor(clerk-js): Consolidate resource events into single event#7980
bratsos wants to merge 2 commits into
mainfrom
alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen

Conversation

@bratsos

@bratsosbratsos commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

Description

Resource state changes were spread across three separate events: resource:update, resource:error, and resource:fetch. Each one triggered its own signal write in State, which meant React could re-render between them. After an API call completed, React would briefly see the updated resource while fetchStatus was still 'fetching', because the resource and fetch-status updates arrived independently.

Even after consolidating into one event, a second source of inconsistency remained: fromJSON emits a resource-only event mid-flight (during an API call), which updated the resource signal before runAsyncResourceTask emitted the completion event with fetchStatus: 'idle'.

A third source came from finalize(), which used runAsyncResourceTask and set fetchStatus: 'fetching' on an already-complete resource even though it's just calling setActive, a session-level operation.

How we fixed it

Three changes, each addressing one source of inconsistency:

  1. Consolidated three events into one.resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

  2. Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

  3. Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

Demo

(showcasing the finalize() fix)

BeforeAfter
resource-events-before.mov
resource-events-after.mov
Sample code used for the demo
exportdefaultfunctionSignInPage(){const{ signIn, errors, fetchStatus }=useSignIn()constrouter=useRouter()const[email,setEmail]=useState('')const[password,setPassword]=useState('')const[toasts,setToasts]=useState<Toast[]>([])consttoastIdRef=useRef(0)useEffect(()=>{if(fetchStatus==='fetching'){setToasts(prev=>[...prev,{id: ++toastIdRef.current,message: 'Verifying your credentials...',type: 'info'}])}if(fetchStatus==='idle'){setToasts(prev=>prev.filter(t=>t.type!=='info'))}},[fetchStatus])useEffect(()=>{if(signIn?.status==='complete'){setToasts(prev=>[...prev,{id: ++toastIdRef.current,message: 'Welcome back! Redirecting...',type: 'success'}])}},[signIn?.status])functiondismissToast(id: number){setToasts(prev=>prev.filter(t=>t.id!==id))}asyncfunctionhandleSubmit(e: React.FormEvent){e.preventDefault()if(!signIn)returnsetToasts([])toastIdRef.current=0awaitsignIn.password({identifier: email, password })if(signIn.status==='complete'){awaitsignIn.finalize({navigate: async({ session, decorateUrl })=>{if(session?.currentTask){router.push(decorateUrl(`/sign-in/tasks/${session.currentTask.key}`))return}router.push(decorateUrl('/'))},})}}return(<divclassName="flex min-h-screen items-center justify-center bg-zinc-50 dark:bg-black"><ToastContainertoasts={toasts}onDismiss={dismissToast}/><divclassName="w-full max-w-sm rounded-lg border border-zinc-200 bg-white p-8 shadow-sm dark:border-zinc-800 dark:bg-zinc-950"><h1className="mb-6 text-center text-2xl font-semibold text-zinc-900 dark:text-zinc-50">Signin</h1><formonSubmit={handleSubmit}className="flex flex-col gap-4"><divclassName="flex flex-col gap-1"><labelhtmlFor="email"className="text-sm font-medium text-zinc-700 dark:text-zinc-300">Email</label><inputid="email"type="email"value={email}onChange={(e)=>setEmail(e.target.value)}placeholder="you@example.com"className="rounded-md border border-zinc-300 px-3 py-2 text-sm text-zinc-900 placeholder:text-zinc-400 focus:border-zinc-500 focus:outline-none focus:ring-1 focus:ring-zinc-500 dark:border-zinc-700 dark:bg-zinc-900 dark:text-zinc-50"/>{errors?.fields?.identifier&&(<pclassName="text-sm text-red-600">{errors.fields.identifier.message}</p>)}</div><divclassName="flex flex-col gap-1"><labelhtmlFor="password"className="text-sm font-medium text-zinc-700 dark:text-zinc-300">Password</label><inputid="password"type="password"value={password}onChange={(e)=>setPassword(e.target.value)}placeholder="Password"className="rounded-md border border-zinc-300 px-3 py-2 text-sm text-zinc-900 placeholder:text-zinc-400 focus:border-zinc-500 focus:outline-none focus:ring-1 focus:ring-zinc-500 dark:border-zinc-700 dark:bg-zinc-900 dark:text-zinc-50"/>{errors?.fields?.password&&(<pclassName="text-sm text-red-600">{errors.fields.password.message}</p>)}</div>{errors?.global?.map((err,i)=>(<pkey={i}className="text-sm text-red-600">{err.longMessage}</p>))}<buttontype="submit"disabled={fetchStatus ==='fetching'}className="mt-2 rounded-md bg-zinc-900 px-4 py-2 text-sm font-medium text-white transition-colors hover:bg-zinc-800 disabled:opacity-50 dark:bg-zinc-50 dark:text-zinc-900 dark:hover:bg-zinc-200">{fetchStatus ==='fetching' ? 'Signing in...' : 'Sign in'}</button></form><pclassName="mt-4 text-center text-sm text-zinc-500">Don&apos;thaveanaccount?{' '}<Linkhref="/sign-up"className="font-medium text-zinc-900 hover:underline dark:text-zinc-50">Signup</Link></p></div></div>)}

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Summary by CodeRabbit

  • Bug Fixes

    • Fixed an issue where completion status could briefly show as complete during sign-in/sign-up while requests were still processing.
  • Improvements

    • Consolidated resource state handling and added batching to reduce unnecessary UI updates and ensure more consistent authentication status updates.

@changeset-bot

changeset-botBot commented Mar 4, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5bd33d4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
NameType
@clerk/clerk-jsPatch
@clerk/chrome-extensionPatch
@clerk/expoPatch

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

@vercel

vercelBot commented Mar 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
clerk-js-sandboxReadyReadyPreview, CommentMay 1, 2026 1:31am

Request Review

@nikosdouvlis

Copy link
Copy Markdown
Member

!snapshot

@clerk-cookie

Copy link
Copy Markdown
Collaborator

Hey @nikosdouvlis - the snapshot version command generated the following package versions:

PackageVersion
@clerk/agent-toolkit0.3.2-snapshot.v20260305105512
@clerk/astro3.0.2-snapshot.v20260305105512
@clerk/backend3.0.2-snapshot.v20260305105512
@clerk/chrome-extension3.0.2-snapshot.v20260305105512
@clerk/clerk-js6.0.1-snapshot.v20260305105512
@clerk/dev-cli0.1.1-snapshot.v20260305105512
@clerk/expo3.0.2-snapshot.v20260305105512
@clerk/expo-passkeys1.0.2-snapshot.v20260305105512
@clerk/express2.0.2-snapshot.v20260305105512
@clerk/fastify3.0.2-snapshot.v20260305105512
@clerk/hono0.0.4-snapshot.v20260305105512
@clerk/localizations4.0.1-snapshot.v20260305105512
@clerk/msw0.0.2-snapshot.v20260305105512
@clerk/nextjs7.0.2-snapshot.v20260305105512
@clerk/nuxt2.0.2-snapshot.v20260305105512
@clerk/react6.0.2-snapshot.v20260305105512
@clerk/react-router3.0.2-snapshot.v20260305105512
@clerk/shared4.0.1-snapshot.v20260305105512
@clerk/tanstack-react-start1.0.2-snapshot.v20260305105512
@clerk/testing2.0.2-snapshot.v20260305105512
@clerk/ui1.0.2-snapshot.v20260305105512
@clerk/upgrade2.0.2-snapshot.v20260305105512
@clerk/vue2.0.2-snapshot.v20260305105512

Tip: Use the snippet copy button below to quickly install the required packages.
@clerk/agent-toolkit

npm i @clerk/agent-toolkit@0.3.2-snapshot.v20260305105512 --save-exact

@clerk/astro

npm i @clerk/astro@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/backend

npm i @clerk/backend@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/chrome-extension

npm i @clerk/chrome-extension@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/clerk-js

npm i @clerk/clerk-js@6.0.1-snapshot.v20260305105512 --save-exact

@clerk/dev-cli

npm i @clerk/dev-cli@0.1.1-snapshot.v20260305105512 --save-exact

@clerk/expo

npm i @clerk/expo@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/expo-passkeys

npm i @clerk/expo-passkeys@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/express

npm i @clerk/express@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/fastify

npm i @clerk/fastify@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/hono

npm i @clerk/hono@0.0.4-snapshot.v20260305105512 --save-exact

@clerk/localizations

npm i @clerk/localizations@4.0.1-snapshot.v20260305105512 --save-exact

@clerk/msw

npm i @clerk/msw@0.0.2-snapshot.v20260305105512 --save-exact

@clerk/nextjs

npm i @clerk/nextjs@7.0.2-snapshot.v20260305105512 --save-exact

@clerk/nuxt

npm i @clerk/nuxt@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/react

npm i @clerk/react@6.0.2-snapshot.v20260305105512 --save-exact

@clerk/react-router

npm i @clerk/react-router@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/shared

npm i @clerk/shared@4.0.1-snapshot.v20260305105512 --save-exact

@clerk/tanstack-react-start

npm i @clerk/tanstack-react-start@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/testing

npm i @clerk/testing@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/ui

npm i @clerk/ui@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/upgrade

npm i @clerk/upgrade@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/vue

npm i @clerk/vue@2.0.2-snapshot.v20260305105512 --save-exact

@bratsos
bratsosforce-pushed the alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen branch from baab30c to 59c6aa4CompareMarch 5, 2026 18:31
@bratsos
bratsos marked this pull request as ready for review March 5, 2026 18:33
@coderabbitai

coderabbitaiBot commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 7ec07b01-a4f4-4323-a5d8-1ebe21587f3f

📥 Commits

Reviewing files that changed from the base of the PR and between 59c6aa4 and cb580f1.

📒 Files selected for processing (13)
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/__tests__/state.test.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/Waitlist.test.ts
  • packages/clerk-js/src/core/state.ts
  • packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts
  • packages/clerk-js/src/utils/runAsyncResourceTask.ts
🚧 Files skipped from review as they are similar to previous changes (7)
  • packages/clerk-js/src/core/resources/tests/SignIn.test.ts
  • packages/clerk-js/src/core/resources/tests/Waitlist.test.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/resources/SignUp.ts

📝 Walkthrough

Walkthrough

This PR consolidates resource state handling in the clerk/clerk-js package by replacing separate ResourceUpdate/ResourceError/ResourceFetch events with a single ResourceStateChange event whose payload includes resource, optional error, and optional fetchStatus. State handling was consolidated into a single onResourceStateChange handler that batches signal updates. Resources and client code were updated to emit the new event shape. runAsyncResourceTask was simplified to emit state-change events. A new signInFetchSignal export and additional tests covering batching and lifecycle edge cases were added.

Possibly related PRs

  • clerk/javascript PR 6549: initial fetch-status tracking that this change extends into a unified ResourceStateChange flow and signal batching.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title accurately and concisely summarizes the primary change: consolidating three separate resource events into a single unified event, which is the core refactoring goal of this PR.
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/clerk-js/src/core/resources/SignIn.ts (1)

1293-1312: ⚠️ Potential issue | 🟠 Major

finalize() can reject unexpectedly and mark the flow discardable before success

Line [1296] throws outside the try, so this method can reject instead of returning { error }. Also, setting #canBeDiscarded before setActive succeeds can prematurely allow null-resource replacement on failure.

Proposed fix
 async finalize(params?: SignInFutureFinalizeParams): Promise<{ error: ClerkError | null }> {
const { navigate } = params || {};
-- if (!this.#resource.createdSessionId) {- throw new Error('Cannot finalize sign-in without a created session.');- }
try {
+ if (!this.#resource.createdSessionId) {+ throw new Error('Cannot finalize sign-in without a created session.');+ }+
// Reload the client if the created session is not in the client's sessions. This can happen during modal SSO
// flows where the in-memory client does not have the created session.
if (SignIn.clerk.client && !SignIn.clerk.client.sessions.some(s => s.id === this.#resource.createdSessionId)) {
await SignIn.clerk.client.reload();
}
- this.#canBeDiscarded = true;
await SignIn.clerk.setActive({ session: this.#resource.createdSessionId, navigate });
+ this.#canBeDiscarded = true;
return { error: null };
} catch (err) {
return { error: err as ClerkError };
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/clerk-js/src/core/resources/SignIn.ts` around lines 1293 - 1312, The
finalize method currently throws if `#resource.createdSessionId` is missing and
sets `#canBeDiscarded` before setActive succeeds, so change finalize to return {
error } instead of throwing and only mark `#canBeDiscarded` true after
SignIn.clerk.setActive completes successfully: move the createdSessionId guard
and any throws into the try/catch (or convert them to an early return of {
error: new ClerkError(...) }) and relocate the assignment of
this.#canBeDiscarded to immediately after await SignIn.clerk.setActive(...)
succeeds; adjust error handling to return the caught error as ClerkError.
Reference finalize, this.#resource.createdSessionId, this.#canBeDiscarded,
SignIn.clerk.client.reload, and SignIn.clerk.setActive.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/clerk-js/src/core/resources/SignIn.ts`:
- Around line 1293-1312: The finalize method currently throws if
`#resource.createdSessionId` is missing and sets `#canBeDiscarded` before setActive
succeeds, so change finalize to return { error } instead of throwing and only
mark `#canBeDiscarded` true after SignIn.clerk.setActive completes successfully:
move the createdSessionId guard and any throws into the try/catch (or convert
them to an early return of { error: new ClerkError(...) }) and relocate the
assignment of this.#canBeDiscarded to immediately after await
SignIn.clerk.setActive(...) succeeds; adjust error handling to return the caught
error as ClerkError. Reference finalize, this.#resource.createdSessionId,
this.#canBeDiscarded, SignIn.clerk.client.reload, and SignIn.clerk.setActive.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: d47a6777-69a9-4390-a604-fcc13018fee0

📥 Commits

Reviewing files that changed from the base of the PR and between e13fc29 and 59c6aa4.

📒 Files selected for processing (13)
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/__tests__/state.test.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/Waitlist.test.ts
  • packages/clerk-js/src/core/state.ts
  • packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts
  • packages/clerk-js/src/utils/runAsyncResourceTask.ts

@bratsos

Copy link
Copy Markdown
ContributorAuthor

!snapshot

@bratsosbratsos closed this Mar 6, 2026
@bratsosbratsos reopened this Mar 6, 2026
@pkg-pr-new

pkg-pr-newBot commented Mar 6, 2026

Copy link
Copy Markdown

Open in StackBlitz

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@7980

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@7980

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@7980

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@7980

@clerk/dev-cli

npm i https://pkg.pr.new/@clerk/dev-cli@7980

@clerk/expo

npm i https://pkg.pr.new/@clerk/expo@7980

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@7980

@clerk/express

npm i https://pkg.pr.new/@clerk/express@7980

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@7980

@clerk/hono

npm i https://pkg.pr.new/@clerk/hono@7980

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@7980

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@7980

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@7980

@clerk/react

npm i https://pkg.pr.new/@clerk/react@7980

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@7980

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@7980

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@7980

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@7980

@clerk/ui

npm i https://pkg.pr.new/@clerk/ui@7980

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@7980

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@7980

commit: 5bd33d4

@dstaley

Copy link
Copy Markdown
Member

This looks great! I just have a few clarifying questions before I approve:

Consolidated three events into one. resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

I'm pretty sure I understand this, but just to state it again to make sure I understand: with this change we're now able to batch the resource/error/fetchStatus signal updates into a single update when those happen in a single call site. Meaning one event emit results in one signal update, but multiple events aren't batched in any way (which is totally fine, just making sure I understand). So, for example:

eventBus.emit('resource:state-change', { resource, error, fetchStatus })

triggers one signal update. I think I'm fine with this, but it does mean we no longer have the ability to update just the resource, or just the fetch status. As evidenced by this PR that's probably fine? But still worth calling out.

Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

This one I'm the most unsure of, because there's times when the resource identity isn't stable. However I can't think of an instance in which the resource identity will change during a method that's set fetchStatus to fetching so this is probably fine? Plus I think this is something we can easily revert if we discover that we do need this.

Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

The original intent here was to put the resource into a fetching status since we were doing something that should disable input (waiting for the navigation to be performed). I think it's fine to remove for now, and we can add back if we feel the need or see people doing things like isFinalizing to render specific UI during finalize().

@bratsos

bratsos commented Mar 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review!

This looks great! I just have a few clarifying questions before I approve:

Consolidated three events into one. resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

I'm pretty sure I understand this, but just to state it again to make sure I understand: with this change we're now able to batch the resource/error/fetchStatus signal updates into a single update when those happen in a single call site. Meaning one event emit results in one signal update, but multiple events aren't batched in any way (which is totally fine, just making sure I understand). So, for example:

eventBus.emit('resource:state-change', { resource, error, fetchStatus })

triggers one signal update. I think I'm fine with this, but it does mean we no longer have the ability to update just the resource, or just the fetch status. As evidenced by this PR that's probably fine? But still worth calling out.

Yeah exactly. One emit -> one handler -> one flush. Multiple emits are still independent.
Regarding losing the ability to update, the handler checks 'error' in payload and 'fetchStatus' in payload before writing each signal, so for example eventBus.emit('resource:state-change', { resource }) only writes the resource signal.

Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

This one I'm the most unsure of, because there's times when the resource identity isn't stable. However I can't think of an instance in which the resource identity will change during a method that's set fetchStatus to fetching so this is probably fine? Plus I think this is something we can easily revert if we discover that we do need this.

Yes that's correct, I think the only thing that would create a new instance mid-flight is a client refresh, which creates a new SignIn(null) but that would be blocked by shouldIgnoreNullUpdate, so it should be safe (and easy to revert if we find other cases).

Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

The original intent here was to put the resource into a fetching status since we were doing something that should disable input (waiting for the navigation to be performed). I think it's fine to remove for now, and we can add back if we feel the need or see people doing things like isFinalizing to render specific UI during finalize().

Makes sense. If we need that in the future, we could add a dedicated isFinalizing flag than reusing fetchStatusto keep semantics stay clean (fetchStatus = API call in progress, isFinalizing = session activation in progress). But agreed, no need to add it right now.

@bratsos
bratsosforce-pushed the alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen branch from 59c6aa4 to cb580f1CompareMarch 11, 2026 17:45
@github-actions

Copy link
Copy Markdown
Contributor

Hello 👋

We currently close PRs after 60 days of inactivity. It's been 50 days since the last update here. If we missed this PR, please reply here. Otherwise, we'll close this PR in 10 days.

Thanks for being a part of the Clerk community! 🙏

@github-actions

Copy link
Copy Markdown
Contributor

Hello 👋

We currently close PRs after 60 days of inactivity. It's been 50 days since the last update here. If we missed this PR, please reply here. Otherwise, we'll close this PR in 10 days.

Thanks for being a part of the Clerk community! 🙏

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@bratsos@nikosdouvlis@clerk-cookie@dstaley@jacekradko
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

refactor(clerk-js): Consolidate resource events into single event - #7980

Closed
bratsos wants to merge 2 commits into
mainfrom
alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen
Closed

refactor(clerk-js): Consolidate resource events into single event#7980
bratsos wants to merge 2 commits into
mainfrom
alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen

Conversation

@bratsos

@bratsosbratsos commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

Description

Resource state changes were spread across three separate events: resource:update, resource:error, and resource:fetch. Each one triggered its own signal write in State, which meant React could re-render between them. After an API call completed, React would briefly see the updated resource while fetchStatus was still 'fetching', because the resource and fetch-status updates arrived independently.

Even after consolidating into one event, a second source of inconsistency remained: fromJSON emits a resource-only event mid-flight (during an API call), which updated the resource signal before runAsyncResourceTask emitted the completion event with fetchStatus: 'idle'.

A third source came from finalize(), which used runAsyncResourceTask and set fetchStatus: 'fetching' on an already-complete resource even though it's just calling setActive, a session-level operation.

How we fixed it

Three changes, each addressing one source of inconsistency:

  1. Consolidated three events into one.resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

  2. Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

  3. Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

Demo

(showcasing the finalize() fix)

BeforeAfter
resource-events-before.mov
resource-events-after.mov
Sample code used for the demo
exportdefaultfunctionSignInPage(){const{ signIn, errors, fetchStatus }=useSignIn()constrouter=useRouter()const[email,setEmail]=useState('')const[password,setPassword]=useState('')const[toasts,setToasts]=useState<Toast[]>([])consttoastIdRef=useRef(0)useEffect(()=>{if(fetchStatus==='fetching'){setToasts(prev=>[...prev,{id: ++toastIdRef.current,message: 'Verifying your credentials...',type: 'info'}])}if(fetchStatus==='idle'){setToasts(prev=>prev.filter(t=>t.type!=='info'))}},[fetchStatus])useEffect(()=>{if(signIn?.status==='complete'){setToasts(prev=>[...prev,{id: ++toastIdRef.current,message: 'Welcome back! Redirecting...',type: 'success'}])}},[signIn?.status])functiondismissToast(id: number){setToasts(prev=>prev.filter(t=>t.id!==id))}asyncfunctionhandleSubmit(e: React.FormEvent){e.preventDefault()if(!signIn)returnsetToasts([])toastIdRef.current=0awaitsignIn.password({identifier: email, password })if(signIn.status==='complete'){awaitsignIn.finalize({navigate: async({ session, decorateUrl })=>{if(session?.currentTask){router.push(decorateUrl(`/sign-in/tasks/${session.currentTask.key}`))return}router.push(decorateUrl('/'))},})}}return(<divclassName="flex min-h-screen items-center justify-center bg-zinc-50 dark:bg-black"><ToastContainertoasts={toasts}onDismiss={dismissToast}/><divclassName="w-full max-w-sm rounded-lg border border-zinc-200 bg-white p-8 shadow-sm dark:border-zinc-800 dark:bg-zinc-950"><h1className="mb-6 text-center text-2xl font-semibold text-zinc-900 dark:text-zinc-50">Signin</h1><formonSubmit={handleSubmit}className="flex flex-col gap-4"><divclassName="flex flex-col gap-1"><labelhtmlFor="email"className="text-sm font-medium text-zinc-700 dark:text-zinc-300">Email</label><inputid="email"type="email"value={email}onChange={(e)=>setEmail(e.target.value)}placeholder="you@example.com"className="rounded-md border border-zinc-300 px-3 py-2 text-sm text-zinc-900 placeholder:text-zinc-400 focus:border-zinc-500 focus:outline-none focus:ring-1 focus:ring-zinc-500 dark:border-zinc-700 dark:bg-zinc-900 dark:text-zinc-50"/>{errors?.fields?.identifier&&(<pclassName="text-sm text-red-600">{errors.fields.identifier.message}</p>)}</div><divclassName="flex flex-col gap-1"><labelhtmlFor="password"className="text-sm font-medium text-zinc-700 dark:text-zinc-300">Password</label><inputid="password"type="password"value={password}onChange={(e)=>setPassword(e.target.value)}placeholder="Password"className="rounded-md border border-zinc-300 px-3 py-2 text-sm text-zinc-900 placeholder:text-zinc-400 focus:border-zinc-500 focus:outline-none focus:ring-1 focus:ring-zinc-500 dark:border-zinc-700 dark:bg-zinc-900 dark:text-zinc-50"/>{errors?.fields?.password&&(<pclassName="text-sm text-red-600">{errors.fields.password.message}</p>)}</div>{errors?.global?.map((err,i)=>(<pkey={i}className="text-sm text-red-600">{err.longMessage}</p>))}<buttontype="submit"disabled={fetchStatus ==='fetching'}className="mt-2 rounded-md bg-zinc-900 px-4 py-2 text-sm font-medium text-white transition-colors hover:bg-zinc-800 disabled:opacity-50 dark:bg-zinc-50 dark:text-zinc-900 dark:hover:bg-zinc-200">{fetchStatus ==='fetching' ? 'Signing in...' : 'Sign in'}</button></form><pclassName="mt-4 text-center text-sm text-zinc-500">Don&apos;thaveanaccount?{' '}<Linkhref="/sign-up"className="font-medium text-zinc-900 hover:underline dark:text-zinc-50">Signup</Link></p></div></div>)}

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Summary by CodeRabbit

  • Bug Fixes

    • Fixed an issue where completion status could briefly show as complete during sign-in/sign-up while requests were still processing.
  • Improvements

    • Consolidated resource state handling and added batching to reduce unnecessary UI updates and ensure more consistent authentication status updates.

@changeset-bot

changeset-botBot commented Mar 4, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5bd33d4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
NameType
@clerk/clerk-jsPatch
@clerk/chrome-extensionPatch
@clerk/expoPatch

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

@vercel

vercelBot commented Mar 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
clerk-js-sandboxReadyReadyPreview, CommentMay 1, 2026 1:31am

Request Review

@nikosdouvlis

Copy link
Copy Markdown
Member

!snapshot

@clerk-cookie

Copy link
Copy Markdown
Collaborator

Hey @nikosdouvlis - the snapshot version command generated the following package versions:

PackageVersion
@clerk/agent-toolkit0.3.2-snapshot.v20260305105512
@clerk/astro3.0.2-snapshot.v20260305105512
@clerk/backend3.0.2-snapshot.v20260305105512
@clerk/chrome-extension3.0.2-snapshot.v20260305105512
@clerk/clerk-js6.0.1-snapshot.v20260305105512
@clerk/dev-cli0.1.1-snapshot.v20260305105512
@clerk/expo3.0.2-snapshot.v20260305105512
@clerk/expo-passkeys1.0.2-snapshot.v20260305105512
@clerk/express2.0.2-snapshot.v20260305105512
@clerk/fastify3.0.2-snapshot.v20260305105512
@clerk/hono0.0.4-snapshot.v20260305105512
@clerk/localizations4.0.1-snapshot.v20260305105512
@clerk/msw0.0.2-snapshot.v20260305105512
@clerk/nextjs7.0.2-snapshot.v20260305105512
@clerk/nuxt2.0.2-snapshot.v20260305105512
@clerk/react6.0.2-snapshot.v20260305105512
@clerk/react-router3.0.2-snapshot.v20260305105512
@clerk/shared4.0.1-snapshot.v20260305105512
@clerk/tanstack-react-start1.0.2-snapshot.v20260305105512
@clerk/testing2.0.2-snapshot.v20260305105512
@clerk/ui1.0.2-snapshot.v20260305105512
@clerk/upgrade2.0.2-snapshot.v20260305105512
@clerk/vue2.0.2-snapshot.v20260305105512

Tip: Use the snippet copy button below to quickly install the required packages.
@clerk/agent-toolkit

npm i @clerk/agent-toolkit@0.3.2-snapshot.v20260305105512 --save-exact

@clerk/astro

npm i @clerk/astro@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/backend

npm i @clerk/backend@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/chrome-extension

npm i @clerk/chrome-extension@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/clerk-js

npm i @clerk/clerk-js@6.0.1-snapshot.v20260305105512 --save-exact

@clerk/dev-cli

npm i @clerk/dev-cli@0.1.1-snapshot.v20260305105512 --save-exact

@clerk/expo

npm i @clerk/expo@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/expo-passkeys

npm i @clerk/expo-passkeys@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/express

npm i @clerk/express@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/fastify

npm i @clerk/fastify@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/hono

npm i @clerk/hono@0.0.4-snapshot.v20260305105512 --save-exact

@clerk/localizations

npm i @clerk/localizations@4.0.1-snapshot.v20260305105512 --save-exact

@clerk/msw

npm i @clerk/msw@0.0.2-snapshot.v20260305105512 --save-exact

@clerk/nextjs

npm i @clerk/nextjs@7.0.2-snapshot.v20260305105512 --save-exact

@clerk/nuxt

npm i @clerk/nuxt@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/react

npm i @clerk/react@6.0.2-snapshot.v20260305105512 --save-exact

@clerk/react-router

npm i @clerk/react-router@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/shared

npm i @clerk/shared@4.0.1-snapshot.v20260305105512 --save-exact

@clerk/tanstack-react-start

npm i @clerk/tanstack-react-start@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/testing

npm i @clerk/testing@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/ui

npm i @clerk/ui@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/upgrade

npm i @clerk/upgrade@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/vue

npm i @clerk/vue@2.0.2-snapshot.v20260305105512 --save-exact

@bratsos
bratsosforce-pushed the alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen branch from baab30c to 59c6aa4CompareMarch 5, 2026 18:31
@bratsos
bratsos marked this pull request as ready for review March 5, 2026 18:33
@coderabbitai

coderabbitaiBot commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 7ec07b01-a4f4-4323-a5d8-1ebe21587f3f

📥 Commits

Reviewing files that changed from the base of the PR and between 59c6aa4 and cb580f1.

📒 Files selected for processing (13)
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/__tests__/state.test.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/Waitlist.test.ts
  • packages/clerk-js/src/core/state.ts
  • packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts
  • packages/clerk-js/src/utils/runAsyncResourceTask.ts
🚧 Files skipped from review as they are similar to previous changes (7)
  • packages/clerk-js/src/core/resources/tests/SignIn.test.ts
  • packages/clerk-js/src/core/resources/tests/Waitlist.test.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/resources/SignUp.ts

📝 Walkthrough

Walkthrough

This PR consolidates resource state handling in the clerk/clerk-js package by replacing separate ResourceUpdate/ResourceError/ResourceFetch events with a single ResourceStateChange event whose payload includes resource, optional error, and optional fetchStatus. State handling was consolidated into a single onResourceStateChange handler that batches signal updates. Resources and client code were updated to emit the new event shape. runAsyncResourceTask was simplified to emit state-change events. A new signInFetchSignal export and additional tests covering batching and lifecycle edge cases were added.

Possibly related PRs

  • clerk/javascript PR 6549: initial fetch-status tracking that this change extends into a unified ResourceStateChange flow and signal batching.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title accurately and concisely summarizes the primary change: consolidating three separate resource events into a single unified event, which is the core refactoring goal of this PR.
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/clerk-js/src/core/resources/SignIn.ts (1)

1293-1312: ⚠️ Potential issue | 🟠 Major

finalize() can reject unexpectedly and mark the flow discardable before success

Line [1296] throws outside the try, so this method can reject instead of returning { error }. Also, setting #canBeDiscarded before setActive succeeds can prematurely allow null-resource replacement on failure.

Proposed fix
 async finalize(params?: SignInFutureFinalizeParams): Promise<{ error: ClerkError | null }> {
const { navigate } = params || {};
-- if (!this.#resource.createdSessionId) {- throw new Error('Cannot finalize sign-in without a created session.');- }
try {
+ if (!this.#resource.createdSessionId) {+ throw new Error('Cannot finalize sign-in without a created session.');+ }+
// Reload the client if the created session is not in the client's sessions. This can happen during modal SSO
// flows where the in-memory client does not have the created session.
if (SignIn.clerk.client && !SignIn.clerk.client.sessions.some(s => s.id === this.#resource.createdSessionId)) {
await SignIn.clerk.client.reload();
}
- this.#canBeDiscarded = true;
await SignIn.clerk.setActive({ session: this.#resource.createdSessionId, navigate });
+ this.#canBeDiscarded = true;
return { error: null };
} catch (err) {
return { error: err as ClerkError };
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/clerk-js/src/core/resources/SignIn.ts` around lines 1293 - 1312, The
finalize method currently throws if `#resource.createdSessionId` is missing and
sets `#canBeDiscarded` before setActive succeeds, so change finalize to return {
error } instead of throwing and only mark `#canBeDiscarded` true after
SignIn.clerk.setActive completes successfully: move the createdSessionId guard
and any throws into the try/catch (or convert them to an early return of {
error: new ClerkError(...) }) and relocate the assignment of
this.#canBeDiscarded to immediately after await SignIn.clerk.setActive(...)
succeeds; adjust error handling to return the caught error as ClerkError.
Reference finalize, this.#resource.createdSessionId, this.#canBeDiscarded,
SignIn.clerk.client.reload, and SignIn.clerk.setActive.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/clerk-js/src/core/resources/SignIn.ts`:
- Around line 1293-1312: The finalize method currently throws if
`#resource.createdSessionId` is missing and sets `#canBeDiscarded` before setActive
succeeds, so change finalize to return { error } instead of throwing and only
mark `#canBeDiscarded` true after SignIn.clerk.setActive completes successfully:
move the createdSessionId guard and any throws into the try/catch (or convert
them to an early return of { error: new ClerkError(...) }) and relocate the
assignment of this.#canBeDiscarded to immediately after await
SignIn.clerk.setActive(...) succeeds; adjust error handling to return the caught
error as ClerkError. Reference finalize, this.#resource.createdSessionId,
this.#canBeDiscarded, SignIn.clerk.client.reload, and SignIn.clerk.setActive.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: d47a6777-69a9-4390-a604-fcc13018fee0

📥 Commits

Reviewing files that changed from the base of the PR and between e13fc29 and 59c6aa4.

📒 Files selected for processing (13)
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/__tests__/state.test.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/Waitlist.test.ts
  • packages/clerk-js/src/core/state.ts
  • packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts
  • packages/clerk-js/src/utils/runAsyncResourceTask.ts

@bratsos

Copy link
Copy Markdown
ContributorAuthor

!snapshot

@bratsosbratsos closed this Mar 6, 2026
@bratsosbratsos reopened this Mar 6, 2026
@pkg-pr-new

pkg-pr-newBot commented Mar 6, 2026

Copy link
Copy Markdown

Open in StackBlitz

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@7980

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@7980

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@7980

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@7980

@clerk/dev-cli

npm i https://pkg.pr.new/@clerk/dev-cli@7980

@clerk/expo

npm i https://pkg.pr.new/@clerk/expo@7980

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@7980

@clerk/express

npm i https://pkg.pr.new/@clerk/express@7980

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@7980

@clerk/hono

npm i https://pkg.pr.new/@clerk/hono@7980

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@7980

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@7980

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@7980

@clerk/react

npm i https://pkg.pr.new/@clerk/react@7980

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@7980

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@7980

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@7980

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@7980

@clerk/ui

npm i https://pkg.pr.new/@clerk/ui@7980

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@7980

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@7980

commit: 5bd33d4

@dstaley

Copy link
Copy Markdown
Member

This looks great! I just have a few clarifying questions before I approve:

Consolidated three events into one. resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

I'm pretty sure I understand this, but just to state it again to make sure I understand: with this change we're now able to batch the resource/error/fetchStatus signal updates into a single update when those happen in a single call site. Meaning one event emit results in one signal update, but multiple events aren't batched in any way (which is totally fine, just making sure I understand). So, for example:

eventBus.emit('resource:state-change', { resource, error, fetchStatus })

triggers one signal update. I think I'm fine with this, but it does mean we no longer have the ability to update just the resource, or just the fetch status. As evidenced by this PR that's probably fine? But still worth calling out.

Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

This one I'm the most unsure of, because there's times when the resource identity isn't stable. However I can't think of an instance in which the resource identity will change during a method that's set fetchStatus to fetching so this is probably fine? Plus I think this is something we can easily revert if we discover that we do need this.

Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

The original intent here was to put the resource into a fetching status since we were doing something that should disable input (waiting for the navigation to be performed). I think it's fine to remove for now, and we can add back if we feel the need or see people doing things like isFinalizing to render specific UI during finalize().

@bratsos

bratsos commented Mar 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review!

This looks great! I just have a few clarifying questions before I approve:

Consolidated three events into one. resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

I'm pretty sure I understand this, but just to state it again to make sure I understand: with this change we're now able to batch the resource/error/fetchStatus signal updates into a single update when those happen in a single call site. Meaning one event emit results in one signal update, but multiple events aren't batched in any way (which is totally fine, just making sure I understand). So, for example:

eventBus.emit('resource:state-change', { resource, error, fetchStatus })

triggers one signal update. I think I'm fine with this, but it does mean we no longer have the ability to update just the resource, or just the fetch status. As evidenced by this PR that's probably fine? But still worth calling out.

Yeah exactly. One emit -> one handler -> one flush. Multiple emits are still independent.
Regarding losing the ability to update, the handler checks 'error' in payload and 'fetchStatus' in payload before writing each signal, so for example eventBus.emit('resource:state-change', { resource }) only writes the resource signal.

Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

This one I'm the most unsure of, because there's times when the resource identity isn't stable. However I can't think of an instance in which the resource identity will change during a method that's set fetchStatus to fetching so this is probably fine? Plus I think this is something we can easily revert if we discover that we do need this.

Yes that's correct, I think the only thing that would create a new instance mid-flight is a client refresh, which creates a new SignIn(null) but that would be blocked by shouldIgnoreNullUpdate, so it should be safe (and easy to revert if we find other cases).

Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

The original intent here was to put the resource into a fetching status since we were doing something that should disable input (waiting for the navigation to be performed). I think it's fine to remove for now, and we can add back if we feel the need or see people doing things like isFinalizing to render specific UI during finalize().

Makes sense. If we need that in the future, we could add a dedicated isFinalizing flag than reusing fetchStatusto keep semantics stay clean (fetchStatus = API call in progress, isFinalizing = session activation in progress). But agreed, no need to add it right now.

@bratsos
bratsosforce-pushed the alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen branch from 59c6aa4 to cb580f1CompareMarch 11, 2026 17:45
@github-actions

Copy link
Copy Markdown
Contributor

Hello 👋

We currently close PRs after 60 days of inactivity. It's been 50 days since the last update here. If we missed this PR, please reply here. Otherwise, we'll close this PR in 10 days.

Thanks for being a part of the Clerk community! 🙏

@github-actions

Copy link
Copy Markdown
Contributor

Hello 👋

We currently close PRs after 60 days of inactivity. It's been 50 days since the last update here. If we missed this PR, please reply here. Otherwise, we'll close this PR in 10 days.

Thanks for being a part of the Clerk community! 🙏

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@bratsos@nikosdouvlis@clerk-cookie@dstaley@jacekradko
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

refactor(clerk-js): Consolidate resource events into single event - #7980

Closed
bratsos wants to merge 2 commits into
mainfrom
alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen
Closed

refactor(clerk-js): Consolidate resource events into single event#7980
bratsos wants to merge 2 commits into
mainfrom
alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen

Conversation

@bratsos

@bratsosbratsos commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

Description

Resource state changes were spread across three separate events: resource:update, resource:error, and resource:fetch. Each one triggered its own signal write in State, which meant React could re-render between them. After an API call completed, React would briefly see the updated resource while fetchStatus was still 'fetching', because the resource and fetch-status updates arrived independently.

Even after consolidating into one event, a second source of inconsistency remained: fromJSON emits a resource-only event mid-flight (during an API call), which updated the resource signal before runAsyncResourceTask emitted the completion event with fetchStatus: 'idle'.

A third source came from finalize(), which used runAsyncResourceTask and set fetchStatus: 'fetching' on an already-complete resource even though it's just calling setActive, a session-level operation.

How we fixed it

Three changes, each addressing one source of inconsistency:

  1. Consolidated three events into one.resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

  2. Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

  3. Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

Demo

(showcasing the finalize() fix)

BeforeAfter
resource-events-before.mov
resource-events-after.mov
Sample code used for the demo
exportdefaultfunctionSignInPage(){const{ signIn, errors, fetchStatus }=useSignIn()constrouter=useRouter()const[email,setEmail]=useState('')const[password,setPassword]=useState('')const[toasts,setToasts]=useState<Toast[]>([])consttoastIdRef=useRef(0)useEffect(()=>{if(fetchStatus==='fetching'){setToasts(prev=>[...prev,{id: ++toastIdRef.current,message: 'Verifying your credentials...',type: 'info'}])}if(fetchStatus==='idle'){setToasts(prev=>prev.filter(t=>t.type!=='info'))}},[fetchStatus])useEffect(()=>{if(signIn?.status==='complete'){setToasts(prev=>[...prev,{id: ++toastIdRef.current,message: 'Welcome back! Redirecting...',type: 'success'}])}},[signIn?.status])functiondismissToast(id: number){setToasts(prev=>prev.filter(t=>t.id!==id))}asyncfunctionhandleSubmit(e: React.FormEvent){e.preventDefault()if(!signIn)returnsetToasts([])toastIdRef.current=0awaitsignIn.password({identifier: email, password })if(signIn.status==='complete'){awaitsignIn.finalize({navigate: async({ session, decorateUrl })=>{if(session?.currentTask){router.push(decorateUrl(`/sign-in/tasks/${session.currentTask.key}`))return}router.push(decorateUrl('/'))},})}}return(<divclassName="flex min-h-screen items-center justify-center bg-zinc-50 dark:bg-black"><ToastContainertoasts={toasts}onDismiss={dismissToast}/><divclassName="w-full max-w-sm rounded-lg border border-zinc-200 bg-white p-8 shadow-sm dark:border-zinc-800 dark:bg-zinc-950"><h1className="mb-6 text-center text-2xl font-semibold text-zinc-900 dark:text-zinc-50">Signin</h1><formonSubmit={handleSubmit}className="flex flex-col gap-4"><divclassName="flex flex-col gap-1"><labelhtmlFor="email"className="text-sm font-medium text-zinc-700 dark:text-zinc-300">Email</label><inputid="email"type="email"value={email}onChange={(e)=>setEmail(e.target.value)}placeholder="you@example.com"className="rounded-md border border-zinc-300 px-3 py-2 text-sm text-zinc-900 placeholder:text-zinc-400 focus:border-zinc-500 focus:outline-none focus:ring-1 focus:ring-zinc-500 dark:border-zinc-700 dark:bg-zinc-900 dark:text-zinc-50"/>{errors?.fields?.identifier&&(<pclassName="text-sm text-red-600">{errors.fields.identifier.message}</p>)}</div><divclassName="flex flex-col gap-1"><labelhtmlFor="password"className="text-sm font-medium text-zinc-700 dark:text-zinc-300">Password</label><inputid="password"type="password"value={password}onChange={(e)=>setPassword(e.target.value)}placeholder="Password"className="rounded-md border border-zinc-300 px-3 py-2 text-sm text-zinc-900 placeholder:text-zinc-400 focus:border-zinc-500 focus:outline-none focus:ring-1 focus:ring-zinc-500 dark:border-zinc-700 dark:bg-zinc-900 dark:text-zinc-50"/>{errors?.fields?.password&&(<pclassName="text-sm text-red-600">{errors.fields.password.message}</p>)}</div>{errors?.global?.map((err,i)=>(<pkey={i}className="text-sm text-red-600">{err.longMessage}</p>))}<buttontype="submit"disabled={fetchStatus ==='fetching'}className="mt-2 rounded-md bg-zinc-900 px-4 py-2 text-sm font-medium text-white transition-colors hover:bg-zinc-800 disabled:opacity-50 dark:bg-zinc-50 dark:text-zinc-900 dark:hover:bg-zinc-200">{fetchStatus ==='fetching' ? 'Signing in...' : 'Sign in'}</button></form><pclassName="mt-4 text-center text-sm text-zinc-500">Don&apos;thaveanaccount?{' '}<Linkhref="/sign-up"className="font-medium text-zinc-900 hover:underline dark:text-zinc-50">Signup</Link></p></div></div>)}

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Summary by CodeRabbit

  • Bug Fixes

    • Fixed an issue where completion status could briefly show as complete during sign-in/sign-up while requests were still processing.
  • Improvements

    • Consolidated resource state handling and added batching to reduce unnecessary UI updates and ensure more consistent authentication status updates.

@changeset-bot

changeset-botBot commented Mar 4, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5bd33d4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
NameType
@clerk/clerk-jsPatch
@clerk/chrome-extensionPatch
@clerk/expoPatch

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

@vercel

vercelBot commented Mar 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
clerk-js-sandboxReadyReadyPreview, CommentMay 1, 2026 1:31am

Request Review

@nikosdouvlis

Copy link
Copy Markdown
Member

!snapshot

@clerk-cookie

Copy link
Copy Markdown
Collaborator

Hey @nikosdouvlis - the snapshot version command generated the following package versions:

PackageVersion
@clerk/agent-toolkit0.3.2-snapshot.v20260305105512
@clerk/astro3.0.2-snapshot.v20260305105512
@clerk/backend3.0.2-snapshot.v20260305105512
@clerk/chrome-extension3.0.2-snapshot.v20260305105512
@clerk/clerk-js6.0.1-snapshot.v20260305105512
@clerk/dev-cli0.1.1-snapshot.v20260305105512
@clerk/expo3.0.2-snapshot.v20260305105512
@clerk/expo-passkeys1.0.2-snapshot.v20260305105512
@clerk/express2.0.2-snapshot.v20260305105512
@clerk/fastify3.0.2-snapshot.v20260305105512
@clerk/hono0.0.4-snapshot.v20260305105512
@clerk/localizations4.0.1-snapshot.v20260305105512
@clerk/msw0.0.2-snapshot.v20260305105512
@clerk/nextjs7.0.2-snapshot.v20260305105512
@clerk/nuxt2.0.2-snapshot.v20260305105512
@clerk/react6.0.2-snapshot.v20260305105512
@clerk/react-router3.0.2-snapshot.v20260305105512
@clerk/shared4.0.1-snapshot.v20260305105512
@clerk/tanstack-react-start1.0.2-snapshot.v20260305105512
@clerk/testing2.0.2-snapshot.v20260305105512
@clerk/ui1.0.2-snapshot.v20260305105512
@clerk/upgrade2.0.2-snapshot.v20260305105512
@clerk/vue2.0.2-snapshot.v20260305105512

Tip: Use the snippet copy button below to quickly install the required packages.
@clerk/agent-toolkit

npm i @clerk/agent-toolkit@0.3.2-snapshot.v20260305105512 --save-exact

@clerk/astro

npm i @clerk/astro@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/backend

npm i @clerk/backend@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/chrome-extension

npm i @clerk/chrome-extension@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/clerk-js

npm i @clerk/clerk-js@6.0.1-snapshot.v20260305105512 --save-exact

@clerk/dev-cli

npm i @clerk/dev-cli@0.1.1-snapshot.v20260305105512 --save-exact

@clerk/expo

npm i @clerk/expo@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/expo-passkeys

npm i @clerk/expo-passkeys@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/express

npm i @clerk/express@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/fastify

npm i @clerk/fastify@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/hono

npm i @clerk/hono@0.0.4-snapshot.v20260305105512 --save-exact

@clerk/localizations

npm i @clerk/localizations@4.0.1-snapshot.v20260305105512 --save-exact

@clerk/msw

npm i @clerk/msw@0.0.2-snapshot.v20260305105512 --save-exact

@clerk/nextjs

npm i @clerk/nextjs@7.0.2-snapshot.v20260305105512 --save-exact

@clerk/nuxt

npm i @clerk/nuxt@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/react

npm i @clerk/react@6.0.2-snapshot.v20260305105512 --save-exact

@clerk/react-router

npm i @clerk/react-router@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/shared

npm i @clerk/shared@4.0.1-snapshot.v20260305105512 --save-exact

@clerk/tanstack-react-start

npm i @clerk/tanstack-react-start@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/testing

npm i @clerk/testing@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/ui

npm i @clerk/ui@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/upgrade

npm i @clerk/upgrade@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/vue

npm i @clerk/vue@2.0.2-snapshot.v20260305105512 --save-exact

@bratsos
bratsosforce-pushed the alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen branch from baab30c to 59c6aa4CompareMarch 5, 2026 18:31
@bratsos
bratsos marked this pull request as ready for review March 5, 2026 18:33
@coderabbitai

coderabbitaiBot commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 7ec07b01-a4f4-4323-a5d8-1ebe21587f3f

📥 Commits

Reviewing files that changed from the base of the PR and between 59c6aa4 and cb580f1.

📒 Files selected for processing (13)
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/__tests__/state.test.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/Waitlist.test.ts
  • packages/clerk-js/src/core/state.ts
  • packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts
  • packages/clerk-js/src/utils/runAsyncResourceTask.ts
🚧 Files skipped from review as they are similar to previous changes (7)
  • packages/clerk-js/src/core/resources/tests/SignIn.test.ts
  • packages/clerk-js/src/core/resources/tests/Waitlist.test.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/resources/SignUp.ts

📝 Walkthrough

Walkthrough

This PR consolidates resource state handling in the clerk/clerk-js package by replacing separate ResourceUpdate/ResourceError/ResourceFetch events with a single ResourceStateChange event whose payload includes resource, optional error, and optional fetchStatus. State handling was consolidated into a single onResourceStateChange handler that batches signal updates. Resources and client code were updated to emit the new event shape. runAsyncResourceTask was simplified to emit state-change events. A new signInFetchSignal export and additional tests covering batching and lifecycle edge cases were added.

Possibly related PRs

  • clerk/javascript PR 6549: initial fetch-status tracking that this change extends into a unified ResourceStateChange flow and signal batching.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title accurately and concisely summarizes the primary change: consolidating three separate resource events into a single unified event, which is the core refactoring goal of this PR.
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/clerk-js/src/core/resources/SignIn.ts (1)

1293-1312: ⚠️ Potential issue | 🟠 Major

finalize() can reject unexpectedly and mark the flow discardable before success

Line [1296] throws outside the try, so this method can reject instead of returning { error }. Also, setting #canBeDiscarded before setActive succeeds can prematurely allow null-resource replacement on failure.

Proposed fix
 async finalize(params?: SignInFutureFinalizeParams): Promise<{ error: ClerkError | null }> {
const { navigate } = params || {};
-- if (!this.#resource.createdSessionId) {- throw new Error('Cannot finalize sign-in without a created session.');- }
try {
+ if (!this.#resource.createdSessionId) {+ throw new Error('Cannot finalize sign-in without a created session.');+ }+
// Reload the client if the created session is not in the client's sessions. This can happen during modal SSO
// flows where the in-memory client does not have the created session.
if (SignIn.clerk.client && !SignIn.clerk.client.sessions.some(s => s.id === this.#resource.createdSessionId)) {
await SignIn.clerk.client.reload();
}
- this.#canBeDiscarded = true;
await SignIn.clerk.setActive({ session: this.#resource.createdSessionId, navigate });
+ this.#canBeDiscarded = true;
return { error: null };
} catch (err) {
return { error: err as ClerkError };
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/clerk-js/src/core/resources/SignIn.ts` around lines 1293 - 1312, The
finalize method currently throws if `#resource.createdSessionId` is missing and
sets `#canBeDiscarded` before setActive succeeds, so change finalize to return {
error } instead of throwing and only mark `#canBeDiscarded` true after
SignIn.clerk.setActive completes successfully: move the createdSessionId guard
and any throws into the try/catch (or convert them to an early return of {
error: new ClerkError(...) }) and relocate the assignment of
this.#canBeDiscarded to immediately after await SignIn.clerk.setActive(...)
succeeds; adjust error handling to return the caught error as ClerkError.
Reference finalize, this.#resource.createdSessionId, this.#canBeDiscarded,
SignIn.clerk.client.reload, and SignIn.clerk.setActive.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/clerk-js/src/core/resources/SignIn.ts`:
- Around line 1293-1312: The finalize method currently throws if
`#resource.createdSessionId` is missing and sets `#canBeDiscarded` before setActive
succeeds, so change finalize to return { error } instead of throwing and only
mark `#canBeDiscarded` true after SignIn.clerk.setActive completes successfully:
move the createdSessionId guard and any throws into the try/catch (or convert
them to an early return of { error: new ClerkError(...) }) and relocate the
assignment of this.#canBeDiscarded to immediately after await
SignIn.clerk.setActive(...) succeeds; adjust error handling to return the caught
error as ClerkError. Reference finalize, this.#resource.createdSessionId,
this.#canBeDiscarded, SignIn.clerk.client.reload, and SignIn.clerk.setActive.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: d47a6777-69a9-4390-a604-fcc13018fee0

📥 Commits

Reviewing files that changed from the base of the PR and between e13fc29 and 59c6aa4.

📒 Files selected for processing (13)
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/__tests__/state.test.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/Waitlist.test.ts
  • packages/clerk-js/src/core/state.ts
  • packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts
  • packages/clerk-js/src/utils/runAsyncResourceTask.ts

@bratsos

Copy link
Copy Markdown
ContributorAuthor

!snapshot

@bratsosbratsos closed this Mar 6, 2026
@bratsosbratsos reopened this Mar 6, 2026
@pkg-pr-new

pkg-pr-newBot commented Mar 6, 2026

Copy link
Copy Markdown

Open in StackBlitz

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@7980

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@7980

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@7980

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@7980

@clerk/dev-cli

npm i https://pkg.pr.new/@clerk/dev-cli@7980

@clerk/expo

npm i https://pkg.pr.new/@clerk/expo@7980

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@7980

@clerk/express

npm i https://pkg.pr.new/@clerk/express@7980

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@7980

@clerk/hono

npm i https://pkg.pr.new/@clerk/hono@7980

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@7980

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@7980

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@7980

@clerk/react

npm i https://pkg.pr.new/@clerk/react@7980

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@7980

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@7980

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@7980

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@7980

@clerk/ui

npm i https://pkg.pr.new/@clerk/ui@7980

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@7980

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@7980

commit: 5bd33d4

@dstaley

Copy link
Copy Markdown
Member

This looks great! I just have a few clarifying questions before I approve:

Consolidated three events into one. resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

I'm pretty sure I understand this, but just to state it again to make sure I understand: with this change we're now able to batch the resource/error/fetchStatus signal updates into a single update when those happen in a single call site. Meaning one event emit results in one signal update, but multiple events aren't batched in any way (which is totally fine, just making sure I understand). So, for example:

eventBus.emit('resource:state-change', { resource, error, fetchStatus })

triggers one signal update. I think I'm fine with this, but it does mean we no longer have the ability to update just the resource, or just the fetch status. As evidenced by this PR that's probably fine? But still worth calling out.

Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

This one I'm the most unsure of, because there's times when the resource identity isn't stable. However I can't think of an instance in which the resource identity will change during a method that's set fetchStatus to fetching so this is probably fine? Plus I think this is something we can easily revert if we discover that we do need this.

Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

The original intent here was to put the resource into a fetching status since we were doing something that should disable input (waiting for the navigation to be performed). I think it's fine to remove for now, and we can add back if we feel the need or see people doing things like isFinalizing to render specific UI during finalize().

@bratsos

bratsos commented Mar 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review!

This looks great! I just have a few clarifying questions before I approve:

Consolidated three events into one. resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

I'm pretty sure I understand this, but just to state it again to make sure I understand: with this change we're now able to batch the resource/error/fetchStatus signal updates into a single update when those happen in a single call site. Meaning one event emit results in one signal update, but multiple events aren't batched in any way (which is totally fine, just making sure I understand). So, for example:

eventBus.emit('resource:state-change', { resource, error, fetchStatus })

triggers one signal update. I think I'm fine with this, but it does mean we no longer have the ability to update just the resource, or just the fetch status. As evidenced by this PR that's probably fine? But still worth calling out.

Yeah exactly. One emit -> one handler -> one flush. Multiple emits are still independent.
Regarding losing the ability to update, the handler checks 'error' in payload and 'fetchStatus' in payload before writing each signal, so for example eventBus.emit('resource:state-change', { resource }) only writes the resource signal.

Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

This one I'm the most unsure of, because there's times when the resource identity isn't stable. However I can't think of an instance in which the resource identity will change during a method that's set fetchStatus to fetching so this is probably fine? Plus I think this is something we can easily revert if we discover that we do need this.

Yes that's correct, I think the only thing that would create a new instance mid-flight is a client refresh, which creates a new SignIn(null) but that would be blocked by shouldIgnoreNullUpdate, so it should be safe (and easy to revert if we find other cases).

Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

The original intent here was to put the resource into a fetching status since we were doing something that should disable input (waiting for the navigation to be performed). I think it's fine to remove for now, and we can add back if we feel the need or see people doing things like isFinalizing to render specific UI during finalize().

Makes sense. If we need that in the future, we could add a dedicated isFinalizing flag than reusing fetchStatusto keep semantics stay clean (fetchStatus = API call in progress, isFinalizing = session activation in progress). But agreed, no need to add it right now.

@bratsos
bratsosforce-pushed the alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen branch from 59c6aa4 to cb580f1CompareMarch 11, 2026 17:45
@github-actions

Copy link
Copy Markdown
Contributor

Hello 👋

We currently close PRs after 60 days of inactivity. It's been 50 days since the last update here. If we missed this PR, please reply here. Otherwise, we'll close this PR in 10 days.

Thanks for being a part of the Clerk community! 🙏

@github-actions

Copy link
Copy Markdown
Contributor

Hello 👋

We currently close PRs after 60 days of inactivity. It's been 50 days since the last update here. If we missed this PR, please reply here. Otherwise, we'll close this PR in 10 days.

Thanks for being a part of the Clerk community! 🙏

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@bratsos@nikosdouvlis@clerk-cookie@dstaley@jacekradko
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

refactor(clerk-js): Consolidate resource events into single event - #7980

Closed
bratsos wants to merge 2 commits into
mainfrom
alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen
Closed

refactor(clerk-js): Consolidate resource events into single event#7980
bratsos wants to merge 2 commits into
mainfrom
alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen

Conversation

@bratsos

@bratsosbratsos commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

Description

Resource state changes were spread across three separate events: resource:update, resource:error, and resource:fetch. Each one triggered its own signal write in State, which meant React could re-render between them. After an API call completed, React would briefly see the updated resource while fetchStatus was still 'fetching', because the resource and fetch-status updates arrived independently.

Even after consolidating into one event, a second source of inconsistency remained: fromJSON emits a resource-only event mid-flight (during an API call), which updated the resource signal before runAsyncResourceTask emitted the completion event with fetchStatus: 'idle'.

A third source came from finalize(), which used runAsyncResourceTask and set fetchStatus: 'fetching' on an already-complete resource even though it's just calling setActive, a session-level operation.

How we fixed it

Three changes, each addressing one source of inconsistency:

  1. Consolidated three events into one.resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

  2. Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

  3. Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

Demo

(showcasing the finalize() fix)

BeforeAfter
resource-events-before.mov
resource-events-after.mov
Sample code used for the demo
exportdefaultfunctionSignInPage(){const{ signIn, errors, fetchStatus }=useSignIn()constrouter=useRouter()const[email,setEmail]=useState('')const[password,setPassword]=useState('')const[toasts,setToasts]=useState<Toast[]>([])consttoastIdRef=useRef(0)useEffect(()=>{if(fetchStatus==='fetching'){setToasts(prev=>[...prev,{id: ++toastIdRef.current,message: 'Verifying your credentials...',type: 'info'}])}if(fetchStatus==='idle'){setToasts(prev=>prev.filter(t=>t.type!=='info'))}},[fetchStatus])useEffect(()=>{if(signIn?.status==='complete'){setToasts(prev=>[...prev,{id: ++toastIdRef.current,message: 'Welcome back! Redirecting...',type: 'success'}])}},[signIn?.status])functiondismissToast(id: number){setToasts(prev=>prev.filter(t=>t.id!==id))}asyncfunctionhandleSubmit(e: React.FormEvent){e.preventDefault()if(!signIn)returnsetToasts([])toastIdRef.current=0awaitsignIn.password({identifier: email, password })if(signIn.status==='complete'){awaitsignIn.finalize({navigate: async({ session, decorateUrl })=>{if(session?.currentTask){router.push(decorateUrl(`/sign-in/tasks/${session.currentTask.key}`))return}router.push(decorateUrl('/'))},})}}return(<divclassName="flex min-h-screen items-center justify-center bg-zinc-50 dark:bg-black"><ToastContainertoasts={toasts}onDismiss={dismissToast}/><divclassName="w-full max-w-sm rounded-lg border border-zinc-200 bg-white p-8 shadow-sm dark:border-zinc-800 dark:bg-zinc-950"><h1className="mb-6 text-center text-2xl font-semibold text-zinc-900 dark:text-zinc-50">Signin</h1><formonSubmit={handleSubmit}className="flex flex-col gap-4"><divclassName="flex flex-col gap-1"><labelhtmlFor="email"className="text-sm font-medium text-zinc-700 dark:text-zinc-300">Email</label><inputid="email"type="email"value={email}onChange={(e)=>setEmail(e.target.value)}placeholder="you@example.com"className="rounded-md border border-zinc-300 px-3 py-2 text-sm text-zinc-900 placeholder:text-zinc-400 focus:border-zinc-500 focus:outline-none focus:ring-1 focus:ring-zinc-500 dark:border-zinc-700 dark:bg-zinc-900 dark:text-zinc-50"/>{errors?.fields?.identifier&&(<pclassName="text-sm text-red-600">{errors.fields.identifier.message}</p>)}</div><divclassName="flex flex-col gap-1"><labelhtmlFor="password"className="text-sm font-medium text-zinc-700 dark:text-zinc-300">Password</label><inputid="password"type="password"value={password}onChange={(e)=>setPassword(e.target.value)}placeholder="Password"className="rounded-md border border-zinc-300 px-3 py-2 text-sm text-zinc-900 placeholder:text-zinc-400 focus:border-zinc-500 focus:outline-none focus:ring-1 focus:ring-zinc-500 dark:border-zinc-700 dark:bg-zinc-900 dark:text-zinc-50"/>{errors?.fields?.password&&(<pclassName="text-sm text-red-600">{errors.fields.password.message}</p>)}</div>{errors?.global?.map((err,i)=>(<pkey={i}className="text-sm text-red-600">{err.longMessage}</p>))}<buttontype="submit"disabled={fetchStatus ==='fetching'}className="mt-2 rounded-md bg-zinc-900 px-4 py-2 text-sm font-medium text-white transition-colors hover:bg-zinc-800 disabled:opacity-50 dark:bg-zinc-50 dark:text-zinc-900 dark:hover:bg-zinc-200">{fetchStatus ==='fetching' ? 'Signing in...' : 'Sign in'}</button></form><pclassName="mt-4 text-center text-sm text-zinc-500">Don&apos;thaveanaccount?{' '}<Linkhref="/sign-up"className="font-medium text-zinc-900 hover:underline dark:text-zinc-50">Signup</Link></p></div></div>)}

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Summary by CodeRabbit

  • Bug Fixes

    • Fixed an issue where completion status could briefly show as complete during sign-in/sign-up while requests were still processing.
  • Improvements

    • Consolidated resource state handling and added batching to reduce unnecessary UI updates and ensure more consistent authentication status updates.

@changeset-bot

changeset-botBot commented Mar 4, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5bd33d4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
NameType
@clerk/clerk-jsPatch
@clerk/chrome-extensionPatch
@clerk/expoPatch

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

@vercel

vercelBot commented Mar 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
clerk-js-sandboxReadyReadyPreview, CommentMay 1, 2026 1:31am

Request Review

@nikosdouvlis

Copy link
Copy Markdown
Member

!snapshot

@clerk-cookie

Copy link
Copy Markdown
Collaborator

Hey @nikosdouvlis - the snapshot version command generated the following package versions:

PackageVersion
@clerk/agent-toolkit0.3.2-snapshot.v20260305105512
@clerk/astro3.0.2-snapshot.v20260305105512
@clerk/backend3.0.2-snapshot.v20260305105512
@clerk/chrome-extension3.0.2-snapshot.v20260305105512
@clerk/clerk-js6.0.1-snapshot.v20260305105512
@clerk/dev-cli0.1.1-snapshot.v20260305105512
@clerk/expo3.0.2-snapshot.v20260305105512
@clerk/expo-passkeys1.0.2-snapshot.v20260305105512
@clerk/express2.0.2-snapshot.v20260305105512
@clerk/fastify3.0.2-snapshot.v20260305105512
@clerk/hono0.0.4-snapshot.v20260305105512
@clerk/localizations4.0.1-snapshot.v20260305105512
@clerk/msw0.0.2-snapshot.v20260305105512
@clerk/nextjs7.0.2-snapshot.v20260305105512
@clerk/nuxt2.0.2-snapshot.v20260305105512
@clerk/react6.0.2-snapshot.v20260305105512
@clerk/react-router3.0.2-snapshot.v20260305105512
@clerk/shared4.0.1-snapshot.v20260305105512
@clerk/tanstack-react-start1.0.2-snapshot.v20260305105512
@clerk/testing2.0.2-snapshot.v20260305105512
@clerk/ui1.0.2-snapshot.v20260305105512
@clerk/upgrade2.0.2-snapshot.v20260305105512
@clerk/vue2.0.2-snapshot.v20260305105512

Tip: Use the snippet copy button below to quickly install the required packages.
@clerk/agent-toolkit

npm i @clerk/agent-toolkit@0.3.2-snapshot.v20260305105512 --save-exact

@clerk/astro

npm i @clerk/astro@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/backend

npm i @clerk/backend@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/chrome-extension

npm i @clerk/chrome-extension@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/clerk-js

npm i @clerk/clerk-js@6.0.1-snapshot.v20260305105512 --save-exact

@clerk/dev-cli

npm i @clerk/dev-cli@0.1.1-snapshot.v20260305105512 --save-exact

@clerk/expo

npm i @clerk/expo@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/expo-passkeys

npm i @clerk/expo-passkeys@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/express

npm i @clerk/express@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/fastify

npm i @clerk/fastify@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/hono

npm i @clerk/hono@0.0.4-snapshot.v20260305105512 --save-exact

@clerk/localizations

npm i @clerk/localizations@4.0.1-snapshot.v20260305105512 --save-exact

@clerk/msw

npm i @clerk/msw@0.0.2-snapshot.v20260305105512 --save-exact

@clerk/nextjs

npm i @clerk/nextjs@7.0.2-snapshot.v20260305105512 --save-exact

@clerk/nuxt

npm i @clerk/nuxt@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/react

npm i @clerk/react@6.0.2-snapshot.v20260305105512 --save-exact

@clerk/react-router

npm i @clerk/react-router@3.0.2-snapshot.v20260305105512 --save-exact

@clerk/shared

npm i @clerk/shared@4.0.1-snapshot.v20260305105512 --save-exact

@clerk/tanstack-react-start

npm i @clerk/tanstack-react-start@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/testing

npm i @clerk/testing@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/ui

npm i @clerk/ui@1.0.2-snapshot.v20260305105512 --save-exact

@clerk/upgrade

npm i @clerk/upgrade@2.0.2-snapshot.v20260305105512 --save-exact

@clerk/vue

npm i @clerk/vue@2.0.2-snapshot.v20260305105512 --save-exact

@bratsos
bratsosforce-pushed the alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen branch from baab30c to 59c6aa4CompareMarch 5, 2026 18:31
@bratsos
bratsos marked this pull request as ready for review March 5, 2026 18:33
@coderabbitai

coderabbitaiBot commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 7ec07b01-a4f4-4323-a5d8-1ebe21587f3f

📥 Commits

Reviewing files that changed from the base of the PR and between 59c6aa4 and cb580f1.

📒 Files selected for processing (13)
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/__tests__/state.test.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/Waitlist.test.ts
  • packages/clerk-js/src/core/state.ts
  • packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts
  • packages/clerk-js/src/utils/runAsyncResourceTask.ts
🚧 Files skipped from review as they are similar to previous changes (7)
  • packages/clerk-js/src/core/resources/tests/SignIn.test.ts
  • packages/clerk-js/src/core/resources/tests/Waitlist.test.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/resources/SignUp.ts

📝 Walkthrough

Walkthrough

This PR consolidates resource state handling in the clerk/clerk-js package by replacing separate ResourceUpdate/ResourceError/ResourceFetch events with a single ResourceStateChange event whose payload includes resource, optional error, and optional fetchStatus. State handling was consolidated into a single onResourceStateChange handler that batches signal updates. Resources and client code were updated to emit the new event shape. runAsyncResourceTask was simplified to emit state-change events. A new signInFetchSignal export and additional tests covering batching and lifecycle edge cases were added.

Possibly related PRs

  • clerk/javascript PR 6549: initial fetch-status tracking that this change extends into a unified ResourceStateChange flow and signal batching.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title accurately and concisely summarizes the primary change: consolidating three separate resource events into a single unified event, which is the core refactoring goal of this PR.
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/clerk-js/src/core/resources/SignIn.ts (1)

1293-1312: ⚠️ Potential issue | 🟠 Major

finalize() can reject unexpectedly and mark the flow discardable before success

Line [1296] throws outside the try, so this method can reject instead of returning { error }. Also, setting #canBeDiscarded before setActive succeeds can prematurely allow null-resource replacement on failure.

Proposed fix
 async finalize(params?: SignInFutureFinalizeParams): Promise<{ error: ClerkError | null }> {
const { navigate } = params || {};
-- if (!this.#resource.createdSessionId) {- throw new Error('Cannot finalize sign-in without a created session.');- }
try {
+ if (!this.#resource.createdSessionId) {+ throw new Error('Cannot finalize sign-in without a created session.');+ }+
// Reload the client if the created session is not in the client's sessions. This can happen during modal SSO
// flows where the in-memory client does not have the created session.
if (SignIn.clerk.client && !SignIn.clerk.client.sessions.some(s => s.id === this.#resource.createdSessionId)) {
await SignIn.clerk.client.reload();
}
- this.#canBeDiscarded = true;
await SignIn.clerk.setActive({ session: this.#resource.createdSessionId, navigate });
+ this.#canBeDiscarded = true;
return { error: null };
} catch (err) {
return { error: err as ClerkError };
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/clerk-js/src/core/resources/SignIn.ts` around lines 1293 - 1312, The
finalize method currently throws if `#resource.createdSessionId` is missing and
sets `#canBeDiscarded` before setActive succeeds, so change finalize to return {
error } instead of throwing and only mark `#canBeDiscarded` true after
SignIn.clerk.setActive completes successfully: move the createdSessionId guard
and any throws into the try/catch (or convert them to an early return of {
error: new ClerkError(...) }) and relocate the assignment of
this.#canBeDiscarded to immediately after await SignIn.clerk.setActive(...)
succeeds; adjust error handling to return the caught error as ClerkError.
Reference finalize, this.#resource.createdSessionId, this.#canBeDiscarded,
SignIn.clerk.client.reload, and SignIn.clerk.setActive.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/clerk-js/src/core/resources/SignIn.ts`:
- Around line 1293-1312: The finalize method currently throws if
`#resource.createdSessionId` is missing and sets `#canBeDiscarded` before setActive
succeeds, so change finalize to return { error } instead of throwing and only
mark `#canBeDiscarded` true after SignIn.clerk.setActive completes successfully:
move the createdSessionId guard and any throws into the try/catch (or convert
them to an early return of { error: new ClerkError(...) }) and relocate the
assignment of this.#canBeDiscarded to immediately after await
SignIn.clerk.setActive(...) succeeds; adjust error handling to return the caught
error as ClerkError. Reference finalize, this.#resource.createdSessionId,
this.#canBeDiscarded, SignIn.clerk.client.reload, and SignIn.clerk.setActive.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: d47a6777-69a9-4390-a604-fcc13018fee0

📥 Commits

Reviewing files that changed from the base of the PR and between e13fc29 and 59c6aa4.

📒 Files selected for processing (13)
  • .changeset/fiery-games-pick.md
  • packages/clerk-js/src/core/__tests__/state.test.ts
  • packages/clerk-js/src/core/events.ts
  • packages/clerk-js/src/core/resources/Client.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
  • packages/clerk-js/src/core/resources/Waitlist.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/Waitlist.test.ts
  • packages/clerk-js/src/core/state.ts
  • packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts
  • packages/clerk-js/src/utils/runAsyncResourceTask.ts

@bratsos

Copy link
Copy Markdown
ContributorAuthor

!snapshot

@bratsosbratsos closed this Mar 6, 2026
@bratsosbratsos reopened this Mar 6, 2026
@pkg-pr-new

pkg-pr-newBot commented Mar 6, 2026

Copy link
Copy Markdown

Open in StackBlitz

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@7980

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@7980

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@7980

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@7980

@clerk/dev-cli

npm i https://pkg.pr.new/@clerk/dev-cli@7980

@clerk/expo

npm i https://pkg.pr.new/@clerk/expo@7980

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@7980

@clerk/express

npm i https://pkg.pr.new/@clerk/express@7980

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@7980

@clerk/hono

npm i https://pkg.pr.new/@clerk/hono@7980

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@7980

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@7980

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@7980

@clerk/react

npm i https://pkg.pr.new/@clerk/react@7980

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@7980

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@7980

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@7980

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@7980

@clerk/ui

npm i https://pkg.pr.new/@clerk/ui@7980

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@7980

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@7980

commit: 5bd33d4

@dstaley

Copy link
Copy Markdown
Member

This looks great! I just have a few clarifying questions before I approve:

Consolidated three events into one. resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

I'm pretty sure I understand this, but just to state it again to make sure I understand: with this change we're now able to batch the resource/error/fetchStatus signal updates into a single update when those happen in a single call site. Meaning one event emit results in one signal update, but multiple events aren't batched in any way (which is totally fine, just making sure I understand). So, for example:

eventBus.emit('resource:state-change', { resource, error, fetchStatus })

triggers one signal update. I think I'm fine with this, but it does mean we no longer have the ability to update just the resource, or just the fetch status. As evidenced by this PR that's probably fine? But still worth calling out.

Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

This one I'm the most unsure of, because there's times when the resource identity isn't stable. However I can't think of an instance in which the resource identity will change during a method that's set fetchStatus to fetching so this is probably fine? Plus I think this is something we can easily revert if we discover that we do need this.

Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

The original intent here was to put the resource into a fetching status since we were doing something that should disable input (waiting for the navigation to be performed). I think it's fine to remove for now, and we can add back if we feel the need or see people doing things like isFinalizing to render specific UI during finalize().

@bratsos

bratsos commented Mar 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review!

This looks great! I just have a few clarifying questions before I approve:

Consolidated three events into one. resource:update, resource:error, and resource:fetch are now a single resource:state-change event with optional error and fetchStatus fields. The State handler wraps all signal writes in startBatch()/endBatch() from alien-signals so they flush as one notification.

I'm pretty sure I understand this, but just to state it again to make sure I understand: with this change we're now able to batch the resource/error/fetchStatus signal updates into a single update when those happen in a single call site. Meaning one event emit results in one signal update, but multiple events aren't batched in any way (which is totally fine, just making sure I understand). So, for example:

eventBus.emit('resource:state-change', { resource, error, fetchStatus })

triggers one signal update. I think I'm fine with this, but it does mean we no longer have the ability to update just the resource, or just the fetch status. As evidenced by this PR that's probably fine? But still worth calling out.

Yeah exactly. One emit -> one handler -> one flush. Multiple emits are still independent.
Regarding losing the ability to update, the handler checks 'error' in payload and 'fetchStatus' in payload before writing each signal, so for example eventBus.emit('resource:state-change', { resource }) only writes the resource signal.

Skip resource-only events while fetching. When State receives a resource-only event (no fetchStatus, no error) and the current fetchStatus is 'fetching', it skips the signal write entirely. Since fromJSON mutates the resource in place, the completion event carries the same already-updated instance.

This one I'm the most unsure of, because there's times when the resource identity isn't stable. However I can't think of an instance in which the resource identity will change during a method that's set fetchStatus to fetching so this is probably fine? Plus I think this is something we can easily revert if we discover that we do need this.

Yes that's correct, I think the only thing that would create a new instance mid-flight is a client refresh, which creates a new SignIn(null) but that would be blocked by shouldIgnoreNullUpdate, so it should be safe (and easy to revert if we find other cases).

Removed runAsyncResourceTask from finalize(). Like reset(), finalize() now handles its own async flow without emitting fetchStatus. The sign-in/sign-up is already complete; setActive is a session-level operation that shouldn't affect sign-in/sign-up fetch status.

The original intent here was to put the resource into a fetching status since we were doing something that should disable input (waiting for the navigation to be performed). I think it's fine to remove for now, and we can add back if we feel the need or see people doing things like isFinalizing to render specific UI during finalize().

Makes sense. If we need that in the future, we could add a dedicated isFinalizing flag than reusing fetchStatusto keep semantics stay clean (fetchStatus = API call in progress, isFinalizing = session activation in progress). But agreed, no need to add it right now.

@bratsos
bratsosforce-pushed the alexbratsos/user-4373-signal-update-for-resource-and-fetchstatus-can-happen branch from 59c6aa4 to cb580f1CompareMarch 11, 2026 17:45
@github-actions

Copy link
Copy Markdown
Contributor

Hello 👋

We currently close PRs after 60 days of inactivity. It's been 50 days since the last update here. If we missed this PR, please reply here. Otherwise, we'll close this PR in 10 days.

Thanks for being a part of the Clerk community! 🙏

@github-actions

Copy link
Copy Markdown
Contributor

Hello 👋

We currently close PRs after 60 days of inactivity. It's been 50 days since the last update here. If we missed this PR, please reply here. Otherwise, we'll close this PR in 10 days.

Thanks for being a part of the Clerk community! 🙏

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@bratsos@nikosdouvlis@clerk-cookie@dstaley@jacekradko