feat(react): add experimental BroadcastChannel sync support#172
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThis PR refactors the React auth hook API from a single ChangesAuth Hooks and Context Refactor
🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/react/src/unstable.tsx`:
- Around line 34-49: The code mutates a module-global __CONTEXT.client inside
AuthProvider which causes last-render-wins when multiple AuthProvider instances
exist; instead, stop writing to __CONTEXT.client and pass the client through
React context value from AuthProvider (e.g., include client in the provider
value object used by useContext consumers), update all hooks that currently read
__CONTEXT.client (and that use optional chaining) to consume the client via
useContext (referencing the same context object used in AuthProvider) and
explicitly throw or assert when the client is missing rather than silently
no-oping with optional chaining so misconfiguration surfaces immediately.
- Around line 68-80: The useEffect creates a BroadcastChannel unguarded and
never closes it; update the effect to obtain the channel via the existing guard
(call getBroadcastChannel() or check typeof window !== "undefined" &&
"BroadcastChannel" in window) and only proceed if a channel exists, attach the
message listener to that channel, and in the cleanup both remove the listener
and call channel.close() (referencing useEffect, BROADCAST_CHANNEL_NAME, and the
message handler/refreshSession to locate the code).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9d1cbc3f-9681-4c2f-a0eb-36d72ea7d8d5
📒 Files selected for processing (7)
apps/nextjs/app-router/src/app/client/page.tsxpackages/next/package.jsonpackages/next/src/unstable.tsxpackages/next/tsdown.config.tspackages/react/package.jsonpackages/react/src/unstable.tsxpackages/react/tsdown.config.ts
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
packages/react/src/hooks.ts (3)
114-174: ⚡ Quick winRemove commented-out code in useUpdateSession and useSignOut.
Lines 115, 123-133, 147, and 153-163 contain commented-out implementations that should be removed for the same reason as the earlier hooks.
Proposed cleanup
export const useUpdateSession = <DefaultUser extends User = User>() => { - //const [isPending, startTransition] = useTransition() const { client } = useAssertContext<DefaultUser>() const { execute, isPending } = useAsyncAction() const updateSession = useCallback( <Options extends UpdateSessionOptions<DefaultUser>>( options: Options ): Promise<UpdateSessionReturn<Options, DefaultUser>> => { - //return new Promise((resolve, reject) => { - // startTransition(async () => { - // try { - // const updated = await client.updateSession<Options>(options) - // broadcast({ type: "session:update", payload: (updated as any).session }) - // resolve(updated) - // } catch (error) { - // reject(error) - // } - // }) - //}) return execute(async () => { const updated = await client.updateSession<Options>(options) broadcast({ type: "session:update", payload: (updated as any).session }) return updated }) }, [client, execute] ) return { updateSession, isPending } as const } export const useSignOut = () => { - //const [isPending, startTransition] = useTransition() const { client } = useAssertContext() const { execute, isPending } = useAsyncAction() const signOut = useCallback( <Options extends SignOutOptions>(options?: Options): Promise<SignOutReturn<Options>> => { - //return new Promise((resolve, reject) => { - // startTransition(async () => { - // try { - // const value = await client.signOut(options) - // broadcast({ type: "session:clear" }) - // resolve(value) - // } catch (error) { - // reject(error) - // } - // }) - //}) return execute(async () => { const value = await client.signOut(options) broadcast({ type: "session:clear" }) return value }) }, [client, execute] ) return { signOut, isPending } as const }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react/src/hooks.ts` around lines 114 - 174, The file contains outdated commented-out implementations inside the hooks useUpdateSession and useSignOut; remove the dead comment blocks (the multi-line commented startTransition/promise code) inside the useUpdateSession function and inside the useSignOut function so only the active execute-based implementations remain, leaving the exported functions (useUpdateSession, useSignOut) and their returned objects unchanged; ensure imports or references (useAssertContext, useAsyncAction, broadcast, client.updateSession, client.signOut) are not altered while deleting those commented sections.
134-138: ⚡ Quick winAvoid
anytype assertion for session payload.The cast
(updated as any).sessionbypasses TypeScript's type checking and could mask type mismatches. Consider either:
- Narrowing the
Optionsgeneric constraint to ensuresessionis present when needed, or- Adding a type guard to safely access the session property.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react/src/hooks.ts` around lines 134 - 138, The current code in execute -> client.updateSession uses a cast (updated as any).session which bypasses type safety; replace that by either tightening the generic on client.updateSession so its return type always includes a session property when Options requires it, or add a runtime type guard (e.g., an isSessionResponse(updated): updated is { session: SessionType }) before calling broadcast to safely extract updated.session; update the execute block to call the type-guarded property (or adjust the generic signature of client.updateSession) and only broadcast when the guard passes to remove the use of any and preserve TypeScript safety.
51-112: ⚡ Quick winRemove commented-out code.
Multiple blocks of commented-out code (lines 53, 61-71, 86-87, 91-101) appear to be remnants from the refactoring to
useAsyncAction. These should be removed before merge.Proposed cleanup for useSignIn and useSignInCredentials
export const useSignIn = () => { const { client } = useAssertContext() - //const [isPending, startTransition] = useTransition() const { execute, isPending } = useAsyncAction() const signIn = useCallback( <Options extends SignInOptions>( oauth: LiteralUnion<BuiltInOAuthProvider>, options?: Options ): Promise<SignInReturn<Options>> => { - //return new Promise((resolve, reject) => { - //startTransition(async () => { - // try { - // const value = await client.signIn(oauth, options) - // broadcast({ type: "session:sync" }) - // resolve(value) - // } catch (error) { - // reject(error) - // } - //}) - //}) return execute(async () => { const value = await client.signIn(oauth, options) broadcast({ type: "session:sync" }) return value }) }, [client, execute] ) return { signIn, isPending } as const } export const useSignInCredentials = () => { const { client } = useAssertContext() - //const [isPending, startTransition] = useTransition() const { execute, isPending } = useAsyncAction() const signInCredentials = useCallback( <Options extends SignInCredentialsOptions>(options: Options): Promise<SignInCredentialsReturn<Options>> => { - //return new Promise((resolve, reject) => { - // startTransition(async () => { - // try { - // const value = await client.signInCredentials(options) - // broadcast({ type: "session:sync" }) - // resolve(value) - // } catch (error) { - // reject(error) - // } - // }) - //}) return execute(async () => { const value = await client.signInCredentials(options) broadcast({ type: "session:sync" }) return value }) }, [client, execute] ) return { signInCredentials, isPending } as const }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react/src/hooks.ts` around lines 51 - 112, Remove the leftover commented-out refactor artifacts inside useSignIn and useSignInCredentials: delete the commented Promise/startTransition blocks and any other commented lines around the client.signIn/client.signInCredentials calls so the functions only contain the current useAsyncAction implementation (keep useSignIn, useSignInCredentials, the execute calls, broadcast({ type: "session:sync" }) and the returned isPending). Ensure no commented blocks remain in those functions before merging.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/nextjs/app-router/src/app/client/page.tsx`:
- Around line 12-15: Remove the development debug comment "It's failing." that
sits above the useSession hook; locate the comment immediately preceding the
const { session, status } = useSession() line in page.tsx and delete it so only
the code and any meaningful doc comments remain (leave useSession and its
destructuring unchanged).
In `@packages/core/src/client/client.ts`:
- Around line 131-133: Update the defensive check for redirect to match other
methods: use the optional-chained property access (options?.redirect === true)
instead of options.redirect to handle cases where redirect is omitted; keep the
existing environment check (typeof window !== "undefined") and json?.redirectURL
logic. Locate the check in the UpdateSession handling (the block referencing
options.redirect and json?.redirectURL) and replace it with the
optional-chaining form so behavior remains the same but is consistent and safer.
In `@packages/core/test/client/client.test.ts`:
- Around line 173-194: Rename the duplicate test titled "signIn with redirect
option" to a unique, descriptive name that reflects the scenario under test
(e.g., "signIn does not redirect when redirect is false" or "signIn returns URL
without navigating when redirect is false"); update the test declaration that
calls client.signIn("github", { redirect: false }) so the test runner no longer
sees two tests with the same name and the behavior around window.location.assign
and the mocked get("/signIn/:oauth", { params..., searchParams: { redirect:
false } }) remains unchanged.
In `@packages/react/src/context.tsx`:
- Around line 55-59: The useEffect currently calls refreshSession when
initialSession === null which contradicts the documented contract; change the
condition to call refreshSession only when initialSession is undefined (i.e.,
trigger fetch on mount) and do nothing when initialSession is null (skip initial
client fetch). Update the useEffect in context.tsx referencing initialSession
and refreshSession so it checks for initialSession === undefined before calling
refreshSession.
In `@packages/react/src/hooks.ts`:
- Around line 19-25: The thrown Error in useAssertContext currently references
the removed hook name "useAuth"; update the message to mention the new hook
surface and provider so it is accurate for callers of useSession/useSignIn/etc.
— locate the useAssertContext function (which reads from AuthContext) and
replace the error string thrown when ctx === undefined with something like "Auth
hooks (useSession, useSignIn, etc.) must be used within an <AuthProvider>."
ensuring the message references AuthContext/useAssertContext so users know which
provider to wrap.
---
Nitpick comments:
In `@packages/react/src/hooks.ts`:
- Around line 114-174: The file contains outdated commented-out implementations
inside the hooks useUpdateSession and useSignOut; remove the dead comment blocks
(the multi-line commented startTransition/promise code) inside the
useUpdateSession function and inside the useSignOut function so only the active
execute-based implementations remain, leaving the exported functions
(useUpdateSession, useSignOut) and their returned objects unchanged; ensure
imports or references (useAssertContext, useAsyncAction, broadcast,
client.updateSession, client.signOut) are not altered while deleting those
commented sections.
- Around line 134-138: The current code in execute -> client.updateSession uses
a cast (updated as any).session which bypasses type safety; replace that by
either tightening the generic on client.updateSession so its return type always
includes a session property when Options requires it, or add a runtime type
guard (e.g., an isSessionResponse(updated): updated is { session: SessionType })
before calling broadcast to safely extract updated.session; update the execute
block to call the type-guarded property (or adjust the generic signature of
client.updateSession) and only broadcast when the guard passes to remove the use
of any and preserve TypeScript safety.
- Around line 51-112: Remove the leftover commented-out refactor artifacts
inside useSignIn and useSignInCredentials: delete the commented
Promise/startTransition blocks and any other commented lines around the
client.signIn/client.signInCredentials calls so the functions only contain the
current useAsyncAction implementation (keep useSignIn, useSignInCredentials, the
execute calls, broadcast({ type: "session:sync" }) and the returned isPending).
Ensure no commented blocks remain in those functions before merging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 31ee0bbd-41a8-46be-b6da-721a742d4c3a
📒 Files selected for processing (10)
apps/nextjs/app-router/src/app/client/page.tsxapps/nextjs/app-router/src/components/header.tsxpackages/core/src/client/client.tspackages/core/test/client/client.test.tspackages/next/src/client.tspackages/react/src/@types/types.tspackages/react/src/context.tsxpackages/react/src/hooks.tspackages/react/src/index.tsxpackages/react/test/hooks.test.tsx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/react-router/app/routes/client/index.tsx (1)
18-24:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBoth sign-in entry points are missing the new redirect opt-in.
redirectTois ignored unlessredirect: trueis set now. As written, the credentials flow will not reload/clientafter success, and the provider buttons will not navigate to the OAuth provider at all.💡 Suggested fix
await signInCredentials({ payload: { username, password, }, + redirect: true, redirectTo: "/client", })- onClick={() => signIn(provider.toLowerCase())} + onClick={() => signIn(provider.toLowerCase(), { redirect: true })}Also applies to: 108-115
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/react-router/app/routes/client/index.tsx` around lines 18 - 24, The sign-in calls are missing the new redirect opt-in so the redirectTo is ignored; update the credential and provider sign-in invocations (e.g., the signInCredentials call shown and the other sign-in call around lines 108-115) to include redirect: true alongside redirectTo so the flow reloads/navigates (i.e., pass { redirect: true, redirectTo: "/client", ... } to signInCredentials and the provider sign-in calls).apps/react-router/app/components/header.tsx (1)
103-104:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd the redirect opt-in to the mobile GitHub button.
This call still relies on the old implicit redirect behavior. After the client change,
signIn("github")will resolve without navigating to the provider.💡 Suggested fix
- <Button type="button" onClick={() => signIn("github")}> + <Button type="button" onClick={() => signIn("github", { redirect: true })}>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/react-router/app/components/header.tsx` around lines 103 - 104, The mobile GitHub button currently calls signIn("github") which relies on old implicit redirect behavior; update the call in the header component (the onClick handler for the mobile GitHub Button) to opt into redirect by passing options, e.g. signIn("github", { callbackUrl: window.location.href, redirect: true }) (or an appropriate callbackUrl) so the provider flow navigates as before.apps/nextjs/pages-router/src/components/header.tsx (1)
105-106:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPass
{ redirect: true }to the mobile sign-in action.
signIn()now only performs browser navigation when redirect is explicitly enabled. This button still callssignIn("github"), so the mobile auth CTA will stay on the current page instead of starting the OAuth flow.💡 Suggested fix
- <Button type="button" onClick={() => signIn("github")}> + <Button type="button" onClick={() => signIn("github", { redirect: true })}>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/nextjs/pages-router/src/components/header.tsx` around lines 105 - 106, The mobile sign-in button calls signIn("github") but doesn't enable navigation; update the onClick handler in header.tsx (the Button that currently calls signIn("github")) to pass the redirect option so it becomes signIn("github", { redirect: true }) ensuring the OAuth flow navigates away from the current page.apps/nextjs/pages-router/src/pages/client/index.tsx (1)
116-123:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOpt the provider buttons into redirect explicitly.
signIn()no longer redirects by default, so these GitHub/GitLab/Bitbucket buttons will stay on the page instead of kicking off the OAuth flow.💡 Suggested fix
- onClick={() => signIn(provider.toLowerCase())} + onClick={() => signIn(provider.toLowerCase(), { redirect: true })}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/nextjs/pages-router/src/pages/client/index.tsx` around lines 116 - 123, The OAuth provider buttons currently call signIn(provider.toLowerCase()) which no longer redirects by default; update the onClick handler used in the provider map (the Button inside the ["Github","Gitlab","Bitbucket"].map) to call signIn with explicit redirect behavior (e.g. pass an options object with redirect: true and an appropriate callbackUrl such as the current location or your post-login route) so clicking the Button triggers a full redirect to the provider's OAuth flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/nextjs/pages-router/src/components/header.tsx`:
- Around line 105-106: The mobile sign-in button calls signIn("github") but
doesn't enable navigation; update the onClick handler in header.tsx (the Button
that currently calls signIn("github")) to pass the redirect option so it becomes
signIn("github", { redirect: true }) ensuring the OAuth flow navigates away from
the current page.
In `@apps/nextjs/pages-router/src/pages/client/index.tsx`:
- Around line 116-123: The OAuth provider buttons currently call
signIn(provider.toLowerCase()) which no longer redirects by default; update the
onClick handler used in the provider map (the Button inside the
["Github","Gitlab","Bitbucket"].map) to call signIn with explicit redirect
behavior (e.g. pass an options object with redirect: true and an appropriate
callbackUrl such as the current location or your post-login route) so clicking
the Button triggers a full redirect to the provider's OAuth flow.
In `@apps/react-router/app/components/header.tsx`:
- Around line 103-104: The mobile GitHub button currently calls signIn("github")
which relies on old implicit redirect behavior; update the call in the header
component (the onClick handler for the mobile GitHub Button) to opt into
redirect by passing options, e.g. signIn("github", { callbackUrl:
window.location.href, redirect: true }) (or an appropriate callbackUrl) so the
provider flow navigates as before.
In `@apps/react-router/app/routes/client/index.tsx`:
- Around line 18-24: The sign-in calls are missing the new redirect opt-in so
the redirectTo is ignored; update the credential and provider sign-in
invocations (e.g., the signInCredentials call shown and the other sign-in call
around lines 108-115) to include redirect: true alongside redirectTo so the flow
reloads/navigates (i.e., pass { redirect: true, redirectTo: "/client", ... } to
signInCredentials and the provider sign-in calls).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7bae888d-d0c4-4fe5-b212-c4663f7043ed
📒 Files selected for processing (13)
apps/nextjs/app-router/src/app/client/page.tsxapps/nextjs/pages-router/src/components/header.tsxapps/nextjs/pages-router/src/pages/client/index.tsxapps/react-router/app/components/header.tsxapps/react-router/app/routes/client/index.tsxpackages/core/src/client/client.tspackages/core/test/client/client.test.tspackages/react-router/package.jsonpackages/react-router/src/client.tspackages/react/CHANGELOG.mdpackages/react/package.jsonpackages/react/src/context.tsxpackages/react/src/hooks.ts
✅ Files skipped from review due to trivial changes (3)
- packages/react-router/src/client.ts
- packages/react-router/package.json
- packages/react/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/core/test/client/client.test.ts
- apps/nextjs/app-router/src/app/client/page.tsx
- packages/core/src/client/client.ts
- packages/react/src/context.tsx
- packages/react/src/hooks.ts
This was referenced May 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request introduces experimental BroadcastChannel synchronization support for authentication state management across browser tabs. With this feature, authentication events are synchronized in real time between tabs, allowing actions such as
signOutto automatically propagate and invalidate sessions across all active tabs in the same browser context. This update also includes a refactor of the React context and hooks APIs to improve consistency, readability, and developer experience.Hooks now return object-based APIs instead of tuple/array responses, exposing:
signIn,signOut, etc.)isPendingstate indicating whether the operation is currently runningAdditionally, a new
useAuthActionshook was introduced to centralize all authentication actions into a single hook.Key Changes
BroadcastChannelsynchronization supportisPendingstate to auth action hooksuseAuthActionshookUsage