Refactor URL parameter handling to improve state management #707
Conversation
- Replaced `useSearchParams` with `useQueryState` from the `nuqs` library across multiple components for better state synchronization with URL parameters. - Updated components to utilize new query state management, enhancing clarity and reducing reliance on direct URL manipulation. - Improved overall code maintainability and readability by streamlining parameter handling logic.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR migrates URL query parameter handling from Next.js's native Changes
Sequence Diagram(s)sequenceDiagram
participant Old as Old Pattern (useSearchParams + Suspense)
participant New as New Pattern (nuqs hooks)
rect rgb(200, 220, 255)
note over Old: Initial page load
Old->>Old: Render parent with Suspense boundary
Old->>Old: Suspense renders fallback
Old->>Old: Handler component mounts (async)
Old->>Old: useSearchParams() reads URL
Old->>Old: Manual parse + decode
Old->>Old: setState update
Old->>Old: Re-render with state
end
rect rgb(200, 255, 220)
note over New: Initial page load
New->>New: useQueryState/useQueryStates hook
New->>New: nuqs parser auto-converts
New->>New: State synced via useEffect
New->>New: Single render cycle
end
rect rgb(255, 220, 220)
note over Old,New: URL query parameter changes
Old->>Old: Re-run handler useEffect
Old->>Old: Parse & update state
New->>New: nuqs detects URL change
New->>New: Parser re-runs automatically
New->>New: State updates reactively
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
client/src/app/as/callback/page.tsx (1)
47-55: Consider bidirectional URL sync for improved UX.The current implementation performs one-way synchronization from URL parameters to local state. When users progress through steps (lines 114, 185), the URL remains unchanged. This means users cannot bookmark their progress, and refreshing the page will reset them to the URL's step.
For better user experience, consider using the setter from
useQueryStatesto update the URL whencurrentStepchanges:// Get code and step from URL params - const [{ code, step: stepParam }] = useQueryStates({ + const [{ code, step: stepParam }, setQueryStates] = useQueryStates({ code: parseAsString, step: parseAsInteger, }); // Sync URL params with local state useEffect(() => { if (code) { setAppsumoCode(code); } if (stepParam && stepParam >= 1 && stepParam <= 3) { setCurrentStep(stepParam); } }, [code, stepParam]); + + // Sync currentStep back to URL + useEffect(() => { + if (currentStep >= 1 && currentStep <= 3) { + setQueryStates({ step: currentStep }); + } + }, [currentStep, setQueryStates]);This would allow users to refresh at any point without losing progress and enable bookmarking/sharing the current step.
client/src/app/[site]/gsc/select-property/page.tsx (1)
21-25:propertiesparsing is safe but could validate shape more defensivelyThe
parseAsJsonvalidator currently just casts the decoded value tostring[], so malformed JSON (e.g.,{}or["foo", 1]) would flow through unchecked. Since this comes from the URL, consider tightening the validator to enforce an array-of-strings shape (e.g.,Array.isArray(value)plus atypeof v === "string"filter) before accepting it.client/src/components/SiteSettings/GSCManager.tsx (1)
17-29: Consider clearinggscafter handling success to avoid repeated toastsReading
gscStatusviauseQueryStateand triggering the success toast/refetch is correct. If you want to avoid re-showing the “connected successfully” toast on every reload/back nav while?gsc=successis in the URL, you could also grab the setter fromuseQueryStateand set the param back tonullonce handled.client/src/app/signup/page.tsx (1)
38-45: Step URL param is now one‑way (URL → state); consider 2‑way sync if neededReading
stepviauseQueryStateand clamping to 1–3 is solid. Note thatcurrentStepchanges (e.g.,setCurrentStep(2)after signup) no longer update thestepquery param, so refreshing or sharing the URL won’t reflect the current step unless it came from the URL initially. If preserving 2‑way sync is important, you could also grab the setter fromuseQueryStateand updatestepwhenevercurrentStepchanges.client/src/app/invitation/page.tsx (1)
18-21: Query parsing viauseQueryStateslooks good; consider DRYing/shared defaultsBoth
AuthComponentandAcceptInvitationInnernow independently calluseQueryStatesto readorganization,inviterEmail, andinvitationId, which is correct but slightly duplicated. You could optionally:
- Extract a small
useInvitationQuery()hook that returns these values once, or- Add parser defaults (e.g., via
.withDefault("")) to avoid any accidental"undefined"text if params are missing.Not required, but would tighten things up a bit.
Also applies to: 52-56
client/src/lib/urlParams.ts (1)
127-181:needsUpdateflag is never usedWithin the initialization effect,
needsUpdateis toggled but never read, so it no longer affects behavior. It can be safely removed (along with its assignments) to keep the effect focused on the actual URL → store synchronization logic.client/src/lib/parsers.ts (3)
39-50: TightentimeModeparser typing to use theTimeunionRight now
timeModeValuesis typed asstring[], soparseAsTimeModewill be typed as a generic string parser instead of theTimeunion you import from@/components/DateSelector/types. To keep things type‑safe and catch drift:const timeModeValues: Time[] = [ "day", "range", "week", "month", "year", "all-time", "past-minutes", ]; export const parseAsTimeMode = parseAsStringEnum<Time>(timeModeValues);This ensures the parser stays in sync with the
Timedomain type if it changes.
74-80: AlignparseAsIsoDatebehavior with its comment and consider light validation for JSON parsers
parseAsIsoDateis currently just an alias forparseAsString, so it doesn’t actually validate ISO date format despite the comment. Either:
- Update the comment to describe it as a plain string parser, or
- Use a small custom parser (e.g., via
createParserwith a simple format check) if you want real ISO validation.For
parseAsFilters/parseAsStringArray, the custom parse functions only cast the parsed JSON, so they add type information but no runtime shape validation. That’s fine if these params are only written by your own UI, but if they can be user‑controlled you might want minimal checks (e.g.,Array.isArrayand basic element checks) before casting.
12-24: Reduce drift risk for enum‑like value listsThe hard‑coded lists in
timeBucketValues,statTypeValues, andwellKnownValuesmust stay in sync with their corresponding domain types (e.g.,StatTypeinclient/src/lib/store.tsline 5). It’s easy for these to diverge over time when new values are added.Not strictly required, but consider either:
- Centralizing these arrays alongside the type definitions and importing them here, or
- Adding small tests that assert the arrays cover all expected values.
That will help catch mismatches early as the set of allowed values evolves.
Also applies to: 27-37, 52-72
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
client/src/app/[site]/components/Sidebar/SiteSelector.tsx(3 hunks)client/src/app/[site]/gsc/select-property/page.tsx(2 hunks)client/src/app/[site]/utils.ts(1 hunks)client/src/app/as/callback/page.tsx(2 hunks)client/src/app/auth/subscription/success/page.tsx(0 hunks)client/src/app/invitation/page.tsx(3 hunks)client/src/app/signup/page.tsx(2 hunks)client/src/components/SiteSettings/GSCManager.tsx(1 hunks)client/src/lib/parsers.ts(1 hunks)client/src/lib/urlParams.ts(2 hunks)
💤 Files with no reviewable changes (1)
- client/src/app/auth/subscription/success/page.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
client/src/lib/urlParams.ts (2)
client/src/lib/parsers.ts (1)
analyticsParsers(98-118)client/src/components/DateSelector/types.ts (1)
Time(44-44)
client/src/app/[site]/utils.ts (1)
client/src/api/admin/sites.ts (1)
useCurrentSite(198-207)
client/src/components/SiteSettings/GSCManager.tsx (1)
client/src/api/gsc/useGSCConnection.ts (3)
useGSCConnection(13-23)useConnectGSC(28-39)useDisconnectGSC(44-62)
client/src/lib/parsers.ts (1)
client/src/lib/store.ts (1)
StatType(6-6)
⏰ 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 Client Images (ubuntu-latest, linux/amd64)
- GitHub Check: Build Client Images (ubuntu-24.04-arm, linux/arm64)
🔇 Additional comments (8)
client/src/app/[site]/components/Sidebar/SiteSelector.tsx (3)
2-2: Import changes look correct.The removal of
useSearchParamsfrom the import statement aligns with the refactoring approach in this file.
59-60: Verify consistency with PR migration strategy.The code correctly preserves query parameters during navigation and is SSR-safe due to the
typeof window !== "undefined"guard on line 31. However, this approach usingwindow.location.searchdiffers from the PR's stated objective of migrating to thenuqslibrary'suseQueryState.Please confirm whether this component intentionally uses a different approach, or if it should also be migrated to
nuqsfor consistency.
103-104: Add window check to prevent potential SSR errors.Accessing
window.location.searchwithout checking ifwindowexists could cause runtime errors during server-side rendering or pre-rendering in Next.js.Apply this diff to add a safe guard:
- const queryString = window.location.search; + const queryString = typeof window !== "undefined" ? window.location.search : ""; router.push(queryString ? `${newPath}${queryString}` : newPath);Alternatively, consider using Next.js's
useSearchParamshook for consistency with Next.js best practices:const searchParams = useSearchParams(); const queryString = searchParams.toString(); router.push(queryString ? `${newPath}?${queryString}` : newPath);Run the following script to check if similar patterns exist elsewhere:
client/src/app/as/callback/page.tsx (2)
41-45: LGTM!The
useQueryStatesusage is syntactically correct for reading URL query parameters.
18-18: Verify nuqs library compatibility with Next.js 15 and React 19.Ensure that the version of
nuqsbeing used is compatible with Next.js 15 and React 19, as this PR targets a codebase using these versions.Run the following script to check the installed version and available updates:
Additionally, search the web for compatibility information:
client/src/app/[site]/utils.ts (1)
3-15:embedhandling viaparseAsBooleankeeps previous behavior intactSwitching from a string comparison to
parseAsBooleanwithif (embed && subscription?.planName !== "free")preserves the intent of only treatingembed=trueas embeddable (and still blocking free plans). This looks consistent with the existing semantics.client/src/lib/urlParams.ts (1)
191-233: Time → URL mapping andnewParamsconstruction look consistentThe
newParamsconstruction correctly:
- Always writes
timeMode,bucket,stat, and only includesfilterswhen non‑empty.- Uses
time.wellKnownto prefer compact presets and explicitly clears all date/past‑minutes fields in that case.- Falls back to explicit date/past‑minutes fields keyed by
time.modewhen there’s nowellKnown.This matches the structure defined in
analyticsParsersand should keep URL/state round‑tripping predictable.client/src/lib/parsers.ts (1)
6-9: Centralized query param parsers look solidMapping individual parsers into
invitationParsers,appSumoCallbackParsers, andanalyticsParsersgives a clear, single place for URL parameter handling and should make future changes easier and less error‑prone. No issues from my side here.Also applies to: 85-96, 98-118
…o#707) - Replaced `useSearchParams` with `useQueryState` from the `nuqs` library across multiple components for better state synchronization with URL parameters. - Updated components to utilize new query state management, enhancing clarity and reducing reliance on direct URL manipulation. - Improved overall code maintainability and readability by streamlining parameter handling logic.
…ybbit-io#707)" This reverts commit c96d21b.
useSearchParamswithuseQueryStatefrom thenuqslibrary across multiple components for better state synchronization with URL parameters.Summary by CodeRabbit