perf: remove global fetchUser from root beforeLoad to improve TTFB#1939
perf: remove global fetchUser from root beforeLoad to improve TTFB#1939
Conversation
- Remove fetchUser call from __root.tsx beforeLoad that was blocking every page request - Move auth check to /app route which is the only route tree that needs it - Marketing pages (/, /pricing, /blog, /docs) no longer wait for Supabase auth call - This should significantly improve TTFB for all marketing pages on Netlify Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughMoved server-side user fetching: removed root-level beforeLoad and fetchUser import; app-level route now calls fetchUser in its beforeLoad and app index adds a loader passing context.user; a file-transcription component now fetches user on the client via Supabase in an effect; one parent view route loader was removed. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Browser
participant Router as Router
participant AppRoute as _view/app Route
participant Auth as fetchUser()
participant AppIndex as _view/app index loader
participant FileComp as file-transcription component
rect `#E6F4EA`
Client->>Router: Navigate to /_view/app
Router->>AppRoute: Run beforeLoad
AppRoute->>Auth: call fetchUser()
Auth-->>AppRoute: returns user or null
alt user exists
AppRoute-->>Router: return { user }
Router->>AppIndex: run loader (context.user)
AppIndex-->>Router: return { user }
Router-->>Client: render with server-provided user
else no user
AppRoute->>Router: throw redirect to "/"
Router-->>Client: navigate to /
end
end
note over FileComp: client-side component loads user on mount
Client->>FileComp: mount
FileComp->>Auth: getSupabaseBrowserClient() -> request current user
Auth-->>FileComp: returns user or null
FileComp-->>Client: set local user state (guarded by mounted flag)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
🧹 Nitpick comments (1)
apps/web/src/routes/_view/app/index.tsx (1)
4-10: Loader correctly projectsuser; consider removing now-redundant auth checkGiven that the parent
/_view/approute’sbeforeLoadnow fetches the user and returns{ user }into the router context (per TanStack Router’s “modifying router context” semantics),context.usershould already be non-null whenever this index route runs. In that case:
loader: async ({ context }) => ({ user: context.user })is a cheap, correct way to expose the user as loader data.- The local
beforeLoadguard that redirects to/app/accounton missingcontext.userlikely never triggers anymore, since an unauthenticated request should already have been redirected by the parent route’sbeforeLoad.If that matches the intended flow, you can optionally drop this child
beforeLoadentirely (and rely solely on the parent) or align its redirect target with the parent for consistency.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/src/routes/__root.tsx(0 hunks)apps/web/src/routes/_view/app/index.tsx(1 hunks)apps/web/src/routes/_view/app/route.tsx(1 hunks)apps/web/src/routes/_view/route.tsx(0 hunks)
💤 Files with no reviewable changes (2)
- apps/web/src/routes/_view/route.tsx
- apps/web/src/routes/__root.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/web/src/routes/_view/app/index.tsxapps/web/src/routes/_view/app/route.tsx
🧬 Code graph analysis (1)
apps/web/src/routes/_view/app/route.tsx (3)
apps/web/src/routes/_view/app/index.tsx (1)
Route(3-10)apps/web/src/routes/_view/app/account.tsx (1)
Route(11-14)apps/web/src/functions/auth.ts (1)
fetchUser(56-67)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: ci (macos, macos-14)
- GitHub Check: fmt
🔇 Additional comments (1)
apps/web/src/routes/_view/app/route.tsx (1)
3-11: I'll help you verify the two key points raised in the review comment. Let me start by checking the usage offetchUserand understanding the broader context.
<function_calls>
#!/bin/bashCheck all usages of fetchUser in the web app
rg "fetchUser" apps/web/src -n -A 2 -B 2
</function_calls>
#!/bin/bashLocate root route file to check for global auth fetching
find apps/web/src -name ".tsx" -o -name ".ts" | grep -E "(root|root|layout)" | head -20
</function_calls>
<function_calls>
#!/bin/bashCheck the root route structure
ls -la apps/web/src/routes/
</function_calls>
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/src/routes/_view/file-transcription.tsx (2)
23-45: Consider usinguseQueryfor better state management.Per coding guidelines, "Use useQuery/useMutation (from tanstack-query) instead for 99% of cases." This would provide:
- Built-in loading state (preventing the sign-in button from flickering while auth loads)
- Better error handling (currently all errors are caught silently, which could hide authentication or network issues)
- Caching and automatic refetching capabilities
Apply this refactor to use
useQuery:+import { useQuery } from "@tanstack/react-query"; + function Component() { - const [user, setUser] = useState<{ email?: string } | null>(null); - - useEffect(() => { - let isMounted = true; - async function loadUser() { - try { - const supabase = getSupabaseBrowserClient(); - const { data } = await supabase.auth.getUser(); - if (!isMounted) return; - if (data.user?.email) { - setUser({ email: data.user.email }); - } else { - setUser(null); - } - } catch { - if (isMounted) setUser(null); - } - } - loadUser(); - return () => { - isMounted = false; - }; - }, []); + const { data: user } = useQuery({ + queryKey: ['user'], + queryFn: async () => { + const supabase = getSupabaseBrowserClient(); + const { data, error } = await supabase.auth.getUser(); + if (error) throw error; + return data.user?.email ? { email: data.user.email } : null; + }, + retry: false, + });As per coding guidelines.
155-162: User-gated transcript display is correctly implemented.The conditional rendering based on user authentication state works correctly for the current sample transcript implementation.
Note for future Deepgram integration: When implementing the actual transcription service (line 59), ensure the transcription API endpoint enforces authentication server-side. The current client-side gating (
user ? transcript : null) is appropriate for display logic but should not be the only security layer for sensitive transcription data.Also applies to: 174-176, 180-186
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/src/routes/_view/file-transcription.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/web/src/routes/_view/file-transcription.tsx
🧬 Code graph analysis (1)
apps/web/src/routes/_view/file-transcription.tsx (1)
apps/web/src/functions/supabase.ts (1)
getSupabaseBrowserClient(7-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: ci (macos, macos-14)
- GitHub Check: fmt
🔇 Additional comments (1)
apps/web/src/routes/_view/file-transcription.tsx (1)
2-2: LGTM!The import additions are necessary for the new client-side user fetching logic and are correctly placed.
Also applies to: 13-13
1 participant
Summary
Removes the global
fetchUser()Supabase call from the root route'sbeforeLoad, which was blocking every page request (including static marketing pages like/,/pricing,/blog,/docs). Auth is now only fetched for/app/*routes that actually need it.Before: Every page request → Netlify serverless function → Supabase auth call → HTML response
After: Marketing pages → Direct HTML response (no auth call);
/app/*routes → Auth call only when neededUpdates since last revision
file-transcription.tsx- this page was usingRoute.useRouteContext()to get user, which broke after removing user from root context. Now uses client-side auth viagetSupabaseBrowserClient()instead, which preserves the "sign in to see transcript" gating behavior without blocking SSR.Review & Testing Checklist for Human
/app/accountpage works - Navigate to/app/accountwhile logged in and confirm user email displays correctly. This is the highest risk area since it depends on TanStack Router context inheritance from the parent route./app/*auth redirect works - Try accessing/app/accountwhile logged out and confirm it redirects to//file-transcriptionpage works - Upload a file, verify the "Sign in" button appears when logged out, and transcript displays when logged in. Note: there may be a brief flash before auth state loads since it's now client-side./,/pricing,/blog,/docsall load correctly without errors/pricingTest Plan
/pricingand note TTFB (should be significantly faster, no Supabase call in waterfall)/auth, then navigate to/app/account- verify user info displays/app/accountdirectly - verify redirect to//file-transcription, upload a file - verify auth gating works (sign in prompt when logged out, transcript when logged in)Notes
context.user- it only uses platform detection, so no changes needed there/file-transcriptionpage now loads auth client-side, so there may be a brief moment where user state is unknown on initial renderRequested by: yujonglee (@yujonglee)
Devin session: https://app.devin.ai/sessions/9b81f9c911944213991f6b368b4b45af