-
Notifications
You must be signed in to change notification settings - Fork 436
fix(clerk-js): Fix broken transitive state when switching organization #7823
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
base: main
Are you sure you want to change the base?
fix(clerk-js): Fix broken transitive state when switching organization #7823
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: d593212 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
📝 WalkthroughWalkthroughThis pull request adds a skip-emission mechanism to Clerk by extending 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
Description
This PR fixes a bug that was introduced when we migrated to
useSyncExternalStore. The bug is that when switching orgs and navigating to a new page, the current page re-rendered with the new organization before the navigation, which can cause problems.Alternative solution: #7815
Compared to that solution, this touches more code, but is scoped to only block
updateClientfrom emitting for the specificsetActivecall that triggered it, it does not block allupdateClientcalls to keep the splash radius as small as possible.We want to refactor this more heavily to have better guarantees, but now is not the time.
I went with a version where a
__internal_touch()does not callupdateClientand instead returns aclientresource, expecting the caller to then callupdateClientwith the result.updateClientnow takes an option to skip emitting.If we want to refactor further in the future, this approach let's us move the
updateClientcall around, and also do things with the intermediaryclientif necessary.Another version would be to thread a
skipUiEmitoption all the way through the base resource calls, but in total that's 6 stacks deep and couples UI concerns to resources in a way I did not like.I have verified this fix works in the dashboard.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores