-
Notifications
You must be signed in to change notification settings - Fork 430
refactor(*): nest satellite config under multiDomain key #7658
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?
Conversation
Why:
The satellite configuration (isSatellite, domain, satelliteAutoSync) was spread
across top-level options, making the API confusing and cluttered. Grouping them
under a single `multiDomain` key makes the intent clearer and the options object
more organized.
What changed:
- Removed top-level `isSatellite`, `domain`, `satelliteAutoSync` from
AuthenticateRequestOptions and ClerkOptions
- Added `multiDomain?: { isSatellite, domain?, proxyUrl?, autoSync? }` key
- `proxyUrl` stays top-level AND is allowed inside `multiDomain` (conflict
detection if both are set with different values)
- Renamed `satelliteAutoSync` → `autoSync` inside multiDomain
- Added `resolveProxyUrl` utility in shared/src/multiDomain.ts
- Updated all SDK packages: backend, nextjs, astro, react-router,
tanstack-react-start, clerk-js, react, vue, nuxt, express
- Updated all affected tests
🦋 Changeset detectedLatest commit: 443cd01 The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@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/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: |
📝 WalkthroughWalkthroughThis pull request restructures multi-domain and satellite configuration across the Clerk SDK by consolidating separate top-level properties— 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this 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 @.changeset/nest-multidomain-config.md:
- Around line 1-13: The changeset incorrectly marks breaking API removals as
minor—update the release type to MAJOR for all listed packages (the ones in the
diff: "@clerk/shared", "@clerk/backend", "@clerk/nextjs", "@clerk/astro",
"@clerk/react-router", "@clerk/tanstack-react-start", "@clerk/clerk-js",
"@clerk/express", "@clerk/react", "@clerk/vue", "@clerk/nuxt") because top-level
options isSatellite, domain, and satelliteAutoSync have been removed and moved
under multiDomain (the code and the HeadlessBrowserClerk interface no longer
accept those root-level fields), while proxyUrl remains at both levels; change
the changeset header from minor to major to reflect this breaking change.
In `@packages/astro/src/internal/merge-env-vars-with-params.ts`:
- Around line 23-34: The isSatellite boolean currently uses || which treats the
string "false" as truthy and overrides an explicit paramMultiDomain?.isSatellite
= false; change the logic in merge-env-vars-with-params.ts to use nullish
coalescing and parse the env value: compute isSatellite as
paramMultiDomain?.isSatellite ?? (import.meta.env.PUBLIC_CLERK_IS_SATELLITE ?
import.meta.env.PUBLIC_CLERK_IS_SATELLITE === "true" : undefined) (or
equivalent) so that explicit false from paramMultiDomain wins and the env string
is parsed to a real boolean before being used in the multiDomain object.
| --- | ||
| "@clerk/shared": minor | ||
| "@clerk/backend": minor | ||
| "@clerk/nextjs": minor | ||
| "@clerk/astro": minor | ||
| "@clerk/react-router": minor | ||
| "@clerk/tanstack-react-start": minor | ||
| "@clerk/clerk-js": minor | ||
| "@clerk/express": minor | ||
| "@clerk/react": minor | ||
| "@clerk/vue": minor | ||
| "@clerk/nuxt": minor | ||
| --- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Search for usage of the old options across the codebase
rg --type=ts --type=js -i 'isSatellite|satelliteAutoSync' -A 2 -B 2 | head -100Repository: clerk/javascript
Length of output: 7831
🏁 Script executed:
# Search for multiDomain usage to understand the new approach
rg --type=ts --type=js -i 'multiDomain' -A 2 -B 2 | head -100Repository: clerk/javascript
Length of output: 7141
🏁 Script executed:
# Look for deprecation warnings or compatibility aliases
rg --type=ts --type=js -i 'deprecat|compat|alias' | grep -i -E '(isSatellite|satelliteAutoSync|domain|multiDomain)' | head -50Repository: clerk/javascript
Length of output: 343
🏁 Script executed:
# Check the changeset file content to see full description
cat .changeset/nest-multidomain-config.mdRepository: clerk/javascript
Length of output: 632
🏁 Script executed:
# Search for any TypeScript type definitions or interfaces that might define these options
fd -e d.ts -e ts | xargs rg 'isSatellite|satelliteAutoSync|multiDomain' -B 3 -A 3 | head -150Repository: clerk/javascript
Length of output: 12265
Correct version bump severity: this is a breaking change requiring MAJOR, not MINOR.
The changeset removes top-level isSatellite, domain, and satelliteAutoSync options—they are no longer accepted at the root configuration level and must be moved under multiDomain. This is an incompatible API change per semver. The HeadlessBrowserClerk interface explicitly excludes isSatellite, and the codebase reads values exclusively from multiDomain.* properties with no backward-compatibility aliases.
While proxyUrl remains available at both levels, the removal of the three other options breaks any existing code using them at the top level. All affected packages should be bumped to MAJOR, not MINOR.
🤖 Prompt for AI Agents
In @.changeset/nest-multidomain-config.md around lines 1 - 13, The changeset
incorrectly marks breaking API removals as minor—update the release type to
MAJOR for all listed packages (the ones in the diff: "@clerk/shared",
"@clerk/backend", "@clerk/nextjs", "@clerk/astro", "@clerk/react-router",
"@clerk/tanstack-react-start", "@clerk/clerk-js", "@clerk/express",
"@clerk/react", "@clerk/vue", "@clerk/nuxt") because top-level options
isSatellite, domain, and satelliteAutoSync have been removed and moved under
multiDomain (the code and the HeadlessBrowserClerk interface no longer accept
those root-level fields), while proxyUrl remains at both levels; change the
changeset header from minor to major to reflect this breaking change.
| const isSatellite = paramMultiDomain?.isSatellite || import.meta.env.PUBLIC_CLERK_IS_SATELLITE; | ||
| const domain = paramMultiDomain?.domain || import.meta.env.PUBLIC_CLERK_DOMAIN; | ||
|
|
||
| return { | ||
| signInUrl: paramSignIn || import.meta.env.PUBLIC_CLERK_SIGN_IN_URL, | ||
| signUpUrl: paramSignUp || import.meta.env.PUBLIC_CLERK_SIGN_UP_URL, | ||
| isSatellite: paramSatellite || import.meta.env.PUBLIC_CLERK_IS_SATELLITE, | ||
| multiDomain: isSatellite | ||
| ? { | ||
| isSatellite: true as const, | ||
| ...(domain ? { domain: domain as string } : {}), | ||
| } | ||
| : paramMultiDomain, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix boolean/env precedence for isSatellite.
Using || with PUBLIC_CLERK_IS_SATELLITE can treat "false" as truthy and also override an explicit multiDomain.isSatellite = false, which can incorrectly enable satellite mode. Prefer nullish coalescing and parse the env string.
Proposed fix
- const isSatellite = paramMultiDomain?.isSatellite || import.meta.env.PUBLIC_CLERK_IS_SATELLITE;
- const domain = paramMultiDomain?.domain || import.meta.env.PUBLIC_CLERK_DOMAIN;
+ const isSatellite =
+ paramMultiDomain?.isSatellite ?? isTruthy(import.meta.env.PUBLIC_CLERK_IS_SATELLITE);
+ const domain = paramMultiDomain?.domain ?? import.meta.env.PUBLIC_CLERK_DOMAIN;🤖 Prompt for AI Agents
In `@packages/astro/src/internal/merge-env-vars-with-params.ts` around lines 23 -
34, The isSatellite boolean currently uses || which treats the string "false" as
truthy and overrides an explicit paramMultiDomain?.isSatellite = false; change
the logic in merge-env-vars-with-params.ts to use nullish coalescing and parse
the env value: compute isSatellite as paramMultiDomain?.isSatellite ??
(import.meta.env.PUBLIC_CLERK_IS_SATELLITE ?
import.meta.env.PUBLIC_CLERK_IS_SATELLITE === "true" : undefined) (or
equivalent) so that explicit false from paramMultiDomain wins and the env string
is parsed to a real boolean before being used in the multiDomain object.
Why
The satellite configuration (
isSatellite,domain,satelliteAutoSync) was scattered across top-level options in every SDK package. This made the API cluttered and it was unclear which options were related to multi-domain setups. Grouping them under a singlemultiDomainkey makes the intent explicit and the options easier to discover.What changed
isSatellite,domain,satelliteAutoSyncfromAuthenticateRequestOptionsandClerkOptionsmultiDomain?: { isSatellite, domain?, proxyUrl?, autoSync? }keyproxyUrlstays top-level AND is allowed insidemultiDomain(throws if both are set with conflicting values)satelliteAutoSync→autoSyncinside multiDomainresolveProxyUrlutility inshared/src/multiDomain.tsPackages affected
@clerk/shared: New types (MultiDomainConfig,MultiDomainAndOrProxy) andresolveProxyUrlutility@clerk/backend:AuthenticateRequestOptionsusesmultiDomain,authenticateContextresolves nested values@clerk/nextjs: Middleware and env merging usemultiDomain@clerk/astro: Middleware, integration, and env merging usemultiDomain@clerk/react-router: Middleware and load options usemultiDomain@clerk/tanstack-react-start: Load options and client utils usemultiDomain@clerk/clerk-js:isSatellitegetter reads fromoptions.multiDomain@clerk/react:isomorphicClerkreads fromoptions.multiDomain@clerk/express: PassesmultiDomaintoauthenticateRequest@clerk/vue: Removed obsoletedomainfromWithouttype@clerk/nuxt: Module and clerkClient usemultiDomainSummary by CodeRabbit
multiDomainobject, streamlining setup for multi-domain deployments.✏️ Tip: You can customize this high-level summary in your review settings.