Skip to content

test(clerk-js): add tests for null resource update behavior#7753

Merged
jacekradko merged 2 commits intomainfrom
jacek/user-4372-verify-null-resource-updates-when-previous-resource-is-not
Feb 11, 2026
Merged

test(clerk-js): add tests for null resource update behavior#7753
jacekradko merged 2 commits intomainfrom
jacek/user-4372-verify-null-resource-updates-when-previous-resource-is-not

Conversation

@jacekradko
Copy link
Member

@jacekradko jacekradko commented Feb 3, 2026

Summary

Adds tests for the shouldIgnoreNullUpdate mechanism in the State class to verify the behavior described in USER-4372.

Investigation Findings

The shouldIgnoreNullUpdate function at packages/clerk-js/src/core/state.ts:117-119 is designed to prevent null resource updates from propagating when the previous resource has not been finalized:

function shouldIgnoreNullUpdate(previousResource, newResource) {
  return !newResource?.id && previousResource && previousResource.__internal_future?.canBeDiscarded === false;
}

Tests verify that the mechanism works correctly:

  1. ✅ Allows first resource update when previous resource is null
  2. ✅ Ignores null update when previous resource exists and canBeDiscarded === false
  3. ✅ Allows null update after finalize() sets canBeDiscarded = true
  4. ✅ Allows updates when new resource has an id (not a null update)
  5. ✅ Handles rapid successive updates correctly
  6. ✅ Handles updates with same instance correctly

Key Insight: Timing Issue in finalize()

The reported flash of the default sign-up form likely occurs due to the order of operations in finalize():

async finalize(...) {
  this.#canBeDiscarded = true;  // ← Set BEFORE setActive
  await SignUp.clerk.setActive({ session: this.#resource.createdSessionId, navigate });
}

The sequence:

  1. User verifies email → sign up is complete
  2. User/code calls finalize()
  3. canBeDiscarded is set to true immediately
  4. setActive() is called
  5. In Next.js, setActive() triggers router.refresh() or navigation
  6. During refresh, client is re-fetched with sign_up: null
  7. Because canBeDiscarded is already true, the null update is NOT ignored
  8. Signal updates with empty SignUp → FLASH of default form
  9. Navigation completes

Recommended Follow-up

If the flash occurs before finalize() is called (as the issue title suggests), then either:

  1. There's a race condition where previousResource is unexpectedly null
  2. Something in Next.js is causing a full page reload that resets module-level signals

Consider adding debug logging to capture:

  • When shouldIgnoreNullUpdate is called
  • What previousResource and canBeDiscarded values are at that moment
  • The exact timing of the flash in relation to these events

Test plan

  • pnpm test passes for new tests
  • Tests cover the scenarios described in USER-4372

Summary by CodeRabbit

  • Tests
    • Added comprehensive tests for sign-up and sign-in state flows, covering null vs non-null updates, behavior after finalize-like flows and resets, resource replacement, rapid successive updates, same-instance updates, event signaling, and test isolation/restoration.
  • Chores
    • Added a small changeset file for release bookkeeping.

@vercel
Copy link

vercel bot commented Feb 3, 2026

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

Project Deployment Actions Updated (UTC)
clerk-js-sandbox Ready Ready Preview, Comment Feb 10, 2026 10:50pm

Request Review

@changeset-bot
Copy link

changeset-bot bot commented Feb 3, 2026

🦋 Changeset detected

Latest commit: 553d6a9

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

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

@pkg-pr-new
Copy link

pkg-pr-new bot commented Feb 3, 2026

Open in StackBlitz

@clerk/agent-toolkit

npm i https://pkg.pr.new/@clerk/agent-toolkit@7753

@clerk/astro

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

@clerk/backend

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

@clerk/chrome-extension

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

@clerk/clerk-js

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

@clerk/dev-cli

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

@clerk/expo

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

@clerk/expo-passkeys

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

@clerk/express

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

@clerk/fastify

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

@clerk/localizations

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

@clerk/nextjs

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

@clerk/nuxt

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

@clerk/react

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

@clerk/react-router

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

@clerk/shared

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

@clerk/tanstack-react-start

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

@clerk/testing

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

@clerk/ui

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

@clerk/upgrade

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

@clerk/vue

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

commit: 553d6a9

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 3, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

A new test suite was added at packages/clerk-js/src/core/tests/state.test.ts covering Clerk JS State flow for SignUp and SignIn resources. Tests exercise initialization/reset of State, eventBus signaling, handling of null updates (first-from-null, ignored when previous resource cannot be discarded, allowed after finalize/reset flows), replacements by non-null resources, rapid successive updates, same-instance updates, and restoration of static clerk references. No public API signatures were changed.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main change: adding tests for null resource update behavior in the Clerk JS State class.
Linked Issues check ✅ Passed The PR fully addresses USER-4372 coding requirements by implementing comprehensive tests for shouldIgnoreNullUpdate behavior across all specified scenarios including initial updates, non-finalized resources, post-finalize behavior, and edge cases.
Out of Scope Changes check ✅ Passed All changes are in-scope: the new test file directly addresses USER-4372 requirements, and the changeset file is a standard project artifact for documenting changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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


No actionable comments were generated in the recent review. 🎉


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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@packages/clerk-js/src/core/__tests__/state.test.ts`:
- Around line 12-25: Tests modify the static SignUp.clerk and never restore it,
causing global state leakage; capture the original value (e.g., const
_originalSignUpClerk = SignUp.clerk) before mutating in your test setup
(beforeEach or beforeAll) and reassign SignUp.clerk = _originalSignUpClerk in
afterEach along with the existing signUpResourceSignal({ resource: null }) /
signInResourceSignal({ resource: null }) and vi.clearAllMocks(); apply the same
snapshot-and-restore pattern wherever SignUp.clerk is overwritten in tests
(references: SignUp.clerk, State constructor that registers handlers,
signUpResourceSignal, signInResourceSignal).
- Around line 55-100: Add concrete assertions to verify null-update handling: in
the first test, call the code path that would trigger a null resource update
(e.g., simulate finalize() behavior or invoke the internal method that sets
canBeDiscarded on the existing SignUp) and assert that
signUpResourceSignal().resource becomes null (or remains the existing resource)
according to shouldIgnoreNullUpdate semantics; in the second test, after
awaiting existingSignUp.__internal_future.reset(), assert that
signUpResourceSignal().resource is a new SignUp with null/undefined id (or that
the previous id 'signup_123' is gone) in addition to
expect(mockClient.resetSignUp).toHaveBeenCalled(), so both the call and the
resulting signal update are validated for SignUp, signUpResourceSignal, and
__internal_future.reset.

@jacekradko jacekradko changed the title test(clerk-js): add tests verifying shouldIgnoreNullUpdate behavior test(clerk-js): add tests for null resource update behavior Feb 10, 2026
@jacekradko jacekradko force-pushed the jacek/user-4372-verify-null-resource-updates-when-previous-resource-is-not branch from 78da62d to f419da1 Compare February 10, 2026 22:19
@jacekradko jacekradko merged commit 98dcad2 into main Feb 11, 2026
41 checks passed
@jacekradko jacekradko deleted the jacek/user-4372-verify-null-resource-updates-when-previous-resource-is-not branch February 11, 2026 01:28
Sign up for free to 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.

2 participants