Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis pull request introduces SWR (stale-while-revalidate) as the primary data-fetching library, replacing manual API calls and localStorage-based caching across multiple dashboard pages. It also adds React.memo optimizations, removes the legacy useSmartRefresh utility, updates event listeners to passive mode, and restructures the production checklist into consolidated categories. Changes
Sequence DiagramsequenceDiagram
participant Component as Dashboard Page
participant SWR as SWR Hook
participant Fetcher as swrFetcher
participant API as API Client
participant Backend as Backend Server
Component->>SWR: useSWR(key, swrFetcher, SWR_CONFIG)
SWR->>Fetcher: Fetch triggered (on mount/revalidate)
Fetcher->>API: api.get(url)
API->>Backend: HTTP GET request
Backend-->>API: Response data
API-->>Fetcher: r.data
Fetcher-->>SWR: Return data
SWR-->>Component: {data, isLoading, mutate}
Note over Component: Component renders with SWR data
Component->>Component: User action (save/delete/update)
Component->>SWR: mutate() to trigger refresh
SWR->>Fetcher: Refetch with same key
Fetcher->>API: api.get(url)
API->>Backend: HTTP GET request
Backend-->>API: Updated data
API-->>Fetcher: r.data
Fetcher-->>SWR: Return updated data
SWR-->>Component: {data (refreshed), isLoading, mutate}
Component->>Component: Re-render with fresh data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 2
🤖 Fix all issues with AI agents
In `@src/app/`(dashboard)/home/page.tsx:
- Around line 98-107: In the useMemo that builds projects from projectsRaw
(inside the projects constant), avoid spreading backend fields after you set
normalized values; change the object construction for SubcontractorProjectApi
cases to spread p first and then explicitly set id, name, location, and status
so backend keys like id (assignment id) cannot overwrite the normalized
id/project_name mapping; update the map callback that returns ({ ...p, id:
p.project_id, name: p.project_name, location: p.project_location, status:
p.is_active ? "active" : "inactive" }) to enforce the correct values.
In `@src/components/forms/CreateBookingForm.tsx`:
- Around line 556-570: The effect currently dispatches SET_ASSET_ERROR whenever
assetsSwrError is truthy which clears assets even if assetsResponse (stale data)
exists; change the logic in the useEffect that reads assetsSwrError and
assetsResponse so you only dispatchAsset({ type: "SET_ASSET_ERROR", error: true
}) when assetsSwrError is truthy AND assetsResponse is falsy/empty, otherwise
preserve existing assets and only dispatch SET_ASSETS from the normalizedAssets
mapping; update the useEffect that references assetsSwrError, assetsResponse,
dispatchAsset, and AssetOption to implement this conditional check.
🧹 Nitpick comments (4)
src/app/context/AuthContext.tsx (1)
233-235: Implicit return in forEach callback.The arrow function implicitly returns the result of
addEventListener, which Biome flags as suspicious. While functionally harmless, wrapping in braces makes intent clearer:🔧 Suggested fix
- events.forEach((e) => - window.addEventListener(e, onActivity, { passive: true }), - ); + events.forEach((e) => { + window.addEventListener(e, onActivity, { passive: true }); + });src/lib/swr.ts (1)
4-4: Consider adding error handling or retry configuration.The fetcher relies on SWR's default error handling. For production, you may want to consider adding
onErrorRetrytoSWR_CONFIGfor customized retry behavior (e.g., no retry on 4xx errors).💡 Optional: Add error retry configuration
export const SWR_CONFIG: SWRConfiguration = { revalidateOnFocus: true, revalidateOnReconnect: true, refreshInterval: 5 * 60 * 1000, // 5 min dedupingInterval: 30_000, // 30s dedup + onErrorRetry: (error, _key, _config, revalidate, { retryCount }) => { + // Don't retry on 4xx errors + if (error?.response?.status >= 400 && error?.response?.status < 500) return; + // Max 3 retries + if (retryCount >= 3) return; + // Retry after 5s + setTimeout(() => revalidate({ retryCount }), 5000); + }, };src/components/multicalendar/MulticalendarPage.tsx (1)
97-107: Date range is fixed at mount time.The date range is computed once when the component mounts (empty dependency array means it never recalculates). If a user keeps the page open for extended periods, the 45-day window won't shift with the current date.
This is likely acceptable for typical usage, but worth noting if users leave the dashboard open overnight.
src/app/(dashboard)/assets/page.tsx (1)
60-66: Consider importingAssetListResponsefrom shared types.This interface duplicates
AssetListResponsefromsrc/types/index.ts. While not critical, importing from the shared types file would reduce duplication and ensure consistency.💡 Optional: Import from shared types
-import { TransformedAsset, getApiErrorMessage } from "@/types"; +import { TransformedAsset, getApiErrorMessage, AssetListResponse } from "@/types"; -interface AssetListResponse { - assets: AssetFromBackend[]; - total: number; - skip: number; - limit: number; - has_more: boolean; -}Note: You'd need to adjust the generic type or keep a local type alias if
AssetListResponseusesApiAssetinstead ofAssetFromBackend.
| const projects = useMemo<ApiProject[]>(() => { | ||
| if (!projectsRaw) return []; | ||
| if (user?.role === "subcontractor") { | ||
| setSubcontractorCount(0); | ||
| return; | ||
| return (projectsRaw as SubcontractorProjectApi[]).map((p) => ({ | ||
| id: p.project_id, | ||
| name: p.project_name, | ||
| location: p.project_location, | ||
| status: p.is_active ? "active" : "inactive", | ||
| ...p, | ||
| })); |
There was a problem hiding this comment.
Prevent backend fields from overwriting normalized project IDs.
Spreading ...p after setting id/name/location can override them if the backend payload includes id (e.g., assignment id), which would break project scoping. Spread first, then override with the normalized fields.
🛠️ Suggested fix (normalize after spread)
return (projectsRaw as SubcontractorProjectApi[]).map((p) => ({
- id: p.project_id,
- name: p.project_name,
- location: p.project_location,
- status: p.is_active ? "active" : "inactive",
- ...p,
+ ...p,
+ id: p.project_id,
+ name: p.project_name,
+ location: p.project_location,
+ status: p.is_active ? "active" : "inactive",
}));📝 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.
| const projects = useMemo<ApiProject[]>(() => { | |
| if (!projectsRaw) return []; | |
| if (user?.role === "subcontractor") { | |
| setSubcontractorCount(0); | |
| return; | |
| return (projectsRaw as SubcontractorProjectApi[]).map((p) => ({ | |
| id: p.project_id, | |
| name: p.project_name, | |
| location: p.project_location, | |
| status: p.is_active ? "active" : "inactive", | |
| ...p, | |
| })); | |
| const projects = useMemo<ApiProject[]>(() => { | |
| if (!projectsRaw) return []; | |
| if (user?.role === "subcontractor") { | |
| return (projectsRaw as SubcontractorProjectApi[]).map((p) => ({ | |
| ...p, | |
| id: p.project_id, | |
| name: p.project_name, | |
| location: p.project_location, | |
| status: p.is_active ? "active" : "inactive", | |
| })); |
🤖 Prompt for AI Agents
In `@src/app/`(dashboard)/home/page.tsx around lines 98 - 107, In the useMemo that
builds projects from projectsRaw (inside the projects constant), avoid spreading
backend fields after you set normalized values; change the object construction
for SubcontractorProjectApi cases to spread p first and then explicitly set id,
name, location, and status so backend keys like id (assignment id) cannot
overwrite the normalized id/project_name mapping; update the map callback that
returns ({ ...p, id: p.project_id, name: p.project_name, location:
p.project_location, status: p.is_active ? "active" : "inactive" }) to enforce
the correct values.
| useEffect(() => { | ||
| const assetString = localStorage.getItem(`assets_${userId}`); | ||
| if (!assetString) { | ||
| if (assetsSwrError) { | ||
| dispatchAsset({ type: "SET_ASSET_ERROR", error: true }); | ||
| return; | ||
| } | ||
| try { | ||
| const parsedAssets = JSON.parse(assetString) as ApiAsset[]; | ||
| const normalizedAssets: AssetOption[] = parsedAssets.map((asset) => { | ||
| const cached = asset as ApiAsset & { | ||
| assetKey?: string; | ||
| assetTitle?: string; | ||
| }; | ||
| return { | ||
| ...asset, | ||
| assetKey: cached.assetKey ?? asset.id, | ||
| assetTitle: cached.assetTitle ?? asset.name, | ||
| }; | ||
| }); | ||
| dispatchAsset({ type: "SET_ASSETS", assets: normalizedAssets }); | ||
| } catch (err) { | ||
| console.error("Error parsing assets:", err); | ||
| dispatchAsset({ type: "SET_ASSET_ERROR", error: true }); | ||
| } | ||
| }, [userId]); | ||
| if (!assetsResponse) return; | ||
| const normalizedAssets: AssetOption[] = (assetsResponse.assets || []).map( | ||
| (asset) => ({ | ||
| ...asset, | ||
| assetKey: asset.id, | ||
| assetTitle: asset.name, | ||
| }), | ||
| ); | ||
| dispatchAsset({ type: "SET_ASSETS", assets: normalizedAssets }); | ||
| }, [assetsResponse, assetsSwrError]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "CreateBookingForm.tsx" -type f 2>/dev/null | head -20Repository: Ross1116/sitespace-app
Length of output: 48
🏁 Script executed:
git ls-files | grep -i "createbookingform" | head -20Repository: Ross1116/sitespace-app
Length of output: 108
🏁 Script executed:
find . -name "*.tsx" -path "*components/forms*" 2>/dev/null | head -20Repository: Ross1116/sitespace-app
Length of output: 286
🏁 Script executed:
wc -l src/components/forms/CreateBookingForm.tsxRepository: Ross1116/sitespace-app
Length of output: 113
🏁 Script executed:
sed -n '540,580p' src/components/forms/CreateBookingForm.tsx | cat -nRepository: Ross1116/sitespace-app
Length of output: 1611
🏁 Script executed:
grep -n "assetsResponse\|assetsSwrError" src/components/forms/CreateBookingForm.tsx | head -20Repository: Ross1116/sitespace-app
Length of output: 330
🏁 Script executed:
grep -n "dispatchAsset\|SET_ASSET_ERROR\|SET_ASSETS" src/components/forms/CreateBookingForm.tsx | head -30Repository: Ross1116/sitespace-app
Length of output: 683
🏁 Script executed:
grep -n "const dispatchAsset\|function.*dispatchAsset" src/components/forms/CreateBookingForm.tsxRepository: Ross1116/sitespace-app
Length of output: 48
🏁 Script executed:
rg "useReducer.*asset" src/components/forms/CreateBookingForm.tsx -A 5Repository: Ross1116/sitespace-app
Length of output: 48
🏁 Script executed:
sed -n '359,365p' src/components/forms/CreateBookingForm.tsx | cat -nRepository: Ross1116/sitespace-app
Length of output: 354
🏁 Script executed:
sed -n '165,175p' src/components/forms/CreateBookingForm.tsx | cat -nRepository: Ross1116/sitespace-app
Length of output: 560
🏁 Script executed:
sed -n '350,375p' src/components/forms/CreateBookingForm.tsx | cat -nRepository: Ross1116/sitespace-app
Length of output: 975
🏁 Script executed:
grep -n "assetError" src/components/forms/CreateBookingForm.tsx | head -20Repository: Ross1116/sitespace-app
Length of output: 357
🏁 Script executed:
sed -n '1,50p' src/components/forms/CreateBookingForm.tsx | cat -nRepository: Ross1116/sitespace-app
Length of output: 1723
🏁 Script executed:
grep -n "useSWR\|import.*swr" src/components/forms/CreateBookingForm.tsxRepository: Ross1116/sitespace-app
Length of output: 208
🏁 Script executed:
grep -r "useSWR" src --include="*.ts" --include="*.tsx" | head -5Repository: Ross1116/sitespace-app
Length of output: 482
🏁 Script executed:
cat src/lib/swr.tsRepository: Ross1116/sitespace-app
Length of output: 405
🏁 Script executed:
sed -n '905,915p' src/components/forms/CreateBookingForm.tsx | cat -nRepository: Ross1116/sitespace-app
Length of output: 653
🏁 Script executed:
sed -n '550,575p' src/components/forms/CreateBookingForm.tsx | cat -nRepository: Ross1116/sitespace-app
Length of output: 1021
🏁 Script executed:
sed -n '470,480p' src/components/forms/CreateBookingForm.tsx | cat -nRepository: Ross1116/sitespace-app
Length of output: 377
🏁 Script executed:
web_search
SWR stale-while-revalidate behavior error with cached dataRepository: Ross1116/sitespace-app
Length of output: 158
🌐 Web query:
SWR stale-while-revalidate error behavior stale data cached
💡 Result:
In SWR (the React data-fetching library), the “stale-while-revalidate” pattern means:
- If there’s cached data for a key, SWR returns it immediately (“stale”) and starts a revalidation request in the background. (You can control this on mount via
revalidateIfStale,revalidateOnMount, etc.) [1] - If that background revalidation request fails, SWR keeps the previously cached data and also exposes the error. In other words,
dataanderrorcan both be set at the same time, so you can keep rendering the last good data while showing an error state. The cache is not overwritten by the failed response. [2] - Retries on error happen automatically (exponential backoff) unless you customize/disable them with
onErrorRetry. [2]
If you instead meant HTTP/CDN caching headers (also often described as “stale-while-revalidate”):
Cache-Control: stale-while-revalidate=Nallows a cache to serve a stale response while it revalidates in the background. [3]- Serving stale specifically when the origin errors is a different directive:
stale-if-error=N. Withoutstale-if-error, an error from the origin is typically not replaced by stale content. [3]
Sources:
[1] SWR docs – Automatic Revalidation https://swr.vercel.app/docs/revalidation
[2] SWR docs – Error Handling https://swr.vercel.app/docs/error-handling
[3] RFC 5861 (stale-while-revalidate, stale-if-error) https://www.rfc-editor.org/rfc/rfc5861.html
🏁 Script executed:
sed -n '139,150p' src/components/forms/CreateBookingForm.tsx | cat -nRepository: Ross1116/sitespace-app
Length of output: 466
🏁 Script executed:
sed -n '135,165p' src/components/forms/CreateBookingForm.tsx | cat -nRepository: Ross1116/sitespace-app
Length of output: 1124
🏁 Script executed:
sed -n '350,375p' src/components/forms/CreateBookingForm.tsx | cat -nRepository: Ross1116/sitespace-app
Length of output: 975
🏁 Script executed:
grep -n "assetError\|assets:" src/components/forms/CreateBookingForm.tsx | grep -A2 -B2 "908"Repository: Ross1116/sitespace-app
Length of output: 197
Only set error when no data exists; preserve stale assets during transient SWR revalidation failures.
SWR can emit error while data remains available (stale-while-revalidate pattern). The current early return on assetsSwrError clears the asset cache via the SET_ASSET_ERROR reducer action, blocking selection even when stale assets are available. Only signal error when both error and no cached data exist.
🛠️ Suggested fix
useEffect(() => {
- if (assetsSwrError) {
- dispatchAsset({ type: "SET_ASSET_ERROR", error: true });
- return;
- }
- if (!assetsResponse) return;
+ if (assetsSwrError && !assetsResponse) {
+ dispatchAsset({ type: "SET_ASSET_ERROR", error: true });
+ return;
+ }
+ if (!assetsResponse) return;
const normalizedAssets: AssetOption[] = (assetsResponse.assets || []).map(
(asset) => ({
...asset,
assetKey: asset.id,
assetTitle: asset.name,
}),
);
dispatchAsset({ type: "SET_ASSETS", assets: normalizedAssets });
}, [assetsResponse, assetsSwrError]);📝 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.
| useEffect(() => { | |
| const assetString = localStorage.getItem(`assets_${userId}`); | |
| if (!assetString) { | |
| if (assetsSwrError) { | |
| dispatchAsset({ type: "SET_ASSET_ERROR", error: true }); | |
| return; | |
| } | |
| try { | |
| const parsedAssets = JSON.parse(assetString) as ApiAsset[]; | |
| const normalizedAssets: AssetOption[] = parsedAssets.map((asset) => { | |
| const cached = asset as ApiAsset & { | |
| assetKey?: string; | |
| assetTitle?: string; | |
| }; | |
| return { | |
| ...asset, | |
| assetKey: cached.assetKey ?? asset.id, | |
| assetTitle: cached.assetTitle ?? asset.name, | |
| }; | |
| }); | |
| dispatchAsset({ type: "SET_ASSETS", assets: normalizedAssets }); | |
| } catch (err) { | |
| console.error("Error parsing assets:", err); | |
| dispatchAsset({ type: "SET_ASSET_ERROR", error: true }); | |
| } | |
| }, [userId]); | |
| if (!assetsResponse) return; | |
| const normalizedAssets: AssetOption[] = (assetsResponse.assets || []).map( | |
| (asset) => ({ | |
| ...asset, | |
| assetKey: asset.id, | |
| assetTitle: asset.name, | |
| }), | |
| ); | |
| dispatchAsset({ type: "SET_ASSETS", assets: normalizedAssets }); | |
| }, [assetsResponse, assetsSwrError]); | |
| useEffect(() => { | |
| if (assetsSwrError && !assetsResponse) { | |
| dispatchAsset({ type: "SET_ASSET_ERROR", error: true }); | |
| return; | |
| } | |
| if (!assetsResponse) return; | |
| const normalizedAssets: AssetOption[] = (assetsResponse.assets || []).map( | |
| (asset) => ({ | |
| ...asset, | |
| assetKey: asset.id, | |
| assetTitle: asset.name, | |
| }), | |
| ); | |
| dispatchAsset({ type: "SET_ASSETS", assets: normalizedAssets }); | |
| }, [assetsResponse, assetsSwrError]); |
🤖 Prompt for AI Agents
In `@src/components/forms/CreateBookingForm.tsx` around lines 556 - 570, The
effect currently dispatches SET_ASSET_ERROR whenever assetsSwrError is truthy
which clears assets even if assetsResponse (stale data) exists; change the logic
in the useEffect that reads assetsSwrError and assetsResponse so you only
dispatchAsset({ type: "SET_ASSET_ERROR", error: true }) when assetsSwrError is
truthy AND assetsResponse is falsy/empty, otherwise preserve existing assets and
only dispatch SET_ASSETS from the normalizedAssets mapping; update the useEffect
that references assetsSwrError, assetsResponse, dispatchAsset, and AssetOption
to implement this conditional check.
1 participant
Summary by CodeRabbit
New Features
Performance
Refactor
Chores