Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 7
Server-render entry word count, read time and relative dates#1663
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
Merged
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
25 changes: 7 additions & 18 deletions
25 ...app/(dynamicPages)/entry/[category]/[author]/[permlink]/_components/entry-page-listen.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| "use client"; | ||
| import { useEffect, useState, useSyncExternalStore } from "react"; | ||
| import { useEffect, useMemo, useState, useSyncExternalStore } from "react"; | ||
| import { | ||
| dateToFormatted, | ||
| dateToFormattedUtc, | ||
| @@ -60,10 +60,15 @@ interface Props { | ||
| * - "fullRelative": long form, e.g. "5 hours ago" | ||
| * - "absolute": formatted date using `format` | ||
| * | ||
| * The first paint (SSR + initial client render) is always a UTC numeric | ||
| * string so the two sides agree and React doesn't fire hydration error | ||
| * #418. After mount the visible text and tooltip swap to the user's local | ||
| * timezone and locale. | ||
| * Relative modes render their value from the first SSR paint: a relative | ||
| * form is the difference of two instants, so it is timezone-independent | ||
| * and safe to compute server-side (#1662). The mount effect re-computes it | ||
| * so an edge-cached page self-corrects; `suppressHydrationWarning` on the | ||
| * span absorbs the boundary case where the two sides disagree by one unit. | ||
| * | ||
| * For "absolute" the first paint stays a UTC numeric string (a local | ||
| * format depends on the viewer's timezone and locale, which the server | ||
| * cannot know) and swaps after mount. | ||
| */ | ||
| mode?: Mode; | ||
| /** dayjs format token used when `mode` is "absolute". Defaults to "LLLL". */ | ||
| @@ -78,15 +83,30 @@ export function TimeLabel({ | ||
| format = "LLLL", | ||
| className = "date", | ||
| }: Props) { | ||
| const [display, setDisplay] = useState<string | null>(null); | ||
| const [display, setDisplay] = useState<string | null>(() => { | ||
| // SERVER-ONLY initializer, deliberately. The client must start at null: | ||
| // with suppressHydrationWarning React keeps the server text in the DOM on | ||
| // a mismatch while its vdom holds the client-rendered value, so if the | ||
| // client initializer computed its own (possibly newer) relative value, | ||
| // the mount effect's setDisplay() would bail out on state equality and | ||
| // the stale server text would stay visible until the NEXT unit change | ||
| // (a day, even a month). Starting at null makes the mount effect a real | ||
| // state transition whose vdom diff (UTC fallback -> relative) always | ||
| // writes the text node. | ||
| if (typeof window !== "undefined") return null; | ||
| if (mode === "fullRelative") return dateToFullRelative(created); | ||
| if (mode === "relative") return dateToRelative(created); | ||
| return null; | ||
| }); | ||
qodo-code-review[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| const [localFormatted, setLocalFormatted] = useState<string | null>(null); | ||
| // Self-tick: re-runs the formatting effect ~once a minute for relative modes, | ||
| // so timestamps stay fresh without the parent re-rendering the whole card. | ||
| const tick = useTick(mode === "relative" || mode === "fullRelative"); | ||
| // Numeric UTC — identical on server and client, no hydration mismatch. | ||
| const ssrSafe = dateToFormattedUtc(created); | ||
| // Numeric UTC fallback; memoized so feeds full of labels parse each date | ||
| // once per value instead of on every render. | ||
| const ssrSafe = useMemo(() => dateToFormattedUtc(created), [created]); | ||
| useEffect(() => { | ||
| setLocalFormatted(dateToFormatted(created)); | ||
59 changes: 59 additions & 0 deletions
59 apps/web/src/specs/features/entry/entry-page-listen-ssr.spec.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| import { vi, describe, it, expect } from "vitest"; | ||
| import { renderToString } from "react-dom/server"; | ||
| import type { ReactNode } from "react"; | ||
| /* | ||
| SSR pin for #1662: the word count and read time are pure derivations of | ||
| entry.body and must be present in the server-rendered HTML, not "0" | ||
| placeholders that flip after hydration. renderToString runs no effects, | ||
| exactly like the server, so a regression back to useMount renders 0 here. | ||
| The collaborators are mocked at their module seams; the component itself, | ||
| countWords and the real getPurePostTextForWordCount run for real. | ||
| */ | ||
| vi.mock("@/utils", async () => ({ | ||
| ...(await vi.importActual("@/utils")), | ||
| getAccessToken: vi.fn(() => undefined), | ||
| ensureValidToken: vi.fn() | ||
| })); | ||
| vi.mock("@/features/shared", () => ({ error: vi.fn(), success: vi.fn() })); | ||
| vi.mock("@/features/text-to-speech", () => ({ | ||
| useTts: vi.fn(() => ({ speechRef: { current: undefined }, hasPaused: false, hasStarted: false })), | ||
| TextToSpeechSettingsDialog: ({ children }: { children: ReactNode }) => <>{children}</> | ||
| })); | ||
qodo-code-review[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| vi.mock("@/api/translation", () => ({ | ||
| getTranslation: vi.fn(), | ||
| getLanguages: vi.fn(async () => []) | ||
| })); | ||
| vi.mock("@/config", () => ({ | ||
| EcencyConfigManager: { useConfig: vi.fn(() => false) } | ||
| })); | ||
| vi.mock("@ui/modal", () => ({ | ||
| Modal: () => null, | ||
| ModalBody: () => null, | ||
| ModalHeader: () => null, | ||
| ModalTitle: () => null | ||
| })); | ||
| import { EntryPageListen } from "@/app/(dynamicPages)/entry/[category]/[author]/[permlink]/_components/entry-page-listen"; | ||
| import { countPostWords } from "@/utils"; | ||
| import { mockEntry } from "@/specs/test-utils"; | ||
| describe("EntryPageListen SSR output (#1662)", () => { | ||
| const body = Array.from({ length: 574 }, (_, i) => `word${i}`).join(" "); | ||
| const entry = mockEntry({ body }); | ||
| it("computes the body-derived stats it renders", () => { | ||
| expect(countPostWords(body)).toBe(574); | ||
| }); | ||
| it("server-renders the real word count and read time, not 0 placeholders", () => { | ||
| // React separates adjacent text expressions with comment nodes; strip them | ||
| // so the assertion reads like the visible text. | ||
| const html = renderToString(<EntryPageListen entry={entry} />).replace(/<!-- -->/g, ""); | ||
| expect(html).toContain(">574<"); | ||
| // 574 words at 225 wpm rounds up to 3 minutes. | ||
| expect(html).toMatch(/>3 entry\.post-read-minutes/); | ||
| expect(html).not.toContain(">0<"); | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
When cached server HTML is stale, or the timestamp crosses a formatting boundary before hydration, this initializer gives the client the new relative value while the DOM still contains the server value. Because the span uses
suppressHydrationWarning, React does not patch that mismatched text during hydration, and the mount effect then callssetDisplaywith the value already held in state, so it can bail out without correcting the DOM. The label can consequently retain the stale server text until the relative string changes again, potentially for days or months; ensure the post-mount correction causes an actual state transition.Useful? React with 👍 / 👎.
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.
Confirmed and fixed in ea33c80, with the mechanism exactly as described: the client initializer already held the corrected value, so the mount effect's
setDisplaybailed out on state equality whilesuppressHydrationWarninghad left the server text in the DOM. The initializer is now server-only (typeof windowguard), so the client starts atnulland the mount effect is always a real state transition whose vdom diff writes the text node. Added the hydration spec you asked for: server HTML rendered at T, hydrated at T+25h with fake timers, asserting the DOM ends on the client value. Verified the spec fails against the previous initializer and passes with the guard.