Skip to content

refactor: migrate query params from use-query-params to nuqs v2#2187

Open
brandon-pereira wants to merge 6 commits intomainfrom
brandon/query-param-refactor
Open

refactor: migrate query params from use-query-params to nuqs v2#2187
brandon-pereira wants to merge 6 commits intomainfrom
brandon/query-param-refactor

Conversation

@brandon-pereira
Copy link
Copy Markdown
Member

@brandon-pereira brandon-pereira commented May 4, 2026 •
edited
Loading

Summary

Replaces the legacy use-query-params (+ next-query-params, serialize-query-params) stack with nuqs v2 across the app, drops the local useQueryParam.tsx shim, and cleans up two workarounds that the migration made obsolete.

What changed

Dependency migration

  • nuqs bumped 1.17.0 → ^2.8.9.
  • Removed use-query-params, next-query-params, serialize-query-params, and the now-unused @jedmao/location devDep.
  • Migrated timeQuery.ts, SessionsPage, *DetailsSidePanel, BenchmarkPage, DBChartPage, DBDashboardImportPage, _app.tsx, and the Storybook preview.tsx to nuqs hooks/parsers.
  • Deleted packages/app/src/useQueryParam.tsx (the wrapper around the old library) and packages/app/src/fixtures.ts (test-only TestRouter built on LocationMock).

Test plan

  • make ci-lint ✅
  • make ci-unit ✅ — 86 suites / 1525 tests pass (was 85 / 1518 + 9 skipped on main).
  • Manual: source dropdown on /search no longer flickers when switching sources.

@brandon-pereira
refactor: migrate query-param hooks from use-query-params to nuqs v2
eb65dd3 
Replaces `use-query-params` (and `next-query-params`,
`serialize-query-params`) with nuqs v2 across the app, dropping
`useQueryParam.tsx` in favour of nuqs's hooks. Also:

- Removes the redundant manual `reset()` sync effect on DBSearchPage —
  RHF's built-in `values`-prop reconciliation already deep-compares and
  honors `resetOptions: { keepDirtyValues: true }`, so the user's
  in-flight source pick is no longer clobbered by the stale URL value
  (the cause of the source dropdown flicker).
- Drops the `tempLiveTailTimeRange` workaround in `useTimeQuery`. The
  hack existed because nuqs v1 / use-query-params updated state
  asynchronously; nuqs v2 updates local state synchronously, so the
  bridge state is dead weight.
- Un-skips `timeQuery.test.tsx` and migrates it to nuqs's official
  `NuqsTestingAdapter`. Drops the `tq`-based and browser-nav cases
  (only applied to the legacy `useTimeQuery`) and adds direct
  `onSearch` / `onTimeRangeSelect` setter coverage.
- Removes the now-unused `@jedmao/location` devDep and `fixtures.ts`.
@vercel
Copy link
Copy Markdown

vercel Bot commented May 4, 2026 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview, Comment May 4, 2026 8:51pm

Request Review

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 4, 2026 •
edited
Loading

🦋 Changeset detected

Latest commit: 8e7fbbc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/app Patch
@hyperdx/api Patch
@hyperdx/otel-collector Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

brandon-pereira
brandon-pereira commented May 4, 2026
View reviewed changes
Comment thread packages/app/src/DBSearchPage.tsx
@@ -932,24 +950,6 @@ export function DBSearchPage() {
updateInput: !isLive,
});

Copy link
Copy Markdown
Member Author

@brandon-pereira brandon-pereira May 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the below code is removed because it was causing extra re-renders and wasn't needed. browser back/forward navigation still works

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026 •
edited
Loading

E2E Test Results

❌ 17 tests failed • 144 passed • 3 skipped • 1363s

Status Count
✅ Passed 144
❌ Failed 17
⚠️ Flaky 2
⏭️ Skipped 3

Tests ran across 4 shards in parallel.

View full report →

@brandon-pereira brandon-pereira marked this pull request as ready for review May 4, 2026 18:15
@github-actions github-actions Bot added the review/tier-3 Standard — full human review required label May 4, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026 •
edited
Loading

🟡 Tier 3 — Standard

Introduces new logic, modifies core functionality, or touches areas with non-trivial risk.

Why this tier:

  • Standard feature/fix — introduces new logic or modifies core functionality

Review process: Full human review — logic, architecture, edge cases.
SLA: First-pass feedback within 1 business day.

Stats
  • Production files changed: 17
  • Production lines changed: 343 (+ 449 in test files, excluded from tier calculation)
  • Branch: brandon/query-param-refactor
  • Author: brandon-pereira

To override this classification, remove the review/tier-3 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@brandon-pereira brandon-pereira requested review from a team, karl-power and pulpdrew and removed request for a team and pulpdrew May 4, 2026 18:16
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026 •
edited
Loading

PR Review

Clean migration from use-query-params → nuqs v2. The mechanics are correct: undefined → null, updateType: 'replaceIn' → history: 'replace', pushIn → history: 'push', enableBatching removed (nuqs v2 batches by default), and clearOnDefault: false applied globally to preserve v1 semantics.

Issues:

  • ⚠️ Deleted behavioral tests without replacement → The rewritten timeQuery.test.tsx drops 4 previously-skipped tests that covered critical logic: tq param overriding from/to, browser back/forward syncing the time range, and async isReady sequencing. These aren't skipped — they're gone. If any of this logic broke during migration, there's no coverage catching it. Add equivalent tests using withNuqsTestingAdapter + onUrlUpdate callbacks to verify tq-priority behavior and history navigation.

  • ⚠️ Removed form-sync useEffect in DBSearchPage.tsx (prevSearched / reset) without explicit test coverage for browser back navigation on the search page. The values prop on useForm should handle this, but it's a behavior change in a sensitive path. Confirm with a manual back-navigation test or integration test.

  • ⚠️ TimePicker.tsx new useEffect syncs defaultRelativeTimeMode → isRelative state. This causes an extra render on every parent re-render that changes the prop. Confirm the parent doesn't pass a new reference each render (e.g. useMemo/stable value), otherwise this could introduce the flicker it's meant to fix.

Minor:

  • clearOnDefault: false on NuqsAdapter is the right call for v1 compatibility, but it's a footgun for future work — worth a brief code comment (already has one in _app.tsx, Storybook adapter lacks it).
  • selectedSessionQueryMap uses parseAsFloat for sfrom/sto without .withDefault(0) or similar — callers must handle null, which they do (selectedSession useMemo checks values). Fine, just confirm downstream null handling is complete.

✅ Zod schema validation added to parseAsJson() calls (DashboardSchema, SavedChartConfigSchema, stringArraySchema) — good improvement over the old unchecked generic casts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@karl-power karl-power Awaiting requested review from karl-power karl-power was automatically assigned from hyperdxio/hyperdx-team

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

review/tier-3 Standard — full human review required

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@brandon-pereira