react-instantsearch icon indicating copy to clipboard operation
react-instantsearch copied to clipboard

Refresh does not request pages in infinite hits again

Open Haroenv opened this issue 4 years ago • 52 comments

Describe the bug 🐛

When updating the refresh token, infinite hits will not react to the refetch.

To Reproduce 🔍

Steps to reproduce the behavior:

  1. go to the example
  2. trigger next page
  3. refresh
  4. see no changes in the infinite hits, even if the index would have changed

The example's index can be changed to an index you control where you make changes to make the effect clearer.

https://codesandbox.io/s/react-instantsearch-app-dq2ro

Expected behavior 💭

Infinite hits reacts to refresh, either by:

  1. clearing the cache completely and restarting on the current page
  2. clearing the cache and the state and restarting on page 0

Likely we should go for option 2, unless there's a showPrevious.

Additional context

Add any other context about the problem here.

When refresh happens, we need to make sure the infinite hits internal cache is also invalidated.

On React Native, where the list component itself is stateful, we can not rely on the "key" hack, because it rerenders with an empty state when we simply clear the cache. What could be an option is:

  1. clear cache
  2. redo current search
  3. save in cache
  4. rerender

The problem is that you can't do that as a user reasonably, since you don't have access to the helper state.

A possible solution is:

In the function refresh in InstantSearchManager, emit an event to all widgets. Then in InfiniteHits, listen to that event, and clear the internal cache as we expect.

Another potential solution is to return a promise from the search which happens in refresh. This should allow people to rerender the InfiniteHits component manually.

Relevant pieces of code:

https://github.com/algolia/react-instantsearch/blob/ec9e0fbd6106d1c3e47f1dbfa6eaac3e20af6bd5/packages/react-instantsearch-core/src/core/createInstantSearchManager.js#L69-L72

Relevant issues:

https://github.com/algolia/react-instantsearch/issues/2464

Haroenv avatar Aug 06 '19 13:08 Haroenv

Hi all,

Just to give you a demonstration of what happens here if you refresh using a key. Unfortunately, you momentarily receive hits of length 0.

I have created an example to use ClearCache and a searchClient object - which does not work. https://snack.expo.io/@alexpchin/aglolia-refresh-problem Please run on iOS and look out for the red flash.

The full code is here: https://github.com/alexpchin/algolia-refresh-issue/

The docs say this method is (browserOnly)? https://www.algolia.com/doc/api-client/advanced/cache-browser-only/javascript/?language=javascript

I think it’s reasonable that the index updates and the results go back to page 0.

Thanks, Alex

alexpchin avatar Aug 22 '19 10:08 alexpchin

browserOnly implies not for node, it works in React Native. We are aware of this issue, but aren't sure what the solution could be unfortunately.

Without having a key, it behaves correctly, except for InfiniteHits, which keeps its cache and doesn't update. The problem is that we need to change things to be able to clear the cache on InfiniteHits too.

There's multiple options here:

  1. add an option to clear the internal cache of InfiniteHits (same problem though, when do you clear it?)
  2. Clear the internal cache of InfiniteHits when refresh is called (how?)

Haroenv avatar Aug 22 '19 10:08 Haroenv

Thank you for the clarification. I believe I was originally the person who contacted RE: the problem and was just adding some more information so that people could see clearly the effect that is causes with the hits being momentarily empty when using a key to refresh.

Our project relies heavily on Algolia to display infinite results.

The idea solution would be that when refresh is given to InstantSearch, an event is emitted to the InfiniteHits so that we do not have to re-mount the component with a key.

To the user, I believe the disruption is only seen by hits length becoming momentarily 0 When the component is re-mounted.

alexpchin avatar Aug 22 '19 11:08 alexpchin

Just to be 100% clear, what is the benefit of using the connectInfiniteHits connector over building a search using the Algolia Search API and passing filters to it? Something not dissimilar to (with a bit more work for pagination etc)?

import algoliasearch from 'algoliasearch/lite';
import * as C from 'src/constants';

const client = algoliasearch(C.ALGOLIA_APP_ID, C.ALGOLIA_API_KEY);

export const algoliaSearch = async ({
  indexName,
  query = '',
  filters = '',
  hitsPerPage = 20,
  attributesToRetrieve = ['*'],
}) => {
  try {
    const index = client.initIndex(indexName);
    const response = await index.search(query, {
      filters,
      attributesToRetrieve,
      hitsPerPage,
    });
    return response;
  } catch (err) {
    console.log(err);
    console.log(err.debugData);
  }
};

alexpchin avatar Aug 28 '19 15:08 alexpchin

If all you are doing is showing results, there's no real advantage to use InfiniteHits in your case. Since we haven't fully solved "refresh" for InfiniteHits, it's a fair solution to fall back to the JS client.

To refresh on it, call client.clearCache and call your search/render function again.

Sorry for not yet fixing this refresh issue in the mean time :)

Haroenv avatar Aug 29 '19 09:08 Haroenv

Hi all,

I was just wondering whether there was any update to this? I'm still having a little trouble "rolling my own" infinite hits using the API. My setup requires the ability to search, show no duplicated results, have infinite scroll etc. Everything with the ConnectInfiniteHits works except the problem stated above.

Thanks in advance, Alex

alexpchin avatar Sep 10 '19 14:09 alexpchin

Sorry @alexpchin, this is very complicated from our end to get something like this out the door, and it has not yet been prioritised as an issue essential to fix right now.

Haroenv avatar Sep 10 '19 15:09 Haroenv

No problem! Thanks for the swift response 👍

alexpchin avatar Sep 10 '19 21:09 alexpchin

Hi @Haroenv any plans to implement this?

alexpchin avatar Nov 13 '19 02:11 alexpchin

Not currently, since you're the only person who has noted this bug / wrong behaviour. I totally agree that it's a problem, but since the solution is unclear we don't have fixing this planned. Sorry!

Haroenv avatar Nov 13 '19 09:11 Haroenv

Hi @Haroenv I'm also having this issue. Tried to implement scroll to top and refresh the search when user taps the tab bar. Using infinitehits on react native. Any workarounds besides re-mounting the entire search component?

yilinjuang avatar Feb 19 '20 20:02 yilinjuang

You can remount the <InfiniteHits> component alone, at the same time as using refresh, which will clear its cache

Haroenv avatar Feb 20 '20 09:02 Haroenv

Just dropping in to say I've encountered the same issue.

DRoghanian avatar Mar 09 '20 13:03 DRoghanian

I have also encountered this issue. I use the pull to refresh of Flatlist with a callback which sets the refresh to true and remounts the <InfiniteHits>. The first time I pull it doesn't show an updated list but the second time I pull then it shows the updated list.

update: as a workaround, I remount the <InfiniteHits> after a second using setTimeout and it seems to be working. Not ideal but at least it works for now.

hamedhemmati avatar Apr 30 '20 20:04 hamedhemmati

Refresh with InifiniteHits seems not to work

JimTeva avatar Jun 21 '20 21:06 JimTeva

My only workaround was to clear cache of inifiniteHits manually using searchClient.clearCache()

JimTeva avatar Jun 21 '20 22:06 JimTeva

You can control the internal cache of InfiniteHits now.

import isEqual from 'react-fast-compare';

function getStateWithoutPage(state) {
  const { page, ...rest } = state || {};
  return rest;
}

function getInMemoryCache() {
  let cachedHits = undefined;
  let cachedState = undefined;
  return {
    read({ state }) {
      return isEqual(cachedState, getStateWithoutPage(state))
        ? cachedHits
        : null;
    },
    write({ state, hits }) {
      cachedState = getStateWithoutPage(state);
      cachedHits = hits;
    },
    clear() {
      cachedHits = undefined;
      cachedState = undefined;
    }
  };
}

const cache = getInMemoryCache();

<InfiniteHits 
  ...
  cache={cache}
/>

And later, you can call cache.clear() right before you refresh.

The internal cache of InfiniteHits should be cleared when refreshing. But, until it's properly fixed, that can be another workaround.

eunjae-lee avatar Jun 23 '20 15:06 eunjae-lee

@eunjae-lee can this also be done when using connectInfiniteHits? I didn't see anything in the documentation. Can I just pass it as an additional prop (see point 3)?

// 0. Initialize the cache
const sessionStorageCache = createInfiniteHitsSessionStorageCache();

// 1. Create a React component
const InfiniteHits = () => {
  // return the DOM output
};

// 2. Connect the component using the connector
const CustomInfiniteHits = connectInfiniteHits(InfiniteHits);

// 3. Use your connected widget
<CustomInfiniteHits cache={cache} />

meck93 avatar Oct 28 '20 09:10 meck93

Yes, the connector will accept the cache prop as well @meck93. You can check that by using a custom cache which has some logging :)

Haroenv avatar Oct 28 '20 09:10 Haroenv

Great, thanks a lot 👍

meck93 avatar Oct 28 '20 09:10 meck93

I am also encountering this issue. Spent hours at this point trying to figure out what's going on. @Haroenv if Algolia isn't going to fix it, it'd be really helpful to at least have a warning in the docs so that others don't find themselves wasting time like I did.

Also react-instantsearch-native does not appear to have the same ability to override the cache.

andrewbeckman avatar Dec 24 '20 18:12 andrewbeckman

react native InstantSearch should work exactly the same, since the connector is just forwarded from the "core" version, so I expect something else is going on.

This is something we're planning to fix in the future, however I don't see how we can give a warning in this case unfortunately?

Haroenv avatar Dec 30 '20 14:12 Haroenv

Hi @Haroenv I'm just circling back to this. I've managed to get this work on the web by using:

<CustomHits
  cache={createInfiniteHitsSessionStorageCache("ais.tattoos")}
/>

Combined with:

import _objectWithoutProperties from "@babel/runtime/helpers/esm/objectWithoutProperties"
import isEqual from "react-fast-compare"

function getStateWithoutPage(state) {
  var _ref = state || {},
    // page = _ref.page,
    rest = _objectWithoutProperties(_ref, ["page"])

  return rest
}

function hasSessionStorage() {
  return (
    typeof window !== "undefined" &&
    typeof window.sessionStorage !== "undefined"
  )
}

export default function createInfiniteHitsSessionStorageCache(
  KEY = "ais.infiniteHits"
) {
  return {
    read: function read(_ref2) {
      var state = _ref2.state

      if (!hasSessionStorage()) {
        return null
      }

      try {
        var cache = JSON.parse(window.sessionStorage.getItem(KEY))
        return cache && isEqual(cache.state, getStateWithoutPage(state))
          ? cache.hits
          : null
      } catch (error) {
        if (error instanceof SyntaxError) {
          try {
            window.sessionStorage.removeItem(KEY)
          } catch (err) {
            // do nothing
          }
        }

        return null
      }
    },
    write: function write(_ref3) {
      var state = _ref3.state,
        hits = _ref3.hits

      if (!hasSessionStorage()) {
        return
      }

      try {
        window.sessionStorage.setItem(
          KEY,
          JSON.stringify({
            state: getStateWithoutPage(state),
            hits: hits,
          })
        )
      } catch (error) {
        // do nothing
      }
    },
  }
}

This allows me to create a different store for each page. However, I'm still a bit stuck on how best to implement this in react-native.

Do you have a guide for clearing the cache for multiple connectInfiniteHits and there no solution where we can clear the internal cache without having to create our own?

Thanks and Happy New Year!

alexpchin avatar Jan 13 '21 22:01 alexpchin

Hi @Haroenv Thanks for the 👍 Do you have a suggestion for resolving this in react-native? I still cannot seem to get things working well. Is it related to: https://github.com/algolia/react-instantsearch/issues/2995 ?

alexpchin avatar Jan 16 '21 14:01 alexpchin

I was still looking what's going on, but I don't have a good react native sandbox and setup, so that took a while, and I had to work on other things. If you can recreate the bad behaviour in a GitHub example, that would help a lot!

Haroenv avatar Jan 18 '21 10:01 Haroenv

Hi @Haroenv

I did make a github repository a while ago: https://github.com/algolia/react-instantsearch/issues/2742#issuecomment-523837227 However, it is a little outdated now so I made a new one here:

https://github.com/alexpchin/react-instant-search-refresh

You will need to change the APP_ID and Index values to one where you can delete an object from an index to see the behaviour.

alexpchin avatar Jan 23 '21 11:01 alexpchin

Hi @Haroenv did you have any suggestion for this fix? Currently, I'm having to use the connectHits and loading a large number of results because I can't seem to get the cache to clear well on connectInfiniteHits in react-native.

alexpchin avatar Jan 29 '21 10:01 alexpchin

can you contact [email protected] with the full reproduction? this way there will be a reminder to reply to you?

Haroenv avatar Feb 01 '21 08:02 Haroenv

@Haroenv I will do that now.

alexpchin avatar Feb 01 '21 09:02 alexpchin

Hello @Haroenv @eunjae-lee I've just updated to "react-instantsearch-native": "6.10.0" following #3011. It seems that the cache does clear if I use the cache prop with a custom cache:

import isEqual from 'react-fast-compare';

function getStateWithoutPage(state) {
  const { page, ...rest } = state || {};
  return rest;
}

function getInMemoryCache() {
  let cachedHits = undefined;
  let cachedState = undefined;
  return {
    clear() {
      cachedHits = undefined;
      cachedState = undefined;
    },
    read({ state }) {
      return isEqual(cachedState, getStateWithoutPage(state))
        ? cachedHits
        : null;
    },
    write({ hits, state }) {
      cachedState = getStateWithoutPage(state);
      cachedHits = hits;
    },
  };
}

export const cache = getInMemoryCache();

However, it doesn't seem to clear if I don't use a cache prop. Is this expected with this fix?

alexpchin avatar Feb 23 '21 14:02 alexpchin