Skip to content

fix(js): preserve SignInFuture reference after resetSignIn #9071

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
FrancoKaddour wants to merge 1 commit into
base: main
Choose a base branch
from
+41 −1
Open

fix(js): preserve SignInFuture reference after resetSignIn #9071

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-signin-future-stale-ref-after-reset.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/clerk-js': patch
---

Fix `useSignIn()` returning a stale `SignInFuture` after `resetSignIn()`. When the next sign-in attempt starts, the existing instance is now updated in-place rather than replaced, preserving the reference held by active hooks.
2 changes: 1 addition & 1 deletion packages/clerk-js/src/core/resources/Client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ export class Client extends BaseResource implements ClientResource {
this.signUp = new SignUp(data.sign_up);
}

if (data.sign_in && this.signIn instanceof SignIn && this.signIn.id === data.sign_in.id) {
if (data.sign_in && this.signIn instanceof SignIn && (this.signIn.id === data.sign_in.id || !this.signIn.id)) {

@coderabbitai coderabbitai Bot Jul 2, 2026 •
edited
Loading

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if resetSignUp + fromJSON exhibits the same stale-instance issue as resetSignIn did
rg -n -A5 'resetSignUp' packages/clerk-js/src/core/resources/Client.ts
rg -n 'this.signUp.id ===' packages/clerk-js/src/core/resources/Client.ts

Repository: clerk/javascript

Length of output: 552

🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,220p' packages/clerk-js/src/core/resources/Client.ts

Repository: clerk/javascript

Length of output: 6391

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -A8 -B4 'class SignUp|class SignIn|__internal_updateFromJSON|useSignUp|useSignIn' packages/clerk-js/src/core/resources packages/clerk-js/src -g '!**/dist/**'

Repository: clerk/javascript

Length of output: 28599

🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,220p' packages/clerk-js/src/core/resources/SignUp.ts
sed -n '500,760p' packages/clerk-js/src/core/resources/SignUp.ts
sed -n '200,280p' packages/clerk-js/src/core/resources/__tests__/Client.test.ts

Repository: clerk/javascript

Length of output: 19406

Align SignUp reuse with the SignIn fix

sign_up still only reuses the existing instance when ids match. After resetSignUp(), this.signUp.id is falsy, so the next fromJSON() with a new attempt id replaces the instance and drops the __internal_future reference that useSignUp() depends on. Add the same || !this.signUp.id guard here, plus a reset→new-attempt test.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/clerk-js/src/core/resources/Client.ts` at line 152, The SignUp reuse
logic in Client.fromJSON is still too strict and replaces the existing SignUp
instance after resetSignUp(), which drops the __internal_future used by
useSignUp(). Update the sign_up branch to mirror the SignIn handling by reusing
this.signUp when the ids match or when this.signUp.id is falsy, using the
existing SignUp and fromJSON paths. Add a test covering resetSignUp() followed
by a new attempt id to ensure the same SignUp instance is preserved and its
internal future remains intact.

@FrancoKaddour FrancoKaddour Jul 2, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good catch. The signUp side is already covered by #9060 which adds the same || !this.signUp.id guard for that path — once that lands both flows will be protected.

@coderabbitai coderabbitai Bot Jul 2, 2026

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.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
gh pr view 9060 --repo clerk/javascript --json title,body,state,url 2>&1 | head -100

Length of output: 4839

@FrancoKaddour Confirmed — I checked #9060 and it does add the identical || !this.signUp.id guard to the sign_up branch in Client.fromJSON, addressing the same stale SignUpFuture issue for useSignUp(). Once that PR merges, both the signIn and signUp reuse paths will be consistent. No action needed here; I'll leave this as a non-blocking note for tracking purposes.

(っ˘̩╭╮˘̩)っ

🧬 Code Graph Analysis Results

packages/clerk-js/src/core/resources/Client.ts

Lines 31-33 (sign-up/sign-in attempt instances live on Client)

  sessions: Session[] = [];
  signUp: SignUpResource = new SignUp();
  signIn: SignInResource = new SignIn();

Lines 141-156 (fromJSON in-place update logic; sign-in already uses the || !this.signIn.id guard, sign-up does not in this file)

  fromJSON(data: ClientJSON | ClientJSONSnapshot | null): this {
    if (data) {
      this.id = data.id;
      this.sessions = (data.sessions || []).map(s => new Session(s));

      if (data.sign_up && this.signUp instanceof SignUp && this.signUp.id === data.sign_up.id) {
        this.signUp.__internal_updateFromJSON(data.sign_up);
      } else {
        this.signUp = new SignUp(data.sign_up);
      }

      if (data.sign_in && this.signIn instanceof SignIn && (this.signIn.id === data.sign_in.id || !this.signIn.id)) {
        this.signIn.__internal_updateFromJSON(data.sign_in);
      } else {
        this.signIn = new SignIn(data.sign_in);
      }

      this.lastActiveSessionId = data.last_active_session_id;
      this.captchaBypass = data.captcha_bypass || false;
      this.cookieExpiresAt = data.cookie_expires_at ? unixEpochToDate(data.cookie_expires_at) : null;
      this.lastAuthenticationStrategy = data.last_authentication_strategy || null;
      this.createdAt = unixEpochToDate(data.created_at || undefined);
      this.updatedAt = unixEpochToDate(data.updated_at || undefined);
    }

    return this;
  }

packages/clerk-js/src/core/resources/SignUp.ts

Line-range: 77-610 (relevant behavior: SignUp tracks id and populates it from API responses; this is what the Client.fromJSON check relies on)

export class SignUp extends BaseResource implements SignUpResource {
  pathRoot = '/client/sign_ups';

  id: string | undefined;
  ...
  constructor(data: SignUpJSON | SignUpJSONSnapshot | null = null) {
    super();
    this.fromJSON(data);
  }
  ...
  protected fromJSON(data: SignUpJSON | SignUpJSONSnapshot | null): this {
    if (data) {
      this.id = data.id;
      ...
    }

    eventBus.emit('resource:update', { resource: this });
    return this;
  }
  ...
}

Docstring-style summary of the relevant parts (from the same snippet range):

/**
 * SignUpResource identity:
 * - `id: string | undefined` (set from API payloads).
 * - `constructor(data)` calls `fromJSON(data)`.
 * - `fromJSON(...)` sets `this.id = data.id` when `data` is present and emits `resource:update`.
 *
 * Parameters:
 * - data: SignUpJSON | SignUpJSONSnapshot | null
 *
 * Returns:
 * - this (SignUp)
 *
 * Exceptions:
 * - none shown in the provided excerpt.
 */
🧠 Learnings used 
Learnt from: wobsoriano
Repo: clerk/javascript PR: 7883
File: packages/backend/src/api/__tests__/M2MTokenApi.test.ts:414-419
Timestamp: 2026-02-24T18:03:27.067Z
Learning: In TypeScript files within the clerk/javascript repository, enforce explicit return type annotations for exported functions and public APIs, but allow internal test helper functions without explicit return types. This helps maintain API clarity and type safety for consumers while not burdening test helpers that are not part of the public surface.

this.signIn.__internal_updateFromJSON(data.sign_in);
} else {
this.signIn = new SignIn(data.sign_in);
Expand Down
35 changes: 35 additions & 0 deletions packages/clerk-js/src/core/resources/__tests__/Client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,41 @@ describe('Client Singleton', () => {
expect(client.signIn.status).toBe('needs_second_factor');
});

it('preserves sign in identity when fromJSON receives a new sign_in after reset', () => {
const user = createUser({ first_name: 'John', last_name: 'Doe', id: 'user_1' });
const session = createSession({ id: 'session_1' }, user);
const initialClientJSON: ClientJSON = {
object: 'client',
id: 'test_id',
status: 'active',
last_active_session_id: 'test_session_id',
sign_in: createSignIn({ id: 'test_sign_in_id', status: 'needs_first_factor' }, user),
sign_up: createSignUp({ id: 'test_sign_up_id', status: 'missing_requirements' }),
sessions: [session],
created_at: Date.now() - 1000,
updated_at: Date.now(),
} as any;

// @ts-expect-error We cannot mess with the singleton when tests are running in parallel
const client = new Client(initialClientJSON);

client.resetSignIn();
const signInAfterReset = client.signIn;
expect(signInAfterReset.id).toBeUndefined();

client.fromJSON({
...initialClientJSON,
sign_in: createSignIn({ id: 'test_sign_in_id_v2', status: 'needs_first_factor', identifier: 'test@example.com' }, user),
updated_at: Date.now() + 1000,
});

// The same SignIn instance from after the reset is reused (preserving its SignInFuture
// reference so useSignIn() hooks stay valid across the reset → new attempt transition).
expect(client.signIn).toBe(signInAfterReset);
expect(client.signIn.id).toBe('test_sign_in_id_v2');
expect(client.signIn.identifier).toBe('test@example.com');
});

it('replaces sign up and sign in identity when fromJSON receives new ids', () => {
const user = createUser({ first_name: 'John', last_name: 'Doe', id: 'user_1' });
const session = createSession({ id: 'session_1' }, user);
Expand Down