Skip to content

fix(status): scope tab/range/refresh search params to the status route#1469

Merged
kylebernhardy merged 1 commit into
stagefrom
kyle/status-route-search-params
Jul 11, 2026
Merged

fix(status): scope tab/range/refresh search params to the status route#1469
kylebernhardy merged 1 commit into
stagefrom
kyle/status-route-search-params

Conversation

@kylebernhardy

Copy link
Copy Markdown
Member

Fixes #1440 — Status tab unmount cleanup can clobber the destination route's search params.

Summary

The Status tab's tab/range/refresh search params were hand-validated inside StatusTabs and stripped by a navigate-on-unmount effect. That cleanup fires after the destination route commits, so it could wipe the destination's own search params, added a spurious history replace on every departure, and double-fired under StrictMode.

The params are now the router's job: a new light statusSearch.ts module (deliberately outside the lazy-loaded analytics chunk; its only import is the constants-only timePresets.ts) defines the schema, defaults, and validator. The status route wires validateSearch + a stripSearchParams(defaults) middleware, so invalid deep links normalize to defaults, default values stay out of the URL, and sibling routes never see the params — verified against router-core 1.171 source that a Link/navigate without search resolves to {}, so nothing inherits them. StatusTabs reads the already-validated values through the same validator (narrowing the strict: false typing), and the unmount effect plus the duplicated VALID_PRESETS/VALID_REFRESH constants are gone.

Where to look

  • src/features/instance/status/routes.ts — the route wiring (the behavioral core, 4 lines).
  • src/features/instance/status/statusSearch.ts — schema/validator; note the empty-string refresh guard (Number('') is 0, which would silently match the "Off" option).
  • src/router/__tests__/statusSearchScope.test.ts — router-level regression tests (matchRoutes/buildLocation, no loaders, same pattern as orgUndefinedRepro): deep-link validation, invalid-value normalization, default-stripping, and sibling-navigation scoping.
  • The url-sync test now asserts no navigation fires on unmount.

Cross-model review: Codex found no issues; Gemini passed all criteria (its one suggestion, the empty-string refresh guard, is included).

Generated by kAIle (Claude Fable 5).

The Status tab's search params were hand-validated inside StatusTabs and
cleaned up by a navigate-on-unmount effect. That cleanup fires after the
destination route has committed, so it could clobber the destination's
own search params, issued a spurious history replace on every departure,
and double-fired under StrictMode.

The params are now declared on the route: a light statusSearch.ts module
(outside the lazy analytics chunk) defines the schema, defaults, and
validator; the status route wires validateSearch plus a
stripSearchParams(defaults) middleware so default values stay out of the
URL; and StatusTabs reads the already-validated values through the same
validator instead of hand-rolled casts. The unmount effect and the
duplicated VALID_PRESETS/VALID_REFRESH constants are deleted.

Fixes #1440

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors search parameter handling for the instance status route by moving validation and defaulting from the StatusTabs component to the route definition level using TanStack Router's validateSearch and stripSearchParams. This declarative approach prevents status-related parameters from bleeding into sibling routes, resolving issue #1440 and eliminating the need for imperative cleanup on unmount. New tests have been added to verify search parameter validation and scoping. I have no feedback to provide as the changes are well-implemented and covered by tests.

@github-actions

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 47.45% 4481 / 9442
🔵 Statements 47.79% 4779 / 9998
🔵 Functions 39.45% 1068 / 2707
🔵 Branches 39.62% 2895 / 7306
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/features/instance/status/routes.ts 60% 100% 50% 60% 15-16
src/features/instance/status/statusSearch.ts 100% 100% 100% 100%
src/features/instance/status/analytics/StatusTabs.tsx 90.74% 80% 83.33% 90% 46-49, 54-61, 128, 136, 188
Generated in workflow #1436 for commit c41ea05 by the Vitest Coverage Report Action

@kylebernhardy kylebernhardy marked this pull request as ready for review July 10, 2026 21:21
@kylebernhardy kylebernhardy requested a review from a team as a code owner July 10, 2026 21:21
@kylebernhardy kylebernhardy added this pull request to the merge queue Jul 11, 2026
Merged via the queue into stage with commit d7646f0 Jul 11, 2026
3 checks passed
@kylebernhardy kylebernhardy deleted the kyle/status-route-search-params branch July 11, 2026 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Status tab unmount cleanup can clobber the destination route's search params

2 participants