Fix PageTitleInput reset on wiki site change#158
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Add key={wikiUrl.href} to PageTitleInput to force React to unmount
and remount the component when the wiki site changes. This fixes three
issues:
1. pageTitle state - now resets to searchState.pageTitle
2. debouncedTitle - debounce timer is cleared and restarted
3. ErrorBoundary - stale errors from the old wiki are cleared
The key prop is React's documented pattern for "resetting all state
when a prop changes" and requires no changes inside PageTitleInput.
Co-Authored-By: Claude
bf8e7b1 to
f306229
Compare
Pull Request ReviewSummaryThis PR fixes a bug where PageTitleInput state wasn't resetting when users switched between wiki sites. The solution uses React's key prop pattern to force component remounting—clean, idiomatic, and well-tested. ✅ Strengths1. Correct Solution PatternUsing key={wikiUrl.href} is the React-recommended approach for resetting component state when a prop changes. This is superior to alternatives like adding useEffect hooks (imperative, more complex), manually managing reset logic (error-prone), or just adding to ErrorBoundary resetKeys (only partial fix). 2. Excellent Test CoverageThe new test at line 332-360 is well-structured with clear arrange-act-assert pattern, tests the actual user-facing behavior (input value resets), has good inline comments explaining the expected behavior, and verifies the bug is fixed without over-specifying implementation details. 3. Comprehensive Problem AnalysisThe PR description clearly documents all three issues being fixed (useState, useDebouncedValue, ErrorBoundary), why this approach was chosen over alternatives, and how TanStack Query's queryKey already handled the re-fetch correctly. 4. Code ConsistencyThe pattern is already used elsewhere in the codebase at SearchForm.tsx:65 and WikiSelector.tsx:23. 5. TDD ApproachThe commit message indicates the test was written to verify the fix and can catch regressions—excellent practice aligned with the project's TDD requirements. 💡 Minor Observations1. Duplicate Test Name (Lines 407, 422)There are two tests with identical names in SearchForm.tsx. While the implementations differ, having duplicate test names can make debugging harder when a test fails. Consider renaming the second test to something like "updates radio button defaultChecked on searchState change". 2. Test Pattern ConsistencyThe test at line 308-330 already tests wiki selector changes using the same fireEvent pattern. The new test is valuable and distinct, but consider extracting shared logic if more similar tests are added in the future. 🔍 Code Quality Assessment
RecommendationAPPROVE ✅ This is a high-quality fix that solves the stated problem elegantly, uses React best practices, includes proper test coverage, and follows the project's functional programming style and TDD workflow. The duplicate test name is a minor nit that can be addressed in a follow-up if desired, but it doesn't block this PR. The fix demonstrates good understanding of React's reconciliation algorithm. By changing the key prop when wikiUrl.href changes, React treats it as a different component instance, unmounting the old PageTitleInput (cleaning up all state, timers, error boundaries) and mounting a fresh one with initial props. This requires zero changes inside PageTitleInput itself—a testament to good component design and separation of concerns. Nice work! 🎉 |
Summary
Fixes PageTitleInput state not resetting when users switch wiki sites (e.g., en.wikipedia.org → ja.wikipedia.org).
Problem
When the wiki selector changed, PageTitleInput retained stale state:
pageTitle(useState) — didn't re-initialize (React reused component instance)debouncedTitle(useDebouncedValue) — carried over from old wiki[debouncedTitle]only)TanStack Query already handled re-fetching correctly via
queryKey: ['pageId', wikiUrl.href, pageTitle]— no changes needed there.Solution
Added
key={wikiUrl.href}to<PageTitleInput>in SearchForm.tsx.This is React's documented pattern for "resetting all state when a prop changes." When the key changes, React unmounts the old instance and mounts a fresh one, resetting all internal state to initial values.
Why This Approach
Option A (chosen):
key={wikiUrl.href}Option B (rejected): Add
wikiUrl.hrefto ErrorBoundary resetKeyskeyTesting
key={wikiUrl.href}✓Test Plan
Co-Authored-By: Claude