fix homepage hydration and chat date rendering#63
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
There was a problem hiding this comment.
Pull request overview
Fixes SSR/hydration mismatches on the homepage by making the demo chat (and related date rendering) deterministic across locale/timezone, and adds regression coverage to prevent reintroductions.
Changes:
- Makes demo chat timestamps and the map demo’s initial state deterministic for stable SSR/hydration.
- Refactors chat date helpers to accept explicit locale/timeZone inputs and adds deterministic unit coverage.
- Adds a production Playwright regression test for homepage hydration and tightens a brittle listing form selector.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/dateUtils.ts | Adds explicit locale/timeZone-aware formatting helpers and deterministic chat date keying. |
| src/utils/dateUtils.test.ts | Adds unit tests ensuring deterministic chat date/timestamp rendering. |
| src/data/demo/threads.ts | Replaces runtime-generated demo timestamps with fixed ISO strings + reference time. |
| src/components/PeelsMapDemo/PeelsMapDemo.tsx | Makes initial rotation deterministic while preserving post-mount randomness. |
| src/components/NewsletterIssueTile/NewsletterIssueTile.tsx | Simplifies the Next.js Link usage by removing prefetch={false}. |
| src/components/ChatWindow/ChatWindow.tsx | Passes explicit locale/timeZone/now options into date rendering and uses stable date keys for grouping. |
| src/components/ChatMessage/ChatMessage.tsx | Passes locale/timeZone into timestamp formatting for deterministic rendering. |
| package.json | Adds a test:unit script for the new deterministic date utils tests. |
| e2e/listings.spec.ts | Scopes selectors to the listing form to reduce brittleness. |
| e2e/home.spec.ts | Adds a production hydration regression test comparing SSR vs hydrated chat labels/timestamps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const serverContext = await browser.newContext({ | ||
| baseURL: "http://127.0.0.1:3000", | ||
| javaScriptEnabled: false, | ||
| }); | ||
| const serverPage = await serverContext.newPage(); | ||
| await serverPage.goto("/"); | ||
| const serverDayLabels = await serverPage |
There was a problem hiding this comment.
The JS-disabled serverContext hard-codes baseURL and doesn’t copy over the project’s locale / extraHTTPHeaders (see playwright.shared.ts), so the server-rendered snapshot could be in a different locale than the hydrated page. This can make the hydration regression test flaky or miss issues. Consider reusing the configured baseURL and passing the same locale/extraHTTPHeaders into browser.newContext(...).
| if (options?.useRelativeDayLabels) { | ||
| if (dateKey === referenceDateKey) { | ||
| return "Today"; | ||
| } | ||
|
|
||
| if (compareDate.getTime() === compareYesterday.getTime()) { | ||
| return formatRelativeDayLabel(1, locale); | ||
| } | ||
| const yesterday = new Date(referenceDate); | ||
| yesterday.setUTCDate(yesterday.getUTCDate() - 1); | ||
|
|
||
| if (diffDays < 7) { | ||
| return new Intl.DateTimeFormat(locale, { | ||
| weekday: "long", | ||
| }).format(date); | ||
| if ( | ||
| dateKey === | ||
| getChatDateKey(yesterday, { | ||
| timeZone, | ||
| }) | ||
| ) { | ||
| return "Yesterday"; | ||
| } |
There was a problem hiding this comment.
formatWeekday returns hard-coded "Today"/"Yesterday" when useRelativeDayLabels is enabled, which ignores the provided locale and will render English labels even for non-English locales. Consider using Intl.RelativeTimeFormat(locale, { numeric: "auto" }) (optionally with capitalization handled at the UI layer) or allow callers to supply localized labels so the demo chat respects the active locale while staying deterministic.
| const { locale, timeZone } = getResolvedOptions(options); | ||
| const referenceDate = toDate(options?.now ?? dateValue); | ||
| const dateKey = getChatDateKey(dateValue, { timeZone }); | ||
| const referenceDateKey = getChatDateKey(referenceDate, { timeZone }); | ||
|
|
||
| if (compareDate.getTime() === compareToday.getTime()) { | ||
| return formatRelativeDayLabel(0, locale); | ||
| } | ||
| if (options?.useRelativeDayLabels) { | ||
| if (dateKey === referenceDateKey) { | ||
| return "Today"; | ||
| } | ||
|
|
||
| if (compareDate.getTime() === compareYesterday.getTime()) { | ||
| return formatRelativeDayLabel(1, locale); | ||
| } | ||
| const yesterday = new Date(referenceDate); | ||
| yesterday.setUTCDate(yesterday.getUTCDate() - 1); | ||
|
|
||
| if (diffDays < 7) { | ||
| return new Intl.DateTimeFormat(locale, { | ||
| weekday: "long", | ||
| }).format(date); | ||
| if ( | ||
| dateKey === | ||
| getChatDateKey(yesterday, { | ||
| timeZone, | ||
| }) | ||
| ) { | ||
| return "Yesterday"; | ||
| } | ||
| } | ||
|
|
||
| const shouldIncludeYear = | ||
| getDatePart(dateValue, "year", { timeZone }) !== | ||
| getDatePart(referenceDate, "year", { timeZone }); | ||
|
|
There was a problem hiding this comment.
When options.now is omitted, referenceDate falls back to dateValue, which makes shouldIncludeYear always false (the compared years will match). In non-demo chat (where now is currently undefined), this means the year will never be included in the weekday header, even for messages from a different year. Consider defaulting referenceDate to the current time (or requiring callers to pass a stable now), or otherwise ensuring cross-year messages include the year.
| const yesterday = new Date(referenceDate); | ||
| yesterday.setUTCDate(yesterday.getUTCDate() - 1); | ||
|
|
||
| if (diffDays < 7) { | ||
| return new Intl.DateTimeFormat(locale, { | ||
| weekday: "long", | ||
| }).format(date); | ||
| if ( | ||
| dateKey === | ||
| getChatDateKey(yesterday, { | ||
| timeZone, | ||
| }) |
There was a problem hiding this comment.
The "Yesterday" calculation subtracts a day using setUTCDate/getUTCDate, which is only correct for UTC. If a non-UTC timeZone is ever passed, the computed "yesterday" key can be wrong around timezone boundaries. Consider computing the previous day based on the specified timeZone (e.g., derive the reference date key in that timezone and subtract one calendar day in that same calendar system).
| setChatRenderOptions((previousOptions) => ({ | ||
| ...previousOptions, | ||
| locale, | ||
| })); | ||
| }, [locale]); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
chatRenderOptions is updated in two effects on locale changes (the [locale] effect and the [isDemo, locale] effect), which causes redundant state updates/renders and makes the update logic harder to follow. Consider removing the [locale] effect and letting the [isDemo, locale] effect fully own chatRenderOptions updates.
| setChatRenderOptions((previousOptions) => ({ | |
| ...previousOptions, | |
| locale, | |
| })); | |
| }, [locale]); | |
| useEffect(() => { |
| timeZone: CHAT_RENDER_TIME_ZONE, | ||
| }) !== | ||
| getChatDateKey(messages[index - 1].created_at, { | ||
| timeZone: CHAT_RENDER_TIME_ZONE, |
There was a problem hiding this comment.
showDateHeader groups messages using getChatDateKey(..., { timeZone: CHAT_RENDER_TIME_ZONE }) instead of the chatRenderOptions.timeZone used for rendering labels/timestamps. Even if they’re currently the same, this can drift and cause headers to split differently than the displayed dates. Consider using chatRenderOptions.timeZone here for consistency.
| timeZone: CHAT_RENDER_TIME_ZONE, | |
| }) !== | |
| getChatDateKey(messages[index - 1].created_at, { | |
| timeZone: CHAT_RENDER_TIME_ZONE, | |
| timeZone: chatRenderOptions.timeZone, | |
| }) !== | |
| getChatDateKey(messages[index - 1].created_at, { | |
| timeZone: chatRenderOptions.timeZone, |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function getDatePart( | ||
| dateValue: string | Date, | ||
| part: "year" | "month" | "day", | ||
| { timeZone = CHAT_RENDER_TIME_ZONE }: Pick<DateFormatOptions, "timeZone"> = {} | ||
| ) { | ||
| return new Intl.DateTimeFormat("en-CA", { | ||
| timeZone, | ||
| year: "numeric", | ||
| month: "2-digit", | ||
| day: "2-digit", | ||
| }) | ||
| .formatToParts(toDate(dateValue)) | ||
| .find((datePart) => datePart.type === part)?.value; | ||
| } | ||
|
|
||
| export function getChatDateKey( | ||
| dateValue: string | Date, | ||
| options?: Pick<DateFormatOptions, "timeZone"> | ||
| ) { | ||
| const year = getDatePart(dateValue, "year", options); | ||
| const month = getDatePart(dateValue, "month", options); | ||
| const day = getDatePart(dateValue, "day", options); |
There was a problem hiding this comment.
getChatDateKey() currently constructs three separate Intl.DateTimeFormat instances (one per part via getDatePart) for every call. In ChatWindow this runs multiple times per message render, which can become a noticeable perf cost for long threads. Consider deriving the key using a single formatter call (or memoizing/caching formatters by timeZone) so grouping by day doesn't allocate multiple formatters per message.
| function getDatePart( | |
| dateValue: string | Date, | |
| part: "year" | "month" | "day", | |
| { timeZone = CHAT_RENDER_TIME_ZONE }: Pick<DateFormatOptions, "timeZone"> = {} | |
| ) { | |
| return new Intl.DateTimeFormat("en-CA", { | |
| timeZone, | |
| year: "numeric", | |
| month: "2-digit", | |
| day: "2-digit", | |
| }) | |
| .formatToParts(toDate(dateValue)) | |
| .find((datePart) => datePart.type === part)?.value; | |
| } | |
| export function getChatDateKey( | |
| dateValue: string | Date, | |
| options?: Pick<DateFormatOptions, "timeZone"> | |
| ) { | |
| const year = getDatePart(dateValue, "year", options); | |
| const month = getDatePart(dateValue, "month", options); | |
| const day = getDatePart(dateValue, "day", options); | |
| const chatDateKeyFormatterCache = new Map<string, Intl.DateTimeFormat>(); | |
| function getChatDateKeyFormatter(timeZone = CHAT_RENDER_TIME_ZONE) { | |
| const cachedFormatter = chatDateKeyFormatterCache.get(timeZone); | |
| if (cachedFormatter) { | |
| return cachedFormatter; | |
| } | |
| const formatter = new Intl.DateTimeFormat("en-CA", { | |
| timeZone, | |
| year: "numeric", | |
| month: "2-digit", | |
| day: "2-digit", | |
| }); | |
| chatDateKeyFormatterCache.set(timeZone, formatter); | |
| return formatter; | |
| } | |
| function getChatDateParts( | |
| dateValue: string | Date, | |
| { timeZone = CHAT_RENDER_TIME_ZONE }: Pick<DateFormatOptions, "timeZone"> = {} | |
| ) { | |
| const parts = getChatDateKeyFormatter(timeZone).formatToParts(toDate(dateValue)); | |
| return { | |
| year: parts.find((datePart) => datePart.type === "year")?.value, | |
| month: parts.find((datePart) => datePart.type === "month")?.value, | |
| day: parts.find((datePart) => datePart.type === "day")?.value, | |
| }; | |
| } | |
| function getDatePart( | |
| dateValue: string | Date, | |
| part: "year" | "month" | "day", | |
| options: Pick<DateFormatOptions, "timeZone"> = {} | |
| ) { | |
| return getChatDateParts(dateValue, options)[part]; | |
| } | |
| export function getChatDateKey( | |
| dateValue: string | Date, | |
| options?: Pick<DateFormatOptions, "timeZone"> | |
| ) { | |
| const { year, month, day } = getChatDateParts(dateValue, options); |
| @@ -15,6 +15,7 @@ | |||
| "i18n:check": "node scripts/check-i18n-messages.mjs", | |||
| "typecheck": "tsc -p tsconfig.typecheck.json --noEmit", | |||
| "check": "npm run i18n:check && npm run format:check", | |||
There was a problem hiding this comment.
A new test:unit script is added, but it is not run by npm run check (which is what CI currently executes) and it isn’t referenced in the GitHub Actions workflow. If these unit tests are meant to guard against regressions, consider wiring them into check or adding a dedicated CI step so they run on pull requests.
| "check": "npm run i18n:check && npm run format:check", | |
| "check": "npm run i18n:check && npm run format:check && npm run test:unit", |
| ? { | ||
| locale, | ||
| now: DEMO_CHAT_REFERENCE_TIME, | ||
| timeZone: CHAT_RENDER_TIME_ZONE, | ||
| useRelativeDayLabels: true, |
There was a problem hiding this comment.
chatRenderOptions enables useRelativeDayLabels: true for isDemo during SSR as well as after hydration, which makes the server-rendered day headers "Today"/"Yesterday". This contradicts the intended behavior described in the PR (absolute labels on SSR, friendlier relative labels after mount) and will also fail the new e2e/home.spec.ts assertions expecting SSR labels like "Thu, May 1". Consider rendering demo day headers with useRelativeDayLabels disabled on the initial render (server + hydration), then enabling it in a useEffect after mount so the post-hydration UI can switch to relative labels without a hydration mismatch.
| const defaultChatRenderOptions: ChatRenderOptions = { | ||
| locale: "en", | ||
| now: undefined, | ||
| timeZone: CHAT_RENDER_TIME_ZONE, | ||
| useRelativeDayLabels: false, | ||
| }; |
There was a problem hiding this comment.
For non-demo chats, defaultChatRenderOptions.timeZone is set to CHAT_RENDER_TIME_ZONE (currently hard-coded to "UTC"). This changes timestamp rendering from the user's local timezone (previous implicit behavior) to UTC across the app, which is likely a user-visible regression. If the goal is only to avoid SSR/hydration mismatches, consider keeping the initial render deterministic (e.g., UTC) but then switching to the client-resolved timezone after mount, or threading an explicit user/account timezone through instead of forcing UTC globally.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await serverContext.close(); | ||
|
|
||
| await page.waitForLoadState("networkidle"); | ||
| await page.waitForTimeout(2_000); |
There was a problem hiding this comment.
Avoid using a fixed waitForTimeout(2_000) here; it makes the test slower and can still be flaky under load. Prefer waiting on the observable hydration outcome (e.g., expect(page.getByTestId("chat-day-label").first()).toHaveText("Yesterday") / toHaveText("Today"), or an expect.poll on allTextContents()) so the test proceeds as soon as the UI updates.
| await page.waitForTimeout(2_000); | |
| await expect | |
| .poll(async () => page.getByTestId("chat-day-label").allTextContents()) | |
| .toEqual(["Yesterday", "Today"]); |
| getChatDateKey(chatMessage.created_at, { | ||
| timeZone: chatRenderOptions.timeZone, | ||
| }) !== | ||
| getChatDateKey(messages[index - 1].created_at, { | ||
| timeZone: chatRenderOptions.timeZone, |
There was a problem hiding this comment.
showDateHeader recomputes getChatDateKey(...) (which uses Intl.DateTimeFormat(...).formatToParts) for both the current and previous message on every render. For long threads this becomes a hot path. Consider computing the current/previous date keys once per iteration (store in locals), or precomputing an array of date keys with useMemo when messages/timeZone change, then comparing adjacent entries.
| @@ -31,6 +31,7 @@ type ListingReadProps = { | |||
| user: User | null; | |||
| listing: ListingReadListing | null; | |||
| presentation?: Presentation; | |||
| referenceNow?: string; | |||
There was a problem hiding this comment.
referenceNow is optional, but when it’s omitted the chat date helpers fall back to new Date() during render (via formatWeekday), which can reintroduce SSR/hydration drift (especially around day/year boundaries). There are real call sites that render ListingRead without referenceNow (e.g. src/features/map/components/MapListingDrawerPanel.tsx), so consider making this required for non-demo presentations, or plumbing a stable referenceNow from the nearest server component into those entry points.
| isChatDrawerOpen: boolean; | ||
| setIsChatDrawerOpen: (open: boolean) => void; | ||
| existingThread: ChatThreadRecord | null; | ||
| referenceNow?: string; |
There was a problem hiding this comment.
referenceNow is optional here, but ChatWindow uses it to keep “now” stable between SSR and hydration. When it’s undefined (e.g., from callers that haven’t been updated), formatWeekday will fall back to new Date() during render and can still trigger hydration mismatches. Consider requiring referenceNow (or only rendering ChatWindow once a stable reference time is available) to keep the intended determinism.
| referenceNow?: string; | |
| referenceNow: string; |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ...defaultChatRenderOptions, | ||
| locale, | ||
| now: referenceNow, | ||
| timeZone: clientTimeZone ?? CHAT_RENDER_TIME_ZONE, |
There was a problem hiding this comment.
In the non-demo branch, chatRenderOptions doesn’t set useRelativeDayLabels, so it remains false (from defaultChatRenderOptions) even after the client timezone is known. This changes the previous formatWeekday behavior (no Today/Yesterday in real chats). If you want relative labels without SSR/hydration mismatches, consider enabling them only after mount (e.g. when clientTimeZone !== null).
| timeZone: clientTimeZone ?? CHAT_RENDER_TIME_ZONE, | |
| timeZone: clientTimeZone ?? CHAT_RENDER_TIME_ZONE, | |
| useRelativeDayLabels: clientTimeZone !== null, |
| isChatDrawerOpen={isChatDrawerOpen} | ||
| setIsChatDrawerOpen={setIsChatDrawerOpen} | ||
| existingThread={existingThread} | ||
| referenceNow={nonDemoReferenceNow as string} | ||
| /> |
There was a problem hiding this comment.
referenceNow={nonDemoReferenceNow as string} relies on a type cast even though this branch is already the non-demo path. This hides potential future regressions (e.g. if the rendering conditions change) and makes the prop contract harder to follow. Prefer narrowing the prop type explicitly (e.g. by branching on presentation === "demo" / presentation !== "demo") and pass referenceNow directly, or add an invariant check before rendering the drawer.
| export function getChatDateKey( | ||
| dateValue: string | Date, | ||
| options?: Pick<DateFormatOptions, "timeZone"> | ||
| ) { | ||
| const { year, month, day } = getChatDateParts(dateValue, options); | ||
|
|
||
| return `${year}-${month}-${day}`; | ||
| } |
There was a problem hiding this comment.
getChatDateKey() builds the key from year/month/day returned by getChatDateParts(), but those values are optional (?.value). If formatToParts() doesn’t return one of these parts (or the input date is invalid), this can produce a key like undefined-undefined-undefined or throw at runtime. Consider asserting all parts are present and throwing a clear error (or providing a fallback) before concatenating.
| <ListItem key={slug}> | ||
| <LinkedRow | ||
| $featured={featured} | ||
| prefetch={false} | ||
| href={`/newsletter/${slug}`} | ||
| > | ||
| <LinkedRow $featured={featured} href={`/newsletter/${slug}`}> | ||
| <Text $featured={featured}> |
There was a problem hiding this comment.
This removes prefetch={false} from the next/link used for newsletter issues. Given the PR description mentions reducing “route prefetch noise”, re-enabling default prefetching here may reintroduce extra background requests. If the intent is to keep prefetch disabled for this list, consider restoring prefetch={false} (or document why prefetch should now be enabled).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| : { | ||
| ...defaultChatRenderOptions, | ||
| locale, | ||
| now: referenceNow, | ||
| timeZone: clientTimeZone ?? CHAT_RENDER_TIME_ZONE, | ||
| useRelativeDayLabels: clientTimeZone !== null, |
There was a problem hiding this comment.
In the non-demo branch of chatRenderOptions, useRelativeDayLabels is never updated when clientTimeZone becomes available. Because you spread defaultChatRenderOptions, this stays false and prevents formatWeekday(..., { useRelativeDayLabels: true }) from ever showing Today/Yesterday for real chats (and defeats the post-mount “friendlier labels” behavior you have for the demo path). Add useRelativeDayLabels: clientTimeZone !== null to the non-demo options as well (similar to the demo branch).
| { timeZone = CHAT_RENDER_TIME_ZONE }: Pick<DateFormatOptions, "timeZone"> = {} | ||
| ): ChatDateParts { | ||
| const parts = getChatDateKeyFormatter(timeZone).formatToParts( | ||
| toDate(dateValue) | ||
| ); |
There was a problem hiding this comment.
getChatDateParts returns year/month/day values that can be undefined (via optional chaining on .find(...)). Downstream, getChatDateKey will happily stringify these into keys like "undefined-undefined-undefined", and subtractDaysFromDateKey will parse them into NaN. Please add a small invariant check (and/or an explicit return type) so missing parts fail fast with a clear error instead of generating invalid date keys.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
This fixes the homepage hydration mismatch caused by the demo chat and hardens shared chat date rendering so server and client agree on first render.
What changed
YesterdayandTodayreferenceNowthrough the real chat surfaces so non-demo chat can’t render without a deterministic server snapshotNotes
The homepage chat is still a canned demo rather than live data. Its timestamps are intentionally fixed under the hood so the server render remains stable across timezones, browser locales, and calendar changes.
This branch no longer changes Next.js prefetch behaviour; the earlier prefetch experiment was reverted.
Verification
npm run checknpm run test:e2e:prod