Review URL-synchronized issue search

Review exercise · TypeScript

Review brief

Intermediate35 min

An issue tracker makes search state shareable and navigable by synchronizing its controls with the URL, then loading matching results asynchronously. Review how URL state, history navigation, and request completion interact.

Behavioral contract

The URL query parameters q, status, and page are the authoritative search state on initial load and every Back or Forward navigation.
Typing in q replaces the current history entry; deliberate status and page changes push one navigable entry.
Changing q or status resets page to 1, while a page-only change retains q and status.
Search URL updates must preserve unrelated parameters, such as project and view, and the current hash fragment.
Only results requested for the current complete URL state may render, even when earlier requests finish later.
Missing or invalid page values resolve to page 1.
API requests in scope resolve successfully; request ordering, rather than an error-state design, is under review.

Review instructions

1Trace behavior across all three changed files and the stated browser-history contract.
2Anchor each finding to the narrowest useful line and explain the navigation or result impact.
3Prioritize no more than five findings and recommend a direction rather than replacement code.

Learning objectives

• Model the URL as navigable application state
• Prevent stale asynchronous work from winning
• Preserve independent query parameters

src/issue-search-state.tsTypeScript

@@ -0,0 +1,33 @@

1+export type IssueStatus = "open" | "closed" | "all";

2+

3+export interface IssueSearchState {

4+ query: string;

5+ status: IssueStatus;

6+ page: number;

7+}

8+

9+function readPage(value: string | null): number {

10+ const page = Number(value);

11+ return Number.isInteger(page) && page > 0 ? page : 1;

12+}

13+

14+function readStatus(value: string | null): IssueStatus {

15+ return value === "closed" || value === "all" ? value : "open";

16+}

17+

18+export function readIssueSearchState(url: URL): IssueSearchState {

19+ return {

20+ query: url.searchParams.get("q") ?? "",

21+ status: readStatus(url.searchParams.get("status")),

22+ page: readPage(url.searchParams.get("page")),

23+ };

24+}

25+

26+export function buildIssueSearchUrl(baseUrl: URL, state: IssueSearchState): string {

27+ const search = new URLSearchParams();

28+ if (state.query) search.set("q", state.query);

29+ search.set("status", state.status);

30+ search.set("page", String(state.page));

31+

32+ return `${baseUrl.pathname}?${search.toString()}`;

33+}

Reference solution

Findings are ordered by priority.

Findings

1. An older request can replace results for the current URL

Location: src/issue-search-controller.ts:33-35

Every load renders when its promise resolves, with no request identity or cancellation check. If a slow request for an earlier query finishes after a newer request, its issues overwrite the current results even though the controls and URL describe different state.

Abort superseded requests or associate each load with a generation or serialized URL state and render only when it still matches the current state.

2. Back and Forward restore only the query text

Location: src/issue-search-controller.ts:57-61

The popstate handler parses all three URL fields but copies only query into state and updates only that control. It then loads with the old status and page, so navigation can show controls and results that disagree with the navigated URL.

Replace the complete in-memory state from the parsed URL, synchronize every control, and load that restored state.

3. Every query keystroke adds a history entry

Location: src/issue-search-controller.ts:46-48

The input handler calls update in push mode for each keystroke. Typing one search can therefore create many entries, making Back step through incomplete transient strings instead of the user's deliberate navigation.

Use replaceState for query typing and reserve pushed entries for the specified status and page changes.

4. URL writes discard unrelated parameters and the hash

Location: src/issue-search-state.ts:26-32

buildIssueSearchUrl starts with an empty URLSearchParams and returns only the pathname and the three search fields. An update silently drops independent state such as project, view, and the current fragment.

Clone the current URL, update only q, status, and page, and serialize the resulting path, query, and hash.

5. A query change keeps a page number that belongs to the old result set

Location: src/issue-search-controller.ts:46-48

The query update spreads the current state and changes only the text. If the user is on page 8, a new query immediately requests page 8 and can appear empty even when matching results exist on earlier pages.

Set page to 1 whenever q changes, just as the status handler already does.

Reasonable non-findings

Invalid, fractional, and non-positive page values already normalize to page 1 in readPage.
Resetting page for a status change is correct, and a page-only change should retain the current query and status.
Issuing a request on each input event is acceptable under this contract; debouncing would be an optional optimization, not a correctness requirement.
The returned cleanup removes all four listeners registered during mounting.
The test's focused initial-hydration coverage is useful; the actionable feedback belongs on the concrete history and request behaviors rather than a separate generic testing finding.