Skip to content

Conversation

@dstaley
Copy link
Member

@dstaley dstaley commented Nov 26, 2025

Description

This PR fixes an issue where SignIn and SignUp instances were unable to be serialized with JSON.stringify due to a circular reference between themselves and their Future classes. The issue was resolved by marking resource as a private field, which excludes it from serialization.

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 a circular reference that prevented SignIn and SignUp instances from being serialized with JSON.stringify. These objects can now be properly serialized for caching, storage, transmission, or logging purposes.

✏️ Tip: You can customize this high-level summary in your review settings.

@changeset-bot
Copy link

changeset-bot bot commented Nov 26, 2025

🦋 Changeset detected

Latest commit: 2b6d58c

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

This PR includes changesets to release 3 packages
Name Type
@clerk/clerk-js Patch
@clerk/chrome-extension Patch
@clerk/clerk-expo Patch

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
Copy link

vercel bot commented Nov 26, 2025

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

Project Deployment Preview Comments Updated (UTC)
clerk-js-sandbox Ready Ready Preview Comment Nov 26, 2025 4:18pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 26, 2025

Walkthrough

This PR fixes a JSON serialization issue in @clerk/clerk-js by converting public resource fields in SignInFuture and SignUpFuture classes to private fields, eliminating circular references that prevented JSON.stringify() from working. Serialization tests are added to verify the fix.

Changes

Cohort / File(s) Summary
Changeset
.changeset/dark-sides-beg.md
Adds patch release note documenting fix for JSON serialization of SignIn and SignUp instances due to circular reference.
Core Resource Refactoring
packages/clerk-js/src/core/resources/SignIn.ts, packages/clerk-js/src/core/resources/SignUp.ts
Converts public resource field to private field #resource in SignInFuture and SignUpFuture classes respectively. Updates all internal references to use the private field. Constructor parameter in SignUpFuture no longer exposed as readonly public field.
Serialization Tests
packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts, packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
Adds tests verifying JSON.stringify() successfully serializes SignIn, SignInFuture, SignUp, and SignUpFuture instances.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Homogeneous refactoring pattern applied consistently across two similar classes
  • Field visibility changes are straightforward with mechanical updates to all references
  • New tests are simple snapshot/defined assertions with no complex logic
  • Primary attention: verify all this.resource references were correctly converted to this.#resource in both files

Poem

🐰 A circular loop once broke the code,
Round and round the data flowed.
Private fields mend the way,
Now JSON strings will work today!
*hops excitedly* 🎉

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: marking SignIn/SignUpFuture resource properties as private to fix JSON serialization issues caused by circular references.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ds.fix/private-resource-future

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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

@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 26, 2025

Open in StackBlitz

@clerk/agent-toolkit

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

@clerk/astro

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

@clerk/backend

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

@clerk/chrome-extension

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

@clerk/clerk-js

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

@clerk/dev-cli

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

@clerk/elements

npm i https://pkg.pr.new/@clerk/elements@7321

@clerk/clerk-expo

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

@clerk/expo-passkeys

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

@clerk/express

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

@clerk/fastify

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

@clerk/localizations

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

@clerk/nextjs

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

@clerk/nuxt

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

@clerk/clerk-react

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

@clerk/react-router

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

@clerk/remix

npm i https://pkg.pr.new/@clerk/remix@7321

@clerk/shared

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

@clerk/tanstack-react-start

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

@clerk/testing

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

@clerk/themes

npm i https://pkg.pr.new/@clerk/themes@7321

@clerk/types

npm i https://pkg.pr.new/@clerk/types@7321

@clerk/upgrade

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

@clerk/vue

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

commit: 2b6d58c

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: 0

🧹 Nitpick comments (3)
packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts (2)

19-23: Consider enhancing the serialization test.

The test successfully verifies that JSON.stringify doesn't throw an error (which would occur with circular references). However, it could be more robust by also validating that the serialized output contains expected properties.

Optional enhancement:

 it('can be serialized with JSON.stringify', () => {
   const signIn = new SignIn();
   const snapshot = JSON.stringify(signIn);
   expect(snapshot).toBeDefined();
+  const parsed = JSON.parse(snapshot);
+  expect(parsed).toBeTypeOf('object');
 });

80-84: Consider enhancing the SignInFuture serialization test.

Similar to the SignIn test, this could be enhanced to validate the JSON structure.

Optional enhancement:

 it('can be serialized with JSON.stringify', () => {
   const signIn = new SignIn();
   const snapshot = JSON.stringify(signIn.__internal_future);
   expect(snapshot).toBeDefined();
+  const parsed = JSON.parse(snapshot);
+  expect(parsed).toBeTypeOf('object');
 });
packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts (1)

29-40: LGTM! Serialization tests provide good coverage.

The tests verify that both SignUp and SignUpFuture can be serialized without circular reference errors. The test structure is consistent with the SignIn tests.

Optional enhancement (similar to SignIn tests):

 it('can be serialized with JSON.stringify', () => {
   const signUp = new SignUp();
   const snapshot = JSON.stringify(signUp);
   expect(snapshot).toBeDefined();
+  const parsed = JSON.parse(snapshot);
+  expect(parsed).toBeTypeOf('object');
 });
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ef7b024 and 2b6d58c.

📒 Files selected for processing (5)
  • .changeset/dark-sides-beg.md (1 hunks)
  • packages/clerk-js/src/core/resources/SignIn.ts (32 hunks)
  • packages/clerk-js/src/core/resources/SignUp.ts (13 hunks)
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts (2 hunks)
  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

All code must pass ESLint checks with the project's configuration

Files:

  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
packages/**/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Follow established naming conventions (PascalCase for components, camelCase for variables)

Prefer importing types from @clerk/shared/types instead of the deprecated @clerk/types alias

Files:

  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
packages/**/src/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels

Files:

  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
**/*.{test,spec}.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests are required for all new functionality
Verify proper error handling and edge cases
Include tests for all new features

Files:

  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
**/*.ts?(x)

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

Files:

  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
**/*.{test,spec,e2e}.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use real Clerk instances for integration tests

Files:

  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Implement type guards for unknown types using the pattern function isType(value: unknown): value is Type
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details in classes
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like <T extends { id: string }>
Use utility types like Omit, Partial, and Pick for data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Use const assertions with as const for literal types
Use satisfies operator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...

Files:

  • packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts
  • packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/clerk-js/src/core/resources/SignUp.ts
🧬 Code graph analysis (2)
packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts (1)
packages/clerk-js/src/core/resources/SignUp.ts (1)
  • SignUp (82-573)
packages/clerk-js/src/core/resources/SignUp.ts (1)
packages/clerk-js/src/utils/runAsyncResourceTask.ts (1)
  • runAsyncResourceTask (10-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: Build Packages
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
.changeset/dark-sides-beg.md (1)

1-5: LGTM! Changeset accurately documents the serialization fix.

The changeset correctly describes the fix for the circular reference issue that prevented JSON.stringify from working with SignIn and SignUp instances.

packages/clerk-js/src/core/resources/SignUp.ts (3)

583-587: LGTM! Private field correctly prevents circular reference in serialization.

The conversion from a public readonly resource property to a private #resource field is the correct fix. The private field syntax ensures the resource is not enumerable, preventing JSON.stringify from encountering the circular reference between SignUp and SignUpFuture instances.


589-685: All getter methods correctly updated to use private field.

All getter methods have been consistently updated to reference this.#resource instead of this.resource. The logic remains unchanged, maintaining functional equivalence while fixing the serialization issue.


687-956: All methods correctly refactored to use private resource field.

All async methods and the new private getCaptchaToken helper have been consistently updated to reference this.#resource. The refactoring maintains functional equivalence while ensuring the resource reference doesn't create circular dependencies during serialization.

packages/clerk-js/src/core/resources/SignIn.ts (4)

634-638: LGTM! Private field correctly prevents circular reference in SignIn serialization.

Identical to SignUpFuture, the conversion to a private #resource field ensures the resource reference is not enumerable, preventing circular reference errors during JSON.stringify.


640-691: All SignInFuture getter methods correctly updated.

All getter methods consistently reference this.#resource, maintaining the same logic while fixing the serialization issue. The nested property accesses in isTransferable and existingSession are handled correctly.


693-1175: All SignInFuture methods correctly refactored to use private field.

All async methods have been comprehensively updated to reference this.#resource for property access, method calls, and when passing the resource to runAsyncResourceTask. The refactoring is thorough and maintains functional behavior.


1177-1231: selectFirstFactor helper method correctly updated.

The private selectFirstFactor helper method has been properly updated to reference this.#resource.supportedFirstFactors and this.#resource.identifier, maintaining its logic while using the private field.

Copy link
Member

@wobsoriano wobsoriano left a comment

Choose a reason for hiding this comment

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

🆒

@dstaley dstaley merged commit ada03e7 into main Nov 26, 2025
47 checks passed
@dstaley dstaley deleted the ds.fix/private-resource-future branch November 26, 2025 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants