-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(ui): Laggy data loading between pages switches #833
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: develop
Are you sure you want to change the base?
Conversation
WalkthroughRemoves all content from ENVEXAMPLE, updates postgres port mapping and healthcheck in docker-compose.yml, refactors ApiKey component to React Query for fetching/mutations, and restructures MainPage to memoize subviews to avoid remounts during tab switches. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant V as ApiKeyManager (UI)
participant Q as React Query
participant S as Backend API
U->>V: Open API Key page
V->>Q: useQuery("apiKey", fetch)
Q->>S: GET /api/key
S-->>Q: { apiKey }
Q-->>V: data, isLoading=false
U->>V: Click Generate
V->>Q: useMutation(generate)
Q->>S: POST /api/key
S-->>Q: { apiKey }
Q-->>Q: setQueryData("apiKey", newKey)
Q-->>V: onSuccess (button re-enabled)
U->>V: Click Delete
V->>Q: useMutation(delete)
Q->>S: DELETE /api/key
S-->>Q: 204
Q-->>Q: setQueryData("apiKey", null)
Q-->>V: onSuccess (button re-enabled)
sequenceDiagram
autonumber
actor U as User
participant M as MainPage
participant R as Memoized Views
note over M,R: Views are memoized via useMemo and remain mounted
U->>M: Switch tab
M->>R: Conditionally show memoized view
R-->>U: Render without remount/refetch
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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 |
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: 1
🧹 Nitpick comments (5)
docker-compose.yml (1)
10-16
: Ports/healthcheck LGTM; add start_period and wait-for-healthy for reliability.Compose will resolve ${DB_USER} at parse time—good. To avoid flapping on cold starts and ensure backend waits for DB readiness, add a start period and depends_on conditions.
Apply:
services: postgres: ... healthcheck: - test: ["CMD-SHELL", "pg_isready -U ${DB_USER}"] - interval: 10s - timeout: 5s - retries: 5 + test: ["CMD-SHELL", "pg_isready -U ${DB_USER}"] + interval: 10s + timeout: 5s + retries: 5 + start_period: 30s backend: ... - depends_on: - - postgres - - minio + depends_on: + postgres: + condition: service_healthy + minio: + condition: service_started frontend: ... - depends_on: - - backend + depends_on: + backend: + condition: service_startedsrc/components/api/ApiKey.tsx (1)
41-49
: Handle fetch errors explicitly (avoid showing “no key” on network errors).When the query errors, apiKey is undefined and the UI shows the “no key” message. Show an error state or toast.
Example:
-const { data: apiKey, isLoading } = useQuery({ +const { data: apiKey, isLoading, isError } = useQuery({ ... }); -if (isLoading) { +if (isLoading) { ... } +if (isError) { + return <Container><Typography color="error">{t('apikey.notifications.fetch_error')}</Typography></Container>; +}src/pages/MainPage.tsx (3)
306-314
: Stabilize abortRunHandler with useCallback to maximize memoization gains.runsView re-memoizes every render because abortRunHandler isn’t stable.
Wrap it:
-const abortRunHandler = (runId: string, robotName: string, browserId: string) => { +const abortRunHandler = useCallback((runId: string, robotName: string, browserId: string) => { ... -} +}, [t, notify, setRerenderRuns, invalidateRuns]);Then keep runsView deps unchanged. Optionally also wrap setRecordingInfo similarly to stabilize robotsView.
323-334
: Improve hidden view accessibility.Add hidden/aria-hidden so offscreen content isn’t read by screen readers.
-<div style={{ display: content === 'robots' ? 'block' : 'none', width: '100%' }}> +<div style={{ display: content === 'robots' ? 'block' : 'none', width: '100%' }} + hidden={content !== 'robots'} + aria-hidden={content !== 'robots'}> {robotsView} </div> -<div style={{ display: content === 'runs' ? 'block' : 'none', width: '100%' }}> +<div style={{ display: content === 'runs' ? 'block' : 'none', width: '100%' }} + hidden={content !== 'runs'} + aria-hidden={content !== 'runs'}> {runsView} </div> -<div style={{ display: content === 'proxy' ? 'block' : 'none', width: '100%' }}> +<div style={{ display: content === 'proxy' ? 'block' : 'none', width: '100%' }} + hidden={content !== 'proxy'} + aria-hidden={content !== 'proxy'}> {proxyView} </div> -<div style={{ display: content === 'apikey' ? 'block' : 'none', width: '100%' }}> +<div style={{ display: content === 'apikey' ? 'block' : 'none', width: '100%' }} + hidden={content !== 'apikey'} + aria-hidden={content !== 'apikey'}> {apiKeyView} </div>
296-305
: Keep-mounted views: consider pausing heavy work when hidden.If these subviews poll or open sockets, pause them when not visible (e.g., pass a visible prop to gate queries with enabled, or pause subscriptions).
Also applies to: 306-314, 315-317
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
ENVEXAMPLE
(0 hunks)docker-compose.yml
(1 hunks)src/components/api/ApiKey.tsx
(6 hunks)src/pages/MainPage.tsx
(2 hunks)
💤 Files with no reviewable changes (1)
- ENVEXAMPLE
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/MainPage.tsx (3)
src/components/robot/Recordings.tsx (1)
Recordings
(24-157)src/components/run/Runs.tsx (1)
Runs
(12-27)src/components/dashboard/MainMenu.tsx (1)
MainMenu
(16-164)
🔇 Additional comments (1)
src/components/api/ApiKey.tsx (1)
45-46
: Confirm axios credential config.Ensure axios adds withCredentials: true (global instance or per call), since auth likely relies on cookies.
Do you have a central axios setup enabling withCredentials? If not, I can propose a small axios instance wrapper.
Also applies to: 54-54, 69-69
// Fetch API key with React Query | ||
const { data: apiKey, isLoading } = useQuery({ | ||
queryKey: ['api-key'], | ||
queryFn: async () => { | ||
const { data } = await axios.get(`${apiUrl}/auth/api-key`); | ||
return data.api_key as string | null; | ||
}, | ||
staleTime: 10 * 60 * 1000, // 10 minutes | ||
}); |
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.
Scope cache by user to prevent API key leakage across logins.
Using a global ['api-key'] with 10m staleTime can show a previous user’s API key after sign‑out/in in the same tab. Scope by user.id (and gate with enabled) and update mutations accordingly.
Apply:
-import React, { useState } from 'react';
+import React, { useState, useContext } from 'react';
...
-import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
+import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
+import { AuthContext } from '../../context/auth';
...
const ApiKeyManager = () => {
const { t } = useTranslation();
const [apiKeyName, setApiKeyName] = useState<string>(t('apikey.default_name'));
const [showKey, setShowKey] = useState<boolean>(false);
const [copySuccess, setCopySuccess] = useState<boolean>(false);
const { notify } = useGlobalInfoStore();
const queryClient = useQueryClient();
+ const { state } = useContext(AuthContext);
+ const { user } = state || {};
// Fetch API key with React Query
- const { data: apiKey, isLoading } = useQuery({
- queryKey: ['api-key'],
+ const { data: apiKey, isLoading } = useQuery({
+ queryKey: ['api-key', user?.id],
queryFn: async () => {
const { data } = await axios.get(`${apiUrl}/auth/api-key`);
return data.api_key as string | null;
},
staleTime: 10 * 60 * 1000, // 10 minutes
+ enabled: !!user?.id,
});
Also update mutations:
- const { mutate: generateApiKey, isPending: isGenerating } = useMutation({
+ const { mutate: generateApiKey, isPending: isGenerating } = useMutation({
mutationFn: async () => {
const { data } = await axios.post(`${apiUrl}/auth/generate-api-key`);
return data.api_key as string;
},
- onSuccess: (newKey) => {
- queryClient.setQueryData(['api-key'], newKey);
+ onSuccess: (newKey) => {
+ queryClient.setQueryData(['api-key', user?.id], newKey);
notify('success', t('apikey.notifications.generate_success'));
},
- const { mutate: deleteApiKey, isPending: isDeleting } = useMutation({
+ const { mutate: deleteApiKey, isPending: isDeleting } = useMutation({
mutationFn: async () => {
await axios.delete(`${apiUrl}/auth/delete-api-key`);
},
onSuccess: () => {
- queryClient.setQueryData(['api-key'], null);
+ queryClient.setQueryData(['api-key', user?.id], null);
notify('success', t('apikey.notifications.delete_success'));
},
📝 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.
// Fetch API key with React Query | |
const { data: apiKey, isLoading } = useQuery({ | |
queryKey: ['api-key'], | |
queryFn: async () => { | |
const { data } = await axios.get(`${apiUrl}/auth/api-key`); | |
return data.api_key as string | null; | |
}, | |
staleTime: 10 * 60 * 1000, // 10 minutes | |
}); | |
// Fetch API key with React Query | |
const { data: apiKey, isLoading } = useQuery({ | |
queryKey: ['api-key', user?.id], | |
queryFn: async () => { | |
const { data } = await axios.get(`${apiUrl}/auth/api-key`); | |
return data.api_key as string | null; | |
}, | |
staleTime: 10 * 60 * 1000, // 10 minutes | |
enabled: !!user?.id, | |
}); |
🤖 Prompt for AI Agents
In src/components/api/ApiKey.tsx around lines 41 to 49, the query uses a global
['api-key'] which can leak one user's API key to another after sign-out/in;
change the queryKey to include the current user's id (e.g. ['api-key', user.id])
and add enabled: !!user?.id so the query only runs when a user is present; also
update any mutations that create/delete/regenerate the API key to invalidate or
update the same scoped key (['api-key', user.id]) instead of the global key so
cache updates are per-user and no stale key is shown across logins.
PR Title
Improve app startup reliability and fix lag when switching pages; add API key caching
Summary
Changes
Backend/docker
maxun/docker-compose.yml
:pg_isready -U postgres
pg_isready -U ${DB_USER}
"${DB_PORT:-5432}:${DB_PORT:-5432}"
"${DB_HOST_PORT:-5433}:5432"
(container stays on 5432; host exposes 5433)DB_HOST=postgres
to.env
so services connect via the Docker network nameDB_PORT=5432
for internal connectionsDB_HOST_PORT=5433
to map host port without collisionsFrontend performance (page switch lag)
maxun/src/pages/MainPage.tsx
:robots
,runs
,proxy
,apikey
) and switched visibility with inlinedisplay: none/block
instead of unmounting/mountinguseMemo
to avoid unnecessary re-rendersFrontend data fetching (API key)
maxun/src/components/api/ApiKey.tsx
:useEffect
+axios
with React QueryuseQuery
keyed as['api-key']
withstaleTime
10 minuseMutation
, updates cache viaqueryClient.setQueryData(['api-key'], newKey)
useMutation
, clears cache viaqueryClient.setQueryData(['api-key'], null)
Rationale
postgres
user while the service usedDB_USER
DB_HOST=postgres
caused services to try localhost instead of the Docker network hostFiles Touched
maxun/docker-compose.yml
${DB_HOST_PORT:-5433}:5432
.env
(created/updated)DB_HOST=postgres
DB_PORT=5432
DB_HOST_PORT=5433
maxun/src/pages/MainPage.tsx
useMemo
forrobotsView
,runsView
,proxyView
,apiKeyView
display: none/block
wrappersmaxun/src/components/api/ApiKey.tsx
@tanstack/react-query
for fetching/mutations and cache updatesHow to Test
Environment and Docker
.env
contains:DB_HOST=postgres
DB_PORT=5432
DB_HOST_PORT=5433
cd /Users/anik/Code/OPEN-SOURCE/maxun && docker-compose up -d
docker-compose ps
(backend on 8080, frontend on 5173, Postgres mapped to 5433)psql "postgresql://<DB_USER>:<DB_PASSWORD>@localhost:5433/<DB_NAME>"
Frontend performance
http://localhost:5173
Robots
,Runs
,Proxy
,API Key
in the side menu repeatedlyAPI Key page
API Key
Compatibility/Notes
GlobalInfoProvider
), so no provider changes requiredSummary by CodeRabbit
Refactor
Chores