ISR Status #2377
ISR Status #2377
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a small ISR cache-status indicator to rule pages to make it easier to see whether content is being served from cache, regenerated, or stale.
Changes:
- Introduced a client-side ISR status badge that inspects Next.js cache headers and shows a colored dot with tooltip.
- Placed the ISR badge next to “Last updated by” metadata on the rule page header.
- Minor formatting changes in
GitHubMetadataloading UI and tooltip spacing.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| components/ui/isr-status-badge.tsx | Adds the ISR status badge component that fetches current page headers and renders a status indicator. |
| components/last-updated-by/index.tsx | Tweaks rendering and (currently) alters the loading lifecycle logic. |
| app/[filename]/ServerRulePage.tsx | Integrates the ISR badge into the rule page header layout. |
| .cache/gatsby-source-git/categories | Adds a cache/artifact file to the repo (likely unintended). |
Comments suppressed due to low confidence (1)
.cache/gatsby-source-git/categories:1
- A
.cache/...artifact is being committed to the repository. Cache/build outputs should typically be excluded from version control (add to.gitignore) and removed from the PR unless the project explicitly vendors this directory.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| setData(null); | ||
| } finally { | ||
| setLoading(false); | ||
| //setLoading(false); |
There was a problem hiding this comment.
loading is set to true at the start of the fetch, but setLoading(false) is commented out in finally, so this component can get stuck permanently rendering the loading state. Restore setLoading(false) in the finally block (or otherwise guarantee loading is cleared on completion/error).
| //setLoading(false); | |
| setLoading(false); |
| function getCacheHeaderValue(headers: Headers): { name: string; value: string | null } { | ||
| // Primary header emitted by Next.js ISR (varies by platform/version). | ||
| const next = headers.get("x-nextjs-cache"); | ||
| if (next) return { name: "x-nextjs-cache", value: next }; | ||
|
|
||
| return { name: "x-nextjs-cache", value: null }; | ||
| } | ||
|
|
||
| const copy: Record<NextCacheState, { label: string; detail?: string }> = { | ||
| HIT: { label: "Live", detail: "cached" }, | ||
| STALE: { label: "Stale", detail: "revalidating…" }, | ||
| MISS: { label: "Generating", detail: "first render" }, | ||
| unknown: { label: "Unknown" }, | ||
| }; |
There was a problem hiding this comment.
There are a few maintainability issues here: (1) getCacheHeaderValue always returns the same name and that name isn’t used anywhere, so the extra structure is unnecessary unless you plan to display it; (2) copy is defined but not used in this component; (3) getStyle takes isLoading, but the component returns early when loading, making that parameter/branch redundant. Consider removing unused constants/fields and simplifying these helpers, or wire them into the UI if they’re intended for display.
| if (isLoading) return ""; | ||
|
|
There was a problem hiding this comment.
There are a few maintainability issues here: (1) getCacheHeaderValue always returns the same name and that name isn’t used anywhere, so the extra structure is unnecessary unless you plan to display it; (2) copy is defined but not used in this component; (3) getStyle takes isLoading, but the component returns early when loading, making that parameter/branch redundant. Consider removing unused constants/fields and simplifying these helpers, or wire them into the UI if they’re intended for display.
| if (isLoading) return ""; |
Description
It is hard to know if the ISR has worked and what status it is in so we want to show a indicator of the ISR status on each rule page
Screenshot (optional)