Conversation
✅ Deploy Preview for adapt-giving ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Did a little package update while I was here. Just minor/patch versions.
| q: indexUiState?.query, | ||
| page: indexUiState?.page, |
There was a problem hiding this comment.
Before, it would throw an error if these weren't "safely" accessed and the indexUiState was unavailable.
components/Search/Search.tsx
Outdated
|
|
||
| return ( | ||
| <InstantSearchNext | ||
| key={routeKey} |
There was a problem hiding this comment.
This was the magic fix to bring stability to the InstantSearch app.
There was a problem hiding this comment.
Pull request overview
Updates the site search implementation to better survive browser back/forward navigation on the /search-results route, and aligns related packages to newer versions.
Changes:
- Updated
Searchto memoize Algolia clients, harden routing state mapping, and forceInstantSearchNextremounts based on current URL search params. - Bumped Next.js / InstantSearch / Storyblok (and related) dependencies to newer patch/minor versions.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| package.json | Dependency bumps (Next.js, InstantSearch Next.js integration, Storyblok, Netlify, etc.). |
| package-lock.json | Lockfile refresh reflecting dependency upgrades and transitive additions. |
| components/Search/Search.tsx | Search routing/back-navigation behavior adjustments (memoization, keying, safer state mapping). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
yvonnetangsu
left a comment
There was a problem hiding this comment.
Tested and worked as described 👍🏼 Thanks for this update 🎉
There was a problem hiding this comment.
Pull request overview
This PR aims to resolve browser back/forward navigation issues on the /search-results route by making the InstantSearch state more resilient to remounts and avoiding crashes when restoring routing state.
Changes:
- Bump Next.js and related dependencies (including
react-instantsearch-nextjs) to newer patch/minor versions. - Update
components/Search/Search.tsxto memoize Algolia client/search client construction and adjust InstantSearch routing/state handling. - Add an InstantSearch configuration workaround (
key+future.preserveSharedStateOnUnmount) intended to improve back-navigation behavior.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| package.json | Updates framework/vendor dependencies related to Next.js, Netlify, Storyblok, and Algolia InstantSearch integration. |
| package-lock.json | Regenerates lockfile to reflect dependency upgrades and transitive dependency changes. |
| components/Search/Search.tsx | Adjusts InstantSearch client initialization and routing state mapping; introduces a remount/reset workaround via a changing key. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
components/Search/Search.tsx
Outdated
| @@ -54,13 +55,25 @@ export const Search = ({ | |||
|
|
|||
| return algoliaClient.search(requests); | |||
| }, | |||
| }; | |||
| }), [algoliaClient]); | |||
|
|
|||
| // This is a workaround to reset the InstantSearch state when the component is unmounted and remounted, | |||
| // which can happen when navigating between pages in Next.js. By changing the key of the | |||
| // InstantSearch component, we force it to reset its state. | |||
| useEffect(() => { | |||
| // eslint-disable-next-line react-hooks/set-state-in-effect | |||
| setKey(`search-component-${Date.now()}`); | |||
| }, []); | |||
There was a problem hiding this comment.
The key state is updated in a mount-only useEffect, which forces InstantSearchNext to mount twice (initial render with search-component, then immediate remount with the timestamp key). This can trigger duplicate searches/flicker and makes the inline comment misleading (it’s not tied to an unmount/remount). Consider initializing the key once in useState (lazy initializer) and removing the effect + lint-disable, or derive the key from navigation state so it only changes when you truly need to reset InstantSearch. Also double-check whether forcing a reset via key aligns with future.preserveSharedStateOnUnmount: true, since the two approaches are at odds (reset vs preserve).
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…nto troubleshooting
READY FOR REVIEW
Summary
Review By (Date)
Criticality
Review Tasks
Reproduction Steps
1) Search Results Page Crash / Hard Failure
Steps
athletics) and submit./search-results?q=athletics./) using the site logo/link (Stanford Giving).Actual (before fix)
Expected
Repeat with deploy preview
2) Missing Search Elements After Back Navigation
Steps
https://giving.stanford.edu/search-results?q=athletics(or perform a search from Home)./) via client-side navigation (logo/link)./search-results?q=athletics.Actual (before fix)
/search-results?..., but search UI elements are missing:Expected
/search-results?...should restore the full search UI and results state every time.Repeat with deploy preview