-
Notifications
You must be signed in to change notification settings - Fork 530
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Instantsearch hooks: router not getting along with Next.JS #5241
Comments
Just to double check, is that with using the getLocation call, like in the hooks-next example? https://codesandbox.io/s/github/algolia/react-instantsearch/tree/master/examples/hooks-next It's definitely not a simple problem keeping the state and routers in sync, so if there's a specific edge case missed I'd love to fix that |
Hey @dsbrianwebster – this is likely because Next.js is using Strict Mode, which isn't yet supported in the library with React 18. Our We're working on React 18 Strict Mode support this week so you can expect a fix in the coming days. In the mean time, could you disable Strict Mode on the Edit: Strict Mode support was introduced in v6.28.0. |
Hi @francoischalifour. Thanks for the response! Yeah we have been seeing a bunch of "additional render' issues with other packages after upgrading to React 18 and pretty much roll with strictMode off by default on all projects for the time being. I can confirm our next config is... and we still experience these issues. |
Hi @Haroenv thanks for the response. And yes, no doubt, def not a simple problem keeping the state and router in sync. Its feels so close 🏁 ! I've forked your sandbox to demonstrate item 2 (Routing example above)... https://codesandbox.io/s/hooks-next-bug-3506-item-2-6ekug6?file=/pages/index.tsx To reproduce...
Perhaps issue is something to do with how the InstantSearch component / router is unmounted. |
@francoischalifour + @Haroenv we've dug a little deeper on Item 1 (The Re-Render Example) and seem to have isolated the issue we're experience there as having something to do with our effort to implement an SEO-friendly URL structure. We're still investigating and will either circle back with an answer there or a sandbox example. The sandbox provided in the previous comment is still very relevant for @Haroenv as it pertains to item algolia/react-instantsearch#2 edge case. |
I'm also experiencing the second issue (broken back button).
Possible clue for narrowing down the issue:
|
I'm running into issue no. 2 here as well, the broken back button. I think it's caused by the instantsearch.js history implementation performing it's own history mutations, over here: e.g., try the following on any app rendered by next.js (in this example I'll use https://nextjs.org/)
import { InstantSearch } from "react-instantsearch-hooks-web";
import { history } from 'instantsearch.js/es/lib/routers';
import { useRouter } from "next/router";
const router = useRouter();
<InstantSearch
routing={{
router: history({
// please note that `onUpdate` is not an existing part of the api
onUpdate: (url) => router.push(url),
})
}}
/>
Edit: the above is already possible with a custom history handler as suggested below. |
You can provide a custom router, you don't need to use I'd advise looking at the implementation of the history router, because there's some edge cases we already cover there as a base |
@klaasman Do you actually have this working? Because it doesn't look like the It would be nice if Please let me know if I'm misunderstanding any of this. |
@joshgeller Sorry for the lack of clarity, what I meant is that it would be nice to make that FYI: I've copied the original |
Can you maybe tell us the status of the Strict Mode support and good routing in next? |
Hi @francoischalifour , i'm still experiencing the 2 issues mentioned by @dsbrianwebster (i'm using nextjs 12 & react 17), especially the broken back button |
I also have this warning:
And when I switch pages in Next like this:
Its crashing in the instantsearch clean up:
Here is a snippet of line 530
|
Hi, I'm also still experiencing issue number 2, mentioned by @dsbrianwebster (i'm using nextjs 12 & react 18). Has anyone come up to a solution? The custom router implementation maybe? Thanks |
@dhayab posted a workaround using Next useRouter and useEffect in a discussion. |
Thanks, but I couldn't make it work with that approach as I have a whole application with tons of search state, that is not attached to a single React useState as it is on the sandbox example. Because of that, I wasn't able to apply URL changes to the algolia UiState, because I don't have access to a function such as "setIndexUiState" provided by useInstantSearch, because hooks can't be called outside the InstatSearch Wrapper. (in the sandbox it uses directly useState setter). |
Thanks @dhmacs, I used that workaround for the moment, triggering a page reload once the popstate occurs over the search page. Something simple like this for the moment, until issue is fixed:
|
I'm experiencing the same back button routing bug as @dsbrianwebster. I recreated it and you can use this vercel link and this source code to check it out. |
I ran into this problem as well and created a, pretty hacky, fix that seems to work (not used in production yet, so fingers crossed). What I did was taking full control of the routing, not using the router provided by Instantsearch. Requirement for this solution is a helper that can convert an UiState to a URL (like the createURL function in the algolia router) and back again (like the parseURL function). In basis the solution works like this:
So you now control the updating of the UiState yourself, and use this to update the NextRouter accordingly. However this is not enough, it works one way only. On
Note that I pass This would work a lot more smoothly if we could actually get a |
Experiencing the same routing problem as described by @dsbrianwebster in n.2 issue. |
@JulesVerdijk your solution mostly worked but broke our infinite scrolling. We've decided to go back to the old instantsearch package which is a shame. @dhayab, is there an ETA on a fix? |
This workaround works for me. |
Hi, here's a sandbox with another workaround that shares some similarities with @JulesVerdijk : https://codesandbox.io/s/rish-next-router-handler-is22ev?file=/pages/%5Bcategory%5D.tsx. It consists of a
It required an explicit definition of the route/state mapping, and supports Next.js dynamic routes (by specifying the route query to inject). I tested it on some sandbox shared in the issues, and it works well. Feel free to share below if there are edge cases that are not handled by this workaround. |
@LefanTan : @wenche : |
@aymeric-giraudet Works perfectly for my use case and just pushed it to production :) |
@aymeric-giraudet Thank you for your reply. I can confirm that things seems to work fine without Might also be an idea to highlight this gotcha in the package's README since it's strongly suggested to enable this flag and for example create-next-app enables this by default. Looking forward to the final release of this package as it solves on of our main issues with using Algolia and Next 🙌 |
Hi @wenche, thanks again for the replies ! |
When using the back button from a page not using InstantSearch to my InstantSearch page, every facet is removed from the url (and from the state). Is it the expected behavior ? |
Doesn't seem to work well with dynamic paths. |
@adesege do you have a reproduction? the example in the repo is using a dynamic path |
@dhayab's fix works for me. My use case requires using one of the facets as a path param. However, using instantsearch-router-next-experimental did not properly re-render the next page when the facet is selected. |
Hi @adesege, do you mean that when you click on a |
First of all, thank you, and congratulations on the work here. It's great to know that we can count on people like you here. :) I know you replied to osseonews that the BACK BUTTON ISSUE was resolved in this package, however, in our project, this problem persists.
From what I've read here, what's happening in our project is the same as in other people's reports.
So, I'm curious to know if you're still receiving reports of this Back Button Issue or if you're aware that the problem might still exist. Also, do you have any predictions as to when the official production use package will be released? We're eagerly waiting for it, and any information you could provide would be greatly appreciated! Thanks! |
The production package has been released, but not yet announced as we didn't finish the documentation. In the mean time you can use the package already: https://www.npmjs.com/package/react-instantsearch-hooks-router-nextjs We're not aware of any further issues relating to the back button, so please create a reproduction so we can debug it. |
Hi @franciskodama, You can also check out this CodeSandbox that shows the router in action : https://codesandbox.io/s/github/algolia/instantsearch/tree/master/examples/react-hooks/next-routing Even by adding a |
@franciskodama I have the same problem with the back button after updating to
|
@franciskodama @JugglerX have similar problem currently with
|
Hey everyone! Sorry for the delay in getting back to you. We're still trying to figure out what's going on, but it seems like the package that Haroenv mentioned above is working well. We realized that the issue is something on our end and we still don't have a solution for that. I just wanted to take a moment to thank all of you for your help, @aymeric-giraudet, and @Haroenv. Your assistance has been much appreciated! |
@mikkio-j, @JugglerX, @franciskodama, does any of you have a reproduction of this issue? |
@JugglerX : @mikkio-j , @franciskodama : |
package.json:
Everything works as expected until we start using sorting. As soon as we use sorting and hit a product the browser back button does not work anymore. Filtering works as expected. |
Hi @DB-Alex, May you send a reproduction please ? |
Hi @mikkio-j, I've seen a ticket you've raised to our support and checked, it's not due to routing really, it's the way Next.js, React and InstantSearch work.
I'll send back a fork of your CodeSandbox with the solution to customer support as it might be private for you :) P.S : Overall maybe this could be avoided by relying on |
Hi everyone ! 🙋♂️ First of all, thank you for your patience and for providing precious feedback. Our solution for creating a Next.js compatible router for InstantSearch is now generally available and published on npm as Its documentation is available on the Algolia docs right there : https://www.algolia.com/doc/api-reference/widgets/instantsearch-next-router/react-hooks/ You can also see it in action on CodeSandbox : https://codesandbox.io/s/github/algolia/instantsearch/tree/master/examples/react-hooks/next-routing If you're seeing any problem still, please raise another issue with a reproduction and make sure to check that it's not just related to back button press, try to see if reloading the page raises the same issue. If you're also on a dynamic route, check that the issue is not caused by components/hooks being unmounted. Thanks again to all of you ! |
This was resolved by adding a key to the SSRProvider:
|
@aymeric-giraudet Thank you for working so hard on a fix for this issue. I wanted to provide an update on my solution. The problem for me seemed to be that my Here is some more detail as It might help others: I needed to add the new package I was able to get a working version with a basic new Nextjs site. But when I installed the above on my existing site the back button issue continued to occur. Stripping back my Nextjs site, the issue was resolved when I removed the layout component that wrapping each page. I modified my layout architecture to that suggested in the Nextjs docs here https://nextjs.org/docs/basic-features/layouts#per-page-layouts |
@aymeric-giraudet I found the issue with the back button! Works!:
Breaks back button:
Even with this key:
|
@aymeric-giraudet Installed the new package and it works great with refinement, menus, clicking through to a Hit and then using the back button. However, I was previously sending url parameters to pre-set search and Menu and those are being cleared from the URL and not applied to the InstantSearch state. Any suggestions on how to tackle pre-defined url parameters? Steps to issue:
|
@kblizeck, could you create a reproduction and a new issue please? |
Hi everyone ! 🙌 I'm sorry but I will have to lock the issue so that people don't have to scroll up to find the solution. Our solution for creating a Next.js compatible router for InstantSearch is now generally available and published on npm as react-instantsearch-hooks-router-nextjs. It has to be a separate package as it has next as a peerDependency. Its documentation is available on the Algolia docs right there : https://www.algolia.com/doc/api-reference/widgets/instantsearch-next-router/react-hooks/ You can also see it in action on CodeSandbox : https://codesandbox.io/s/github/algolia/instantsearch/tree/master/examples/react-hooks/next-routing If you're seeing any problem still, please raise another issue with a reproduction. Thanks again to all of you ! |
We are trying out a server rendered search implementation with NextJS. All seems okay until we try to add a routing object to InstantSearch. We are using routing to have urls for our search state, and to make them SEO friendly (q=potato&type=tuber). There are all sorts of quirks though, from additional re-renders on load to the rest of the application routing breaking.
1. Re-Render Example:
If we try starting at url
/search?q=potato
we immediately see a re-render to/search
.2. Routing Example:
When we click on a hit/result
<Link href={
/post/${hit.slug}>🥔 hit</Link>
we are taken to our expected/post/${hit.slug}
url, but then from there our routing in general seems to be broken. Clicking back moves to/search?q=potato
, but only the url changes. Page content is not updated.The text was updated successfully, but these errors were encountered: