feat: getSession on root beforeLoad to avoid spinner#740
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughConsolidates session sourcing by introducing a central ChangesSession / SSR Consolidation
Theme & UI tweaks
Small navigation change
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (client)
participant Route as Route beforeLoad (__root__)
participant SSR as initAuthSsr (server fn)
participant AuthAPI as auth.api.getSession
participant RouteCtx as RootRouteApi / route context
Browser->>Route: initial navigation (SSR)
Route->>SSR: call initAuthSsr()
SSR->>AuthAPI: auth.api.getSession (uses request headers)
AuthAPI-->>SSR: returns authSession
SSR-->>Route: { authSession }
Route-->>RouteCtx: expose authSession via route context
Browser->>useSession: client hook call
useSession->>RouteCtx: read authSession
useSession-->>Browser: return seeded session (if client pending)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/ui/theme-switcher.tsx (1)
38-48:⚠️ Potential issue | 🟡 MinorHydration flicker / mismatch after removing
useHydratedguard.
next-themescannot readlocalStorageduring SSR, sothemeisundefinedon the server and the?? 'system'fallback always rendersSunMoonIcon+ the "system" label at SSR. On the client, after hydrationthemeresolves to the user's persisted value (e.g.'light'/'dark'), causing an icon/label swap on first paint and a React hydration mismatch on this subtree. ThesuppressHydrationWarningon the root<html>element does not suppress warnings in nested components that render differently between server and client.This affects:
src/components/ui/theme-switcher.tsx(lines 38–48)src/layout/manager/nav-user.tsx(lines 114–132)Consider wrapping these components with
suppressHydrationWarning, or usenext-themes'resolvedThemewith an explicit placeholder instead of readingthemedirectly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/theme-switcher.tsx` around lines 38 - 48, The hydration mismatch comes from reading theme directly (variable theme, themes array, match, SunMoonIcon/SunIcon/MoonIcon, t) during SSR; switch to using next-themes' resolvedTheme (from useTheme) or guard client-only rendering: in src/components/ui/theme-switcher.tsx and src/layout/manager/nav-user.tsx, replace reads of theme ?? 'system' with resolvedTheme (or check mounted/window) and if resolvedTheme is undefined render a stable SSR placeholder (e.g., the neutral system icon/label) or wrap the icon/label subtree in an element with suppressHydrationWarning to avoid hydration errors; ensure the match() calls use resolvedTheme (or the mounted check) so server and client initial output match.src/features/auth/utils.ts (1)
14-17:⚠️ Potential issue | 🟠 MajorRedirect as soon as prefetched session data is available.
The new
useSession()can expose root-prefetchedsession.datawhile the client session request is still pending. Keepingsession.isPendingin this guard delays redirects until the client refetch completes, which undermines the spinner-avoidance flow.Proposed fix
- if (session.isPending || !session.data) { + if (!session.data) { return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/auth/utils.ts` around lines 14 - 17, The effect's exec function delays redirects by checking session.isPending even when root-prefetched session.data is already available; update the guard inside useEffect/exec to only gate on session.data (e.g., replace "if (session.isPending || !session.data) return" with a check that returns when !session.data) so redirects run immediately when session.data exists from server prefetch while avoiding waiting for client refetch.src/features/auth/guard-public-only.tsx (1)
10-20:⚠️ Potential issue | 🟠 MajorAvoid showing the public page or full spinner when a prefetched session exists.
useSession()can providesession.datafrom the root prefetch while the client refetch is still pending. This branch still shows<Spinner full />onisPending, and when not pending it renderschildreneven for authenticated users until the redirect effect runs.Proposed fix
export const GuardPublicOnly = ({ children }: { children?: ReactNode }) => { const session = useSession(); useRedirectAfterLogin(); + const isLoadingWithoutSession = session.isPending && !session.data; + const shouldShowChildren = !isLoadingWithoutSession && !session.data; if (session.error && session.error.status > 0) { return <PageError type="unknown-auth-error" />; } return ( <> - {session.isPending && <Spinner full />} - <Activity mode={session.isPending ? 'hidden' : 'visible'}> + {isLoadingWithoutSession && <Spinner full />} + <Activity mode={shouldShowChildren ? 'visible' : 'hidden'}> {children} </Activity> </> ); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/auth/guard-public-only.tsx` around lines 10 - 20, The guard currently shows the full-page spinner or public children while a prefetched session exists; update the logic in the component that calls useSession() and useRedirectAfterLogin() so that if session.data is present (i.e., a prefetched authenticated session) you do not render the public children or the full <Spinner full />: when session.data && session.isPending render a minimal/hidden loading state (avoid the full spinner), and when session.data && !session.isPending return nothing (or a placeholder) so authenticated users are not shown the public page while the redirect effect in useRedirectAfterLogin runs; keep the existing session.error check intact.
🧹 Nitpick comments (2)
src/routes/__root.tsx (1)
5-11: Merge duplicate@tanstack/react-routerimports.As flagged by SonarCloud,
getRouteApican be added to the existing import block instead of a second import statement.♻️ Proposed refactor
import { createRootRouteWithContext, + getRouteApi, HeadContent, Outlet, Scripts, } from '@tanstack/react-router'; -import { getRouteApi } from '@tanstack/react-router';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/__root.tsx` around lines 5 - 11, Merge the duplicate imports from `@tanstack/react-router` by adding getRouteApi to the existing import declaration that already imports createRootRouteWithContext, HeadContent, Outlet, and Scripts; update the import statement in the module containing createRootRouteWithContext/HeadContent/Outlet/Scripts to a single import that includes getRouteApi and remove the separate getRouteApi import.src/routes/index.tsx (1)
7-10: Reuse the root-routeauthSessionfrom context instead of refetching.The root
beforeLoadalready fetches the session on each request and exposes it through route context. CallinggetAuthSession()here duplicates the server round-trip unnecessarily. Consume the parent context instead:♻️ Proposed refactor
-import { getAuthSession } from '@/features/auth/session.server'; - export const Route = createFileRoute('/')({ component: RouteComponent, - beforeLoad: async () => { - const authSession = await getAuthSession(); - throw redirect({ to: authSession?.session ? '/app' : '/login' }); + beforeLoad: ({ context }) => { + throw redirect({ to: context.authSession?.session ? '/app' : '/login' }); }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/index.tsx` around lines 7 - 10, The beforeLoad handler currently calls getAuthSession() and causes an extra server round-trip; instead read the existing authSession exposed by the root route context and use that value to decide the redirect. Replace the getAuthSession() call in beforeLoad with the parent/root route context lookup (the authSession previously set on the root loader) and then call redirect({ to: authSession?.session ? '/app' : '/login' }) using that context value (referencing beforeLoad, getAuthSession, authSession, and redirect to locate and update the code).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/auth/use-session.ts`:
- Around line 17-23: The guard currently uses authSession?.session !== undefined
which is always true when authSession exists (because getAuthSession returns
session ?? null), causing null server-prefetched sessions to be treated as valid
fallbacks; update the conditional in use-session.ts where session.isPending is
checked (around the block that returns {...session, data: authSession.session
satisfies typeof session.data, isPending: false}) to use a truthy check (e.g.,
authSession?.session) instead of !== undefined so the fallback only applies for
real prefetched session data and not for explicit nulls or client-only
navigations.
---
Outside diff comments:
In `@src/components/ui/theme-switcher.tsx`:
- Around line 38-48: The hydration mismatch comes from reading theme directly
(variable theme, themes array, match, SunMoonIcon/SunIcon/MoonIcon, t) during
SSR; switch to using next-themes' resolvedTheme (from useTheme) or guard
client-only rendering: in src/components/ui/theme-switcher.tsx and
src/layout/manager/nav-user.tsx, replace reads of theme ?? 'system' with
resolvedTheme (or check mounted/window) and if resolvedTheme is undefined render
a stable SSR placeholder (e.g., the neutral system icon/label) or wrap the
icon/label subtree in an element with suppressHydrationWarning to avoid
hydration errors; ensure the match() calls use resolvedTheme (or the mounted
check) so server and client initial output match.
In `@src/features/auth/guard-public-only.tsx`:
- Around line 10-20: The guard currently shows the full-page spinner or public
children while a prefetched session exists; update the logic in the component
that calls useSession() and useRedirectAfterLogin() so that if session.data is
present (i.e., a prefetched authenticated session) you do not render the public
children or the full <Spinner full />: when session.data && session.isPending
render a minimal/hidden loading state (avoid the full spinner), and when
session.data && !session.isPending return nothing (or a placeholder) so
authenticated users are not shown the public page while the redirect effect in
useRedirectAfterLogin runs; keep the existing session.error check intact.
In `@src/features/auth/utils.ts`:
- Around line 14-17: The effect's exec function delays redirects by checking
session.isPending even when root-prefetched session.data is already available;
update the guard inside useEffect/exec to only gate on session.data (e.g.,
replace "if (session.isPending || !session.data) return" with a check that
returns when !session.data) so redirects run immediately when session.data
exists from server prefetch while avoiding waiting for client refetch.
---
Nitpick comments:
In `@src/routes/__root.tsx`:
- Around line 5-11: Merge the duplicate imports from `@tanstack/react-router` by
adding getRouteApi to the existing import declaration that already imports
createRootRouteWithContext, HeadContent, Outlet, and Scripts; update the import
statement in the module containing
createRootRouteWithContext/HeadContent/Outlet/Scripts to a single import that
includes getRouteApi and remove the separate getRouteApi import.
In `@src/routes/index.tsx`:
- Around line 7-10: The beforeLoad handler currently calls getAuthSession() and
causes an extra server round-trip; instead read the existing authSession exposed
by the root route context and use that value to decide the redirect. Replace the
getAuthSession() call in beforeLoad with the parent/root route context lookup
(the authSession previously set on the root loader) and then call redirect({ to:
authSession?.session ? '/app' : '/login' }) using that context value
(referencing beforeLoad, getAuthSession, authSession, and redirect to locate and
update 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2af05936-1682-4f7c-b69e-48a1c3a2a1e3
📒 Files selected for processing (19)
src/components/ui/theme-switcher.tsxsrc/features/account/change-name-drawer.tsxsrc/features/account/user-card.tsxsrc/features/auth/guard-authenticated.tsxsrc/features/auth/guard-public-only.tsxsrc/features/auth/page-login-verify.tsxsrc/features/auth/page-logout.tsxsrc/features/auth/page-onboarding.tsxsrc/features/auth/session.server.tssrc/features/auth/use-session.tssrc/features/auth/utils.tssrc/features/auth/with-permissions.tsxsrc/features/demo/demo-app-switch.tsxsrc/features/user/manager/form-user.tsxsrc/features/user/manager/page-user-update.tsxsrc/features/user/manager/page-user.tsxsrc/layout/manager/nav-user.tsxsrc/routes/__root.tsxsrc/routes/index.tsx
ivan-dalmet
left a comment
ivan-dalmet
left a comment
There was a problem hiding this comment.
Deploy and e2e need to be fixed 😉
| throw redirect({ to: '/login' }); | ||
| beforeLoad: async () => { | ||
| const authSession = await getAuthSession(); | ||
| throw redirect({ to: authSession?.session ? '/app' : '/login' }); |
There was a problem hiding this comment.
It should be /app or /manager depending of the role to match previous behaviour?
| const session = authClient.useSession(); | ||
| const { authSession } = RootRouteApi.useRouteContext(); | ||
|
|
||
| if (session.isPending && authSession?.session !== undefined) { |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/routes/__root.tsx (1)
37-41: Naming consistency nit.The existing helper above is
initSsrApp(verb +Ssr+ scope), but the new one isinitAuthSsr(verb + scope +Ssr). ConsiderinitSsrAuthto keep a consistent pattern, or rename both to a single convention.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/__root.tsx` around lines 37 - 41, Rename the new server-side helper initAuthSsr to follow the existing verb + Ssr + scope pattern (e.g., rename initAuthSsr to initSsrAuth) so it matches initSsrApp; update the declaration created from createServerFn({ method: 'GET' }).handler(async () => { return { authSession: await getAuthSession(), }; }); and change all references/usages of initAuthSsr to initSsrAuth across the codebase to keep naming consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/routes/__root.tsx`:
- Around line 46-52: beforeLoad currently awaits initAuthSsr() directly which
lets failures from auth.api.getSession() bubble and break the app shell; wrap
the await initAuthSsr() call inside a try/catch in the beforeLoad function (only
on SSR path) and on any exception return { authSession: null } (and optionally
log the error), so unauthenticated/public routes still render and
authClient.useSession() can re-fetch on the client; reference: beforeLoad and
initAuthSsr / AuthSession.
---
Nitpick comments:
In `@src/routes/__root.tsx`:
- Around line 37-41: Rename the new server-side helper initAuthSsr to follow the
existing verb + Ssr + scope pattern (e.g., rename initAuthSsr to initSsrAuth) so
it matches initSsrApp; update the declaration created from createServerFn({
method: 'GET' }).handler(async () => { return { authSession: await
getAuthSession(), }; }); and change all references/usages of initAuthSsr to
initSsrAuth across the codebase to keep naming consistent.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7cb9649e-5d86-40ad-9932-edcb4525965f
📒 Files selected for processing (1)
src/routes/__root.tsx
| beforeLoad: async (): Promise<{ authSession: AuthSession | null }> => { | ||
| if (!import.meta.env.SSR) { | ||
| return { authSession: null }; | ||
| } | ||
| const { authSession } = await initAuthSsr(); | ||
| return { authSession }; | ||
| }, |
There was a problem hiding this comment.
Consider guarding beforeLoad against getAuthSession failures.
If auth.api.getSession() throws (DB outage, transient network blip, malformed cookie, etc.), the rejection propagates out of the root beforeLoad, which will fail the entire app shell — including public/unauthenticated routes that don't need a session at all. Since the whole point of this change is to improve perceived load behavior, swallowing the failure to { authSession: null } keeps the app rendering and lets authClient.useSession() re-fetch on the client.
🛡️ Suggested resilience tweak
beforeLoad: async (): Promise<{ authSession: AuthSession | null }> => {
if (!import.meta.env.SSR) {
return { authSession: null };
}
- const { authSession } = await initAuthSsr();
- return { authSession };
+ try {
+ const { authSession } = await initAuthSsr();
+ return { authSession };
+ } catch {
+ // Fall back to client-side session fetch if SSR prefetch fails.
+ return { authSession: null };
+ }
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| beforeLoad: async (): Promise<{ authSession: AuthSession | null }> => { | |
| if (!import.meta.env.SSR) { | |
| return { authSession: null }; | |
| } | |
| const { authSession } = await initAuthSsr(); | |
| return { authSession }; | |
| }, | |
| beforeLoad: async (): Promise<{ authSession: AuthSession | null }> => { | |
| if (!import.meta.env.SSR) { | |
| return { authSession: null }; | |
| } | |
| try { | |
| const { authSession } = await initAuthSsr(); | |
| return { authSession }; | |
| } catch { | |
| // Fall back to client-side session fetch if SSR prefetch fails. | |
| return { authSession: null }; | |
| } | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/__root.tsx` around lines 46 - 52, beforeLoad currently awaits
initAuthSsr() directly which lets failures from auth.api.getSession() bubble and
break the app shell; wrap the await initAuthSsr() call inside a try/catch in the
beforeLoad function (only on SSR path) and on any exception return {
authSession: null } (and optionally log the error), so unauthenticated/public
routes still render and authClient.useSession() can re-fetch on the client;
reference: beforeLoad and initAuthSsr / AuthSession.
HugoPerard
force-pushed
the
improve-session-loader
branch
from
081c092 to
5cc2d94
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/features/auth/session.ts (1)
6-20: Consider a name that reflects the actual usage.
initAuthSsrreads as a one-shot SSR initializer, but this is a regular TanStack server function that is also invoked from client-sidebeforeLoad(e.g.src/routes/index.tsxon client navigation, where it becomes an HTTP roundtrip). A name likegetAuthSession/fetchAuthSession/getAuthSessionFnwould more accurately convey that it is a server function fetching the current session on demand, not just an SSR bootstrap step. The exportedAuthSessiontype would then naturally read as the result of that fetch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/auth/session.ts` around lines 6 - 20, The name initAuthSsr is misleading—rename the server function to something like getAuthSession (or fetchAuthSession/getAuthSessionFn) and update the exported type name accordingly (AuthSession should remain the result type of the renamed function); locate the function created via createServerFn and the AuthSession type definition, change the identifier initAuthSsr to getAuthSession, and update all call sites/imports (e.g., client beforeLoad usage and any references to initAuthSsr) so they import and call the new name; ensure the implementation still calls auth.api.getSession with getRequestHeaders() and that any tests or types referencing Awaited<ReturnType<typeof initAuthSsr>> are updated to Awaited<ReturnType<typeof getAuthSession>>.src/routes/index.tsx (1)
9-28: Reuse the root route's prefetchedauthSessionfrom context to avoid a duplicate session fetch.The root route's
beforeLoadalready awaitsinitAuthSsr()and exposesauthSessionon the route context. On initial SSR load, thisbeforeLoadthen callsinitAuthSsr()a second time, producing a redundantauth.api.getSessionlookup (a duplicate DB roundtrip) for every visit to/. On client navigation the root returns{ authSession: null }, so a fresh fetch is still required there.You can read from the parent context first and only invoke the server function when it is unavailable:
♻️ Proposed refactor
- beforeLoad: async () => { - const { authSession } = await initAuthSsr(); + beforeLoad: async ({ context }) => { + const authSession = + context.authSession ?? (await initAuthSsr()).authSession; if (!authSession?.session) { throw redirect({ to: '/login' }); } if ( authClient.admin.checkRolePermission({ - role: authSession.user.role as Role, + role: authSession.user.role as Role, permissions: { apps: ['manager'], }, }) ) { throw redirect({ to: '/manager' }); } throw redirect({ to: '/app' }); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/index.tsx` around lines 9 - 28, The beforeLoad currently always calls initAuthSsr() causing a duplicate session fetch; change beforeLoad to first read the parent route context for authSession and only call initAuthSsr() when that context value is missing (e.g., on client navigation). Update the logic inside beforeLoad (referencing beforeLoad, authSession, and initAuthSsr) so you use the parent-provided authSession if present, then perform the same role check via authClient.admin.checkRolePermission and redirect accordingly; only fall back to calling initAuthSsr() when parent context has no authSession.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/features/auth/session.ts`:
- Around line 6-20: The name initAuthSsr is misleading—rename the server
function to something like getAuthSession (or fetchAuthSession/getAuthSessionFn)
and update the exported type name accordingly (AuthSession should remain the
result type of the renamed function); locate the function created via
createServerFn and the AuthSession type definition, change the identifier
initAuthSsr to getAuthSession, and update all call sites/imports (e.g., client
beforeLoad usage and any references to initAuthSsr) so they import and call the
new name; ensure the implementation still calls auth.api.getSession with
getRequestHeaders() and that any tests or types referencing
Awaited<ReturnType<typeof initAuthSsr>> are updated to Awaited<ReturnType<typeof
getAuthSession>>.
In `@src/routes/index.tsx`:
- Around line 9-28: The beforeLoad currently always calls initAuthSsr() causing
a duplicate session fetch; change beforeLoad to first read the parent route
context for authSession and only call initAuthSsr() when that context value is
missing (e.g., on client navigation). Update the logic inside beforeLoad
(referencing beforeLoad, authSession, and initAuthSsr) so you use the
parent-provided authSession if present, then perform the same role check via
authClient.admin.checkRolePermission and redirect accordingly; only fall back to
calling initAuthSsr() when parent context has no authSession.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5ecd6803-4a05-4b26-8663-25e16401cc81
📒 Files selected for processing (4)
src/features/auth/session.tssrc/features/auth/use-session.tssrc/routes/__root.tsxsrc/routes/index.tsx
✅ Files skipped from review due to trivial changes (1)
- src/routes/__root.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 (1)
src/features/user/manager/page-user-new.tsx (1)
41-49:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSuccessful submit navigation can be blocked by dirty-form guard
At Line 48,
router.navigate(...)can be intercepted becausePreventNavigation(Line 69) still blocks whenform.formState.isDirtyis true. After create success, this may show a blocker prompt instead of redirecting, which regresses the post-submit flow.Suggested fix
onSuccess: async () => { // Invalidate Users list await queryClient.invalidateQueries({ queryKey: orpc.user.getAll.key(), type: 'all', }); + form.reset(); await router.navigate({ to: '/manager/users' }); },🤖 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 `@src/features/user/manager/page-user-new.tsx` around lines 41 - 49, The post-submit redirect can be blocked by the PreventNavigation guard because form.formState.isDirty remains true; in the onSuccess handler (function onSuccess where you call queryClient.invalidateQueries and router.navigate) reset the form's dirty state before navigating — e.g. call form.reset(...) or otherwise clear form.formState.isDirty (or flip a guard-bypass flag consumed by PreventNavigation) right after invalidating queries and before calling router.navigate({ to: '/manager/users' }) so the navigation isn't intercepted.
🤖 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 `@src/features/user/manager/page-user-new.tsx`:
- Around line 41-49: The post-submit redirect can be blocked by the
PreventNavigation guard because form.formState.isDirty remains true; in the
onSuccess handler (function onSuccess where you call
queryClient.invalidateQueries and router.navigate) reset the form's dirty state
before navigating — e.g. call form.reset(...) or otherwise clear
form.formState.isDirty (or flip a guard-bypass flag consumed by
PreventNavigation) right after invalidating queries and before calling
router.navigate({ to: '/manager/users' }) so the navigation isn't intercepted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fca642b4-df59-40a0-849d-dd1bfd284d60
📒 Files selected for processing (2)
src/features/auth/guard-authenticated.tsxsrc/features/user/manager/page-user-new.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/features/auth/guard-authenticated.tsx
HugoPerard
force-pushed
the
improve-session-loader
branch
from
0997b30 to
e6db0cd
Compare
|
2 participants
During first app load or after refreshing page we had a full spinner caused by guard components session fetch. This loading state combined with other loading states can be cause some visual shifts. The idea is to fetch beforeLoad the session and to use it on guard (and other session usages)
Before
Screen.Recording.2026-04-22.at.17.15.39.mov
After
Screen.Recording.2026-04-22.at.17.15.19.mov
Summary by CodeRabbit
New Features
Bug Fixes
Changes