-
-
Notifications
You must be signed in to change notification settings - Fork 134
[1753] Replace custom routing with react-instantsearch-router-nextjs #1766
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
base: main
Are you sure you want to change the base?
[1753] Replace custom routing with react-instantsearch-router-nextjs #1766
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - I don't think this needs to wait for the routing bugs to be fixed to merge (if anything, we should merge this to ensure the fix for those bugs is compatible with this change), but we can discuss at hack night before merging.
e2b118a
to
de8d970
Compare
One thing that just occured to me was this seems to obliterate our Feature Flag handling - we have the ability to use query params to override feature flags on the front end for testing, and we had to add a special workaround to ignore query params with flagNames in the Instantsearch routing so that these flags weren't obliterated by the Instantsearch router logic. If we can, it'd be nice to try and preserve this flag query param logic with the new router: maple/components/search/useRouting.tsx Line 12 in 96308cb
It may also just be time for us to upgrade to a better feature flag override solution - if fixing this proves too painful, I don't think this is a blocker. |
de8d970
to
fa1c7f1
Compare
2075295
to
b76f5f0
Compare
b76f5f0
to
58b11b4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - going to wait to deploy until next week because the V2 launch is tomorrow and we're putting a blocker on prod out of an abundance of caution.
Summary
Replaces our custom routing with the router provided by
react-instantsearch-router-nextjs
. This package is Algolia's official solution for nextjs compatible routing.Known issues
There are currently some navigation bugs. Upgrading to use this package doesn't solve those bugs, so I recommend that this gets merged after those issues are resolved.