instantsearch: Performance regression from dom to hooks - hits hooks and components have costly `escapeHits`
🐛 Bug description
I noticed that performance of updating the search query became quite a bit worse when I changed from -dom
to -web-hooks
.
Some functions that seem responsible for pretty much all of the wait time:
escapeHits
recursiveEscape
getWidgetRenderState
🔍 Bug reproduction
Steps to reproduce the behavior:
- Set up a basic React-Instantsearch with
Hits
andSearchBox
withreact-instantsearch-dom
- Start typing, note the quick rendering
- Switch to
react-instantsearch-dom
- Start typing, note the slower rendering
Sorry for the lack of demo repo, my project is currently closed source. I’ll try to dive in deeper, I’ll update this issue
Some findings / thoughts:
- The issue scales with the amount of
HitsPerPage
- Even if my custom
Hit
component returns null, the issue is bad, so it’s not really the rendering part, I think. - It may have something to do with my relatively large documents, which contains many keys (about 100 KV combos)
Live reproduction:
https://codesandbox.io/s/github/algolia/react-instantsearch/tree/master/examples/hooks
Environment
- React InstantSearch Hooks version: 6.38.0 (also tried 6.40.0)
- React version: 18.2.0
- Browser: Firefox 108
- OS: MacOS
POSSIBLE FIX:
- Set
escapeHTML
to false in allInfiniteHits
/Hits
components anduseHits
,useInfiniteHits
hooks. - Use production build
About this issue
- Original URL
- State: open
- Created 2 years ago
- Reactions: 1
- Comments: 15 (6 by maintainers)
Commits related to this issue
- chore(major): leave major comments on how to improve highlighting A problem we currently have (see https://github.com/algolia/instantsearch/issues/5237) is that escapeHTML is one of the slower parts ... — committed to algolia/instantsearch by Haroenv a year ago
- chore(major): leave major comments on how to improve highlighting (#5648) A problem we currently have (see https://github.com/algolia/instantsearch/issues/5237) is that escapeHTML is one of the slowe... — committed to algolia/instantsearch by Haroenv a year ago
- useHits: don't escape html in the results We don't need this as we don't treat any of the fields as html (i.e. we don't use `__dangerouslySetHtml=...` on any of our result fields. This saves another... — committed to iFixit/react-commerce by danielbeardsley a year ago
- useHits: don't escape html in the results We don't need this as we don't treat any of the fields as html (i.e. we don't use `__dangerouslySetHtml=...` on any of our result fields. This saves another... — committed to iFixit/react-commerce by danielbeardsley a year ago
@Haroenv: I’ve setescapeHTML
tofalse
in theuseInfiniteHits
hook (only one instance) and in theInfiniteHits
component, but I can still seeescapeHits
being called (and causing about 80% of all cycles in the JS), also in the production bundle. I think theescapeHits=false
options are not working properly in either theuseInfiniteHits
hook or in theInfiniteHits
components.EDIT: Whoops… I think I missed my
useHits
hook. Everything seems fine now!@AntoineDuComptoirDesPharmacies, Hits already exposes
sendEvent
tohitComponent
👍@Haroenv
No multiple components, but I do have a
useHits
somewhere else to render the count.Let me try this:
EDIT: it works! Updated post
escapeHTML can be avoided in React, because you don’t render dangerouslySetInnerHTML, therefore it can be faster if you do this:
https://codesandbox.io/s/silly-fermi-qj4dbn?file=/App.tsx:4270-4410
I’ll think about how this can be improved and leads to less wasted cycles by default too
Thanks for the fast reply! I haven’t yet tried production mode.