fix: resolve counter display inconsistency due to Next.js caching #457
Conversation
WalkthroughRemoved Suspense and fallback UI from the marketing counter page, rendering CurrentCount directly. Corresponding “loading_counter” translation key was deleted from English and French locale files. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Page as Counter Page
participant CurrentCount
Note over Page: Updated flow (Suspense removed)
User->>Page: Navigate to /[locale]/counter
Page->>CurrentCount: Render component directly
CurrentCount-->>Page: Display count
Page-->>User: Rendered page (no loading fallback)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (3)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (4)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #457 +/- ##
========================================
+ Coverage 5.74% 5.75% +0.01%
========================================
Files 44 44
Lines 1532 1529 -3
Branches 44 44
========================================
Hits 88 88
+ Misses 1417 1414 -3
Partials 27 27 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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 (1)
src/components/CurrentCount.tsx (1)
16-19: Do not await headers(); harden parsing to avoid NaN ids leaking to the DB filter
headers()is synchronous;awaitis unnecessary noise.Number(...) ?? 0does not guard againstNaN. If the header is present but non-numeric,idbecomesNaN, which can propagate to your query.Use a safe parse with a fallback to
0.Apply this diff:
- const id = Number((await headers()).get('x-e2e-random-id')) ?? 0; + const idHeader = headers().get('x-e2e-random-id'); + const id = Number.isFinite(Number(idHeader)) ? Number(idHeader) : 0;
🧹 Nitpick comments (1)
src/components/CurrentCount.tsx (1)
9-29: Optional: prefer page/route-level dynamic config; keep component-level noStore as immediate fixFor long-term clarity, consider driving caching behavior from the segment level:
- In the page or route that renders this component, export
revalidate = 0ordynamic = 'force-dynamic'. Component-levelnoStore()is fine, but segment-level config avoids surprises when composing components.If you adopt segment-level config, you can remove
noStore()here. Otherwise, keepingnoStore()locally is acceptable.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/components/CurrentCount.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
Review the Typescript and React code for conformity with best practices in React, and Typescript. Highlight any deviations.
Files:
src/components/CurrentCount.tsx
⏰ 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). (2)
- GitHub Check: Build with 24.x
- GitHub Check: Build with 22.x
🔇 Additional comments (2)
src/components/CurrentCount.tsx (2)
4-4: Remove invalid import; use next/cache noStore() to disable caching
next/serverdoes not exportconnection. This will fail type-checking and at runtime. If the intent is to prevent caching to fix the counter inconsistency, useunstable_noStorefromnext/cacheinstead.Apply this diff:
-import { connection } from 'next/server'; +import { unstable_noStore as noStore } from 'next/cache';Likely an incorrect or invalid review comment.
9-29: ✅ All clear: no stray caching or stale connectionsI’ve verified that:
connection()is only imported and called in src/components/CurrentCount.tsx- No other
await headers()usages exist beyond the intended component and API route- There are no segment-level
dynamicorrevalidateexports anywhere in the TS filesWith no explicit caching configuration and Next.js defaulting to dynamic SSR, each request—including E2E tests using
x-e2e-random-id—will fetch fresh data.
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 (1)
src/app/[locale]/api/counter/route.ts (1)
21-21: NaN bug for id default and unnecessary await; use request.headers and coalesce before Number().Number(undefined) yields NaN, so the nullish coalescing after Number() never applies. Also, headers() is synchronous and not needed here; in a route handler, prefer request.headers.
Apply this diff:
- const id = Number((await headers()).get('x-e2e-random-id')) ?? 0; + const id = Number(request.headers.get('x-e2e-random-id') ?? 0);
🧹 Nitpick comments (1)
src/app/[locale]/api/counter/route.ts (1)
3-3: Prefer request.headers in route handlers; drop next/headers import once above change is applied.This keeps the handler self-contained and avoids the dynamic function from next/headers.
Apply this diff:
-import { headers } from 'next/headers';
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/app/[locale]/api/counter/route.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
Review the Typescript and React code for conformity with best practices in React, and Typescript. Highlight any deviations.
Files:
src/app/[locale]/api/counter/route.ts
🧬 Code graph analysis (1)
src/app/[locale]/api/counter/route.ts (1)
src/app/[locale]/(marketing)/counter/page.tsx (2)
Counter(24-60)generateMetadata(9-22)
⏰ 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). (1)
- GitHub Check: Build with 24.x
🔇 Additional comments (1)
src/app/[locale]/api/counter/route.ts (1)
2-2: Import of revalidatePath is appropriate for this use-case.Good call adding revalidation at the write path to keep the localized counter page fresh.
| @@ -30,6 +31,8 @@ export const PUT = async (request: Request) => { | |||
|
|
|||
| logger.info('Counter has been incremented'); | |||
|
|
|||
| revalidatePath('/[locale]/counter', 'page'); | |||
There was a problem hiding this comment.
Fix revalidatePath target: use the concrete locale path, not the bracketed segment.
revalidatePath expects a concrete pathname; passing '/[locale]/counter' won’t hit the cached entry for '/en/counter', '/fr/counter', etc. Use the actual locale from the dynamic route params.
Apply this diff to the changed line:
- revalidatePath('/[locale]/counter', 'page');
+ revalidatePath(`/${params.locale}/counter`, 'page');Additionally, update the handler signature to access params (outside the changed lines):
export const PUT = async (
request: Request,
{ params }: { params: { locale: string } }
) => {
// ...unchanged code...
};🤖 Prompt for AI Agents
In src/app/[locale]/api/counter/route.ts around line 34, revalidatePath is being
called with the dynamic segment '/[locale]/counter' which won't match concrete
cached paths; update the handler signature to accept params as: export const PUT
= async (request: Request, { params }: { params: { locale: string } }) => { ...
} and replace the revalidatePath call to use the actual locale from params (e.g.
revalidatePath using `/${params.locale}/counter` with the same 'page' type) so
the concrete locale path is revalidated.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/app/[locale]/(marketing)/counter/page.tsx (1)
31-31: Prefer keying the component at the parent to guarantee a full remount across requestsPutting
key={count}on a div insideCurrentCountonly remounts that DOM subtree. If the goal is to force React/Next to treatCurrentCountitself as a fresh node whenever the request-varying input changes, pass a key from the parent (e.g., derived fromx-e2e-random-id). This avoids any chance of RSC payload reuse across client navigations.Apply this diff within the selected lines:
- <CurrentCount /> + <CurrentCount key={e2eId} />And add the following outside the selected lines to define
e2eIdin this file:// at top-level imports import { headers } from 'next/headers'; // inside Counter(), above the return const e2eId = Number(headers().get('x-e2e-random-id')) || 0;Optional hardening: if you want an extra guard against caching at the route segment level, you can explicitly mark this page dynamic:
export const dynamic = 'force-dynamic'; // or export const revalidate = 0;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/app/[locale]/(marketing)/counter/page.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
Review the Typescript and React code for conformity with best practices in React, and Typescript. Highlight any deviations.
Files:
src/app/[locale]/(marketing)/counter/page.tsx
🧬 Code graph analysis (1)
src/app/[locale]/(marketing)/counter/page.tsx (1)
src/components/CurrentCount.tsx (1)
CurrentCount(8-26)
🔇 Additional comments (1)
src/app/[locale]/(marketing)/counter/page.tsx (1)
31-31: LGTM: Removing Suspense eliminates the cached fallback artifactDropping the Suspense boundary here is a reasonable way to prevent a stale fallback from being cached/rehydrated across navigations. Expect slightly higher TTFB for this section, which is likely acceptable given the trivial query.
|
🎉 This PR is included in version 5.1.7 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary by CodeRabbit