Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat(ui): replace sync update with deferred promise pattern #7586

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.

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
jacekradko wants to merge 1 commit into fredrik/user-4043-remove-clerk-state-contexts-from-provider-and-rely-on
base: fredrik/user-4043-remove-clerk-state-contexts-from-provider-and-rely-on
Choose a base branch
Loading
from fix/remove-flushsync-baserouter

Conversation

@jacekradko
Copy link
Member

@jacekradko jacekradko commented Jan 13, 2026

Summary

Replaces the flushSync call in BaseRouter's baseNavigate with a deferred promise pattern.

Problem

The current implementation uses flushSync to guarantee the re-render happens before returning control to the caller. While this works, flushSync has performance implications:

  • Forces React to synchronously flush the entire update queue
  • Bypasses React's batching and scheduling optimizations
  • Can cause layout thrashing in complex UIs

Solution

Use a deferred promise pattern instead:

  1. Store the resolve function in state alongside the new route parts
  2. An effect resolves the promise after React commits the state update
  3. This achieves the same guarantee without forcing synchronous updates
// Before (flushSync)
flushSync(() => {
 setRouteParts({ path: toURL.pathname, queryString: toURL.search });
});
return internalNavRes;
// After (deferred promise)
return new Promise(resolve => {
 setRouteParts({ path: toURL.pathname, queryString: toURL.search });
 setPendingNavigation({
 routeParts: { path: toURL.pathname, queryString: toURL.search },
 result: internalNavRes,
 resolve,
 });
});

Benefits

  • Avoids flushSync performance penalty
  • Works better with React concurrent features
  • Maintains the same behavioral guarantee (re-render completes before promise resolves)

Replace the flushSync call in BaseRouter's baseNavigate with a deferred
promise pattern. This achieves the same guarantee (re-render completes
before returning control to the caller) without the performance penalty
of synchronously flushing React's update queue.
The new approach stores the resolve function in state alongside the new
route parts, and an effect resolves the promise after React commits the
state update.
Copy link

changeset-bot bot commented Jan 13, 2026

⚠️ No Changeset found

Latest commit: c418183

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

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

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

vercel bot commented Jan 13, 2026

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

Project Deployment Review Updated (UTC)
clerk-js-sandbox Ready Ready Preview, Comment Jan 13, 2026 5:18pm

@jacekradko jacekradko changed the title (削除) refactor(ui): replace flushSync with deferred promise pattern (削除ここまで) (追記) feat(ui): replace flushSync with deferred promise pattern (追記ここまで) Jan 13, 2026
@jacekradko jacekradko changed the title (削除) feat(ui): replace flushSync with deferred promise pattern (削除ここまで) (追記) feat(ui): replace sync update with deferred promise pattern (追記ここまで) Jan 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Ephem Ephem Awaiting requested review from Ephem

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

AltStyle によって変換されたページ (->オリジナル) /