fix(comments): return readers to the post after comment sign-in#154
Conversation
The comment composer's `<SignInButton mode="modal">` passed no redirect props, so after the GitHub OAuth full-page round-trip Clerk fell back to its default destination of `/` — sending readers to the home page instead of back to the post they were commenting on. Pin the sign-in and sign-up redirects to the current path via `usePathname()`, so a reader returns to the page they signed in from. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BDuq4Dh5vNvggsSmLfXHcr
|
@claude review Generated by Claude Code |
🚀 Preview deploymenthttps://btnopen-pr-154.vercel.app Deployed |
| <SignInButton | ||
| mode="modal" | ||
| forceRedirectUrl={pathname} | ||
| signUpForceRedirectUrl={pathname} | ||
| > |
There was a problem hiding this comment.
Important — per REVIEW.md's mandatory acceptance-criteria check against #153: criterion "the return-to-post behavior also holds for a first-time GitHub sign-up" isn't confirmable from this diff. signUpForceRedirectUrl is wired correctly per Clerk's documented API, but whether Clerk actually classifies the GitHub OAuth callback as a sign-up completion (vs. a plain sign-in, which would use forceRedirectUrl instead) is live provider behavior, not something static review of the diff can prove.
The PR description already tracks this as an open check ("needs manual/preview check"), so this is a required restatement of that gap per the repo's review policy rather than a newly discovered issue — please confirm it on the Clerk-configured preview before merging.
Code review1 important, 0 nits. Important
Mandatory checks (per REVIEW.md)
|
🚀 Preview deploymentTorn down — the isolated Turso database ( |
Summary
The comment composer's only sign-in surface — the "Sign in with GitHub" button shown under a post to signed-out readers — used Clerk's
SignInButton(mode="modal") with no redirect props. In modal mode Clerk never records aredirect_urlin the URL, and GitHub OAuth is a full-page round-trip, so on return Clerk fell back to its default post-auth destination of/— sending readers to the home page regardless of which post they were reading.This pins the sign-in and sign-up redirects to the current path, captured with
usePathname()(an existing pattern inapp/(app)/_components/page-view-tracking.tsx), so a reader returns to the page they signed in from.forceRedirectUrlcovers the sign-in path andsignUpForceRedirectUrlcovers first-time GitHub account creation via the same button. Because the redirect is derived from wherever the button renders, it also works for a sign-in begun from any other page — it isn't hard-coded to posts.The change is one component, +12/−1. It produces no DOM/appearance change: Clerk destructures these props to build its OAuth flow options and never spreads them onto the rendered button.
Current Clerk docs consulted (July 2026): the
SignInButtonreference and "Customize your redirect URLs".Related issues
Closes #153
Verification
Commands run in this checkout:
npm run format— clean, no fixes applied.npm run lint—biome checkclean (only a pre-existing, unrelated warning inscripts/prune-preview-blobs.mjs; none in the changed file).npm run build— succeeded end-to-end (compile, type-check, prerender of all routes) after applying local Payload migrations.npm run test:unit— 186/186 across 23 suites.npm run test:e2e— could not run in this checkout: the environment has Playwright browser build 1194 but@playwright/test ^1.61.1pins build 1228, and this environment is configured not to runplaywright install. The changed code path is Clerk-gated (Clerk is disabled here) and produces no DOM change, so the suite could not exercise it regardless. The per-PR preview-deploy pipeline runs the e2e suite against a Clerk-configured preview as the gate.Acceptance criteria
/. Verified against the installed@clerk/nextjstypes:forceRedirectUrltakes precedence over Clerk's default/fallback, andusePathname()resolves to the post's own path (no locale-prefix routing in play).signUpForceRedirectUrlis wired per Clerk's documented API — but whether Clerk classifies the OAuth callback as a sign-up completion is runtime behavior only a live check can confirm. On the Clerk-configured preview, click sign-in with a GitHub account that has never signed into the app, complete account creation, and confirm the browser lands back on the post URL, not/. (The independent review restated this as its one Important finding, per the repo review policy that flags any criterion unconfirmable from the diff.)comments.tsx) and its Clerk-unavailable short-circuit are untouched.usePathname()is called unconditionally at the top of the component (no Rules-of-Hooks violation), and the change is confined tocomment-composer.tsx. Corroborated by theuseHookAtTopLevelBiome rule in the clean lint pass.npm run format,npm run lint, andnpm run buildpass (see above).Notes for reviewers
Start with the single changed file,
app/(app)/posts/[slug]/_components/comments/comment-composer.tsx. The behavioral criteria (the real OAuth redirect) aren't observable in this checkout — Clerk is disabled locally and the redirect URL is consumed by Clerk's internal handler rather than rendered — so the honest verification path for those is the Clerk-configured preview deploy. No new automated test is added: the repo's Jest suite is pure-logic only (no component/jsdom/Clerk-mock harness), and the redirect isn't DOM-observable for Playwright; the sign-in scenarios ine2e/scenarios.mdaremay-priority and Clerk-token-gated.