Skip to content
This repository has been archived by the owner on Dec 30, 2022. It is now read-only.

feat(infinite-hits): support cache #2921

Merged
merged 30 commits into from
Jun 8, 2020
Merged

Conversation

eunjae-lee
Copy link
Contributor

@eunjae-lee eunjae-lee commented Apr 27, 2020

Summary

This PR adds a cache layer to InfiniteHits.
With the addition, you can add custom cache which is more persistent and it will give more persistent UX especially when user leaves for product detail page, and comes back to infinite hits.

How to test

Load more pages a couple of times, click "Details", come back and see if the hits are instantly loaded and the scroll is restored.

This examples uses routing because the scroll restoration works when the routing is enabled. But, of course, without the routing the cache itself works fine.

@eunjae-lee eunjae-lee force-pushed the feat/infinite-hits-cache branch from e468e05 to ae83757 Compare April 27, 2020 14:57
@algobot
Copy link
Contributor

algobot commented Apr 27, 2020

Deploy preview for react-instantsearch ready!

Built with commit e468e05

https://deploy-preview-2921--react-instantsearch.netlify.app

@eunjae-lee eunjae-lee force-pushed the feat/infinite-hits-cache branch from ae83757 to a55cf1f Compare April 27, 2020 15:04
@algobot
Copy link
Contributor

algobot commented Apr 27, 2020

Deploy preview for react-instantsearch ready!

Built with commit fc0a8fc

https://deploy-preview-2921--react-instantsearch.netlify.app

@eunjae-lee
Copy link
Contributor Author

I didn't know well about browser's scroll restoration.

If you take a look at this example,

It's a very simple example and the scroll restoration doesn't work there.

  useEffect(() => {
    setResult(true);
  }, []);

  return (
    <div className="App">
      <h1>Hello CodeSandbox</h1>
      {result && (
           ....

I need to research more, but from my understanding, the scroll is restored right after the first dom rendering, which breaks this current example which renders the big chunk of dom elements at the second render.

For the same reason, when going back to a page with InstantSearch, it renders after /queries API. So the scroll is not restored.

This might initiate a new RFC which saves the current search result at beforeunload or something like that, and later restore it without triggering API call. Without this, when going back to InstantSearch, the page will flash and the scroll restoration needs to be done manually.

At the moment, we ship this as is and document how to manually restore scroll position, which is still better than before.

What do you think?

@eunjae-lee eunjae-lee marked this pull request as ready for review April 28, 2020 15:05
@eunjae-lee eunjae-lee requested a review from a team April 28, 2020 15:06
@eunjae-lee
Copy link
Contributor Author

I talked to Haroen, and he said there's a render before the search api call. So I'll try to render the InfiniteHits with the cache at the first render. Then the scroll might be restored correctly.

@eunjae-lee eunjae-lee marked this pull request as draft April 29, 2020 08:29
@eunjae-lee eunjae-lee force-pushed the feat/infinite-hits-cache branch from 9f13f91 to 900519a Compare May 4, 2020 12:57
@eunjae-lee eunjae-lee marked this pull request as ready for review May 4, 2020 13:03
@eunjae-lee eunjae-lee requested a review from yannickcr May 5, 2020 09:54
Copy link
Member

@francoischalifour francoischalifour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the new example supposed to be accessible at this address?

@eunjae-lee eunjae-lee requested a review from yannickcr May 6, 2020 12:47
@eunjae-lee
Copy link
Contributor Author

fixing tests

@eunjae-lee eunjae-lee force-pushed the feat/infinite-hits-cache branch 2 times, most recently from 4c64a33 to df59325 Compare May 12, 2020 09:16
@eunjae-lee eunjae-lee force-pushed the feat/infinite-hits-cache branch from df59325 to 5d603be Compare May 12, 2020 09:18
@eunjae-lee
Copy link
Contributor Author

Is "argos" expected to run, or not?
Should I do something to force-run the task?

@Haroenv
Copy link
Contributor

Haroenv commented May 12, 2020

We don't rely on Argos anymore, it was too flaky :)

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 18, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit fc0a8fc:

Sandbox Source
quirky-volhard-5sgvi Configuration
white-moon-gcubj Configuration
nervous-currying-3cx3h PR

Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code seems to be working well

@@ -120,8 +156,13 @@ export default createConnector({
},

refine(props, searchState, event, index) {
if (index === undefined && this._lastReceivedPage !== undefined) {
index = this._lastReceivedPage + 1;
const pages = Object.keys(this._cachedHits || {}).map(Number);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are storing elements with a number index in an object, what about using an array to store that instead? I guess it's no big difference either way

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially started with an object because user could've reached the page with params like page=3. Still in that case, an array can be used, though.

@eunjae-lee eunjae-lee merged commit 7b26adc into master Jun 8, 2020
@eunjae-lee eunjae-lee deleted the feat/infinite-hits-cache branch June 8, 2020 12:19
eunjae-lee added a commit that referenced this pull request Jun 15, 2020
* **infinite-hits:** support cache ([#2921](#2921)) ([7b26adc](7b26adc))
Haroenv added a commit that referenced this pull request Jul 20, 2020
# [6.7.0](v6.5.0...v6.7.0) (2020-07-20)

### Bug Fixes

* **core:** appending successful index search results by returning new object reference ([#2953](#2953)) ([0a711a7](0a711a7))
* **ssr:** remove second instance of "query" in the response "params" for SSR ([#2945](#2945)) ([bf837c5](bf837c5)), closes [#2941](#2941)

### Features

* **infinite-hits:** support cache ([#2921](#2921)) ([7b26adc](7b26adc))
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants