fix(react): deprecate SignOutButton signOutOptions prop#8147
fix(react): deprecate SignOutButton signOutOptions prop#8147jacekradko wants to merge 4 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 99d8b9b The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 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 |
@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: |
📝 WalkthroughWalkthroughThe pull request deprecates the 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/react/src/components/SignOutButton.tsx`:
- Around line 21-35: The clickHandler in SignOutButton currently spreads
signOutOptions after the explicit redirectUrl/sessionId so nested values can
overwrite top-level props; change the argument to clerk.signOut so
signOutOptions is spread first and then explicit redirectUrl and sessionId are
applied (i.e., spread signOutOptions before adding redirectUrl and the
conditional sessionId) to ensure top-level redirectUrl/sessionId on the
SignOutButton take precedence over deprecated signOutOptions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2a369f34-83d1-4bb9-8cb3-d3c10e7a8ccf
📒 Files selected for processing (1)
packages/react/src/components/SignOutButton.tsx
| const { redirectUrl = '/', sessionId, signOutOptions, getContainer, component, ...rest } = props; | ||
|
|
||
| if (signOutOptions) { | ||
| deprecated('SignOutButton `signOutOptions`', 'Use the `redirectUrl` and `sessionId` props directly instead.'); | ||
| } | ||
|
|
||
| children = normalizeWithDefaultValue(children, 'Sign out'); | ||
| const child = assertSingleChild(children)('SignOutButton'); | ||
|
|
||
| const clickHandler = () => clerk.signOut({ redirectUrl, ...signOutOptions }); | ||
| const clickHandler = () => | ||
| clerk.signOut({ | ||
| redirectUrl, | ||
| ...(sessionId !== undefined && { sessionId }), | ||
| ...signOutOptions, | ||
| }); |
There was a problem hiding this comment.
Top-level props lose precedence when both APIs are provided (migration bug)
At Line 34, ...signOutOptions is spread after Line 32-33, so deprecated nested values override top-level redirectUrl/sessionId. That keeps the ambiguous behavior this deprecation is meant to eliminate and can sign out/redirect using the wrong target when consumers pass both during migration.
Proposed fix (preserve backward compatibility, make top-level authoritative)
- const { redirectUrl = '/', sessionId, signOutOptions, getContainer, component, ...rest } = props;
+ const { redirectUrl, sessionId, signOutOptions, getContainer, component, ...rest } = props;
+
+ const resolvedRedirectUrl = redirectUrl ?? signOutOptions?.redirectUrl ?? '/';
+ const resolvedSessionId = sessionId ?? signOutOptions?.sessionId;
@@
const clickHandler = () =>
clerk.signOut({
- redirectUrl,
- ...(sessionId !== undefined && { sessionId }),
- ...signOutOptions,
+ redirectUrl: resolvedRedirectUrl,
+ ...(resolvedSessionId !== undefined && { sessionId: resolvedSessionId }),
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/react/src/components/SignOutButton.tsx` around lines 21 - 35, The
clickHandler in SignOutButton currently spreads signOutOptions after the
explicit redirectUrl/sessionId so nested values can overwrite top-level props;
change the argument to clerk.signOut so signOutOptions is spread first and then
explicit redirectUrl and sessionId are applied (i.e., spread signOutOptions
before adding redirectUrl and the conditional sessionId) to ensure top-level
redirectUrl/sessionId on the SignOutButton take precedence over deprecated
signOutOptions.
1 participant
Summary
signOutOptionsprop on<SignOutButton />in favor of flat top-levelredirectUrlandsessionIdprops@deprecatedJSDoc annotation and runtime deprecation warning viadeprecated()helpersignOutOptionsstill works (backwards compatible) but emits a console warning in developmentThis addresses user feedback about the confusing API where
redirectUrlcould be passed both as a top-level prop and nested insidesignOutOptions.Related: SDK-17
Test plan
sessionIdpropsignOutOptionstest passes unchanged (backwards compat)npx turbo build --filter=@clerk/reactpassesnpx turbo test --filter=@clerk/reactpasses (396 tests)Summary by CodeRabbit
New Features
<SignOutButton />now acceptssessionIdas a top-level prop.Deprecations
signOutOptionsis deprecated in favor of usingredirectUrlandsessionIdas separate props; backward compatibility is retained and a deprecation warning is emitted.Tests
sessionIdis passed correctly on sign-out.