Skip to content
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

Url params not updating when using react-instantsearch-nextjs (after upgrade to 0.4.2) #6541

Closed
1 task done
8byr0 opened this issue Jan 22, 2025 · 5 comments · Fixed by #6542
Closed
1 task done

Comments

@8byr0
Copy link

8byr0 commented Jan 22, 2025

🐛 Current behavior

When using [email protected] with routing, url params are updated once, only after the first refinement is applied (refinement checked or query changed).

After that url is never updated again.

🔍 Steps to reproduce

  1. open the sandbox
  2. Toggle a brand from the refinements list
  3. See that url is updated
  4. Toggle another one
  5. See that url did not change (but hits did properly)

Live reproduction

https://codesandbox.io/p/devbox/example-react-instantsearch-next-app-dir-example-forked-g7v9wp

💭 Expected behavior

Url should be updated anytime a refinement changes.

Reverting to [email protected] fixes the issue. Probably related to #6534

Package version

[email protected] [email protected] [email protected]

Operating system

No response

Browser

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@8byr0 8byr0 added the triage Issues to be categorized by the team label Jan 22, 2025
@aymeric-giraudet
Copy link
Member

Hey @8byr0,

Thanks for the reproduction. I see there are hydration problems at first, it may be related. I'm checking right now

@aymeric-giraudet aymeric-giraudet removed the triage Issues to be categorized by the team label Jan 22, 2025
@aymeric-giraudet
Copy link
Member

aymeric-giraudet commented Jan 22, 2025

The hydration problem seems to only occur with the CSB preview window, it still works directly from that link :
https://g7v9wp-3001.csb.app

So it has to be something different. Do you have this hydration problem on your own environment ? Or something that updates the <Search> component, like a state upwards ?

@8byr0
Copy link
Author

8byr0 commented Jan 22, 2025

Thanks for checking !

I just checked and yes, I do have an hydration error. If I disable the faulty component then routing works properly.

This error is not new, I had it before and it did not prevent routing with 0.4.1

@8byr0
Copy link
Author

8byr0 commented Jan 22, 2025

Seems like my hydration errors come from the use of "useInstantSearch". If I use "useInstantSearchContext" then it works without errors

Using useInstantSearch

Error : nbHits on the server is not the same as on the client therefore rendering callback and hiding children (which then changes on the client)

import React from 'react';
import { useInstantSearch } from 'react-instantsearch';
import { twMerge } from 'tailwind-merge';

interface NoResultsBoundaryProps {
  fallback: React.ReactNode | null;
  children: React.ReactNode;
}

export const NoResultsBoundary: React.FC<NoResultsBoundaryProps> = ({
  children,
  fallback,
}) => {
  const { results } = useInstantSearch();
 
  const hasResults = React.useMemo(() => {
    return results.nbHits > 0;
  }, [results]);

  return (
    <>
      {!hasResults && <div>{fallback}</div>}
      <div className={twMerge(hasResults ? '' : 'hidden')}>{children}</div>
    </>
  );
};

Using useInstantSearchContext

No errors

import React from 'react';
import { useInstantSearchContext } from 'react-instantsearch';
import { twMerge } from 'tailwind-merge';

interface NoResultsBoundaryProps {
  fallback: React.ReactNode | null;
  children: React.ReactNode;
  mainIndex?: string;
}

export const NoResultsBoundary: React.FC<NoResultsBoundaryProps> = ({
  children,
  fallback,
  mainIndex = 'my_index',
}) => {
  const { renderState } = useInstantSearchContext();

  const hasResults = React.useMemo(
    () => (renderState?.[mainIndex]?.hits?.items.length || 0) > 0,
    [renderState, mainIndex],
  );

  return (
    <>
      {!hasResults && <div>{fallback}</div>}
      <div className={twMerge(hasResults ? '' : 'hidden')}>{children}</div>
    </>
  );
};

Thoughts

Changing to useInstantSearchContext then fixes the hydration error and the routing issue.
Is it an intended behavior that routing fails on hydration errors? Maybe an explicit error / warning could be useful in such cases to inform users

@aymeric-giraudet
Copy link
Member

@8byr0

It's not intended, but at the same time we don't test if the implementation works even with hydration errors. Not sure whether it should be the case or not honestly, but I still went ahead and shipped a fix !

About useInstantSearch, I think server and client render differently because useInstantSearch does not wait for results like widgets do. We're thinking of providing a hook to manually wait for results on the server once we release the stable version of the package.

Does the renderState solution always have hasResults == false on the server and when hydrating on the client ? I expect that's why this works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants