-
Notifications
You must be signed in to change notification settings - Fork 525
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
getRefinements can receive null and system crashes when it does. #6441
Comments
In some cases `results` in `render` can be null: - no index name passed - stalled render In the vast majority of cases we already handled `results` being `null` or `undefined` just in case, but now i've changed the type to ensure we always catch this problem This issue doesn't have a reproduction, but i believe it now guards for all possible cases of "no results", so: fixes #6441 [CR-7353]
Thanks for catching this, I believe the case you're ending up may be when you're having a widget that reads results (current refinements, or clear refinements probably in this case) in a root index without index name passed. The error is confusing so I've opened a PR to guard for that case and ensure that we can't have other cases of this behaviour |
Hmm. I'm using a tabbed multi-index search were the query is always visible, but it isn't associated to any of the child indexes. So I'll guess that's the source point of the error. The indexes being searched are somewhat unrelated so I didn't set a master indexName on the InstantSearch object itself, only specifying them on the children. I'll go ahead and set one and see if the error goes away and if the presence of the keyword doesn't break anything. My pattern is therefore something like this <InstantSearch>/*no indexName specified here */
<SearchMast /> /* The query box is inside this component */
<Index indexName="provider">Provider searching - additional filters specific to providers here</Index>
<Index indexName="location">Location searches - additional filters specific to locations here</Index>
<Index indexName="page">Page searches</Index>
</InstantSearch> |
Yes, this verifies that the pull request would fix your use case. What you can do to work around it as well is pass a dummy index name, and filter that out in the In SearchMast do you only have search box, or other widgets too? |
I used to have the Location control up there but I've since moved it. The location control has useSortBy in it and allows the user to provide their location and order results in distance from them order. |
By "filter that out in the search method" is this what you had in mind? const algoliaClient = algoliasearch(appId, appKey);
const searchClient = {
...algoliaClient,
search(requests) {
const filtered = requests.filter( request => request.indexName !== 'root')
const rootPresent = filtered.length === requests.length
const query = algoliaClient.search(filtered)
return query.then(response => {
if (rootPresent) {
response.results.push({
index: 'root',
hits: [],
nbHits: 0,
nbPages: 0,
page: 0,
processingTimeMS: 0
})
}
return response
})
}
}
export default searchClient Cause it sorta works, but the child indexes aren't getting their data and all the renders fail. Still sorting out why. |
the fake response needs to be in the same position as in the requests, so the unshift instead of push (and really to be more correct you'd want to splice, keep track of the index etc, but in most cases it will be first) |
Since the response requests are in the same order other than the omission of the root a map is sufficient. response.results = requests.map(request =>
request.indexName === 'root' ? {
index: 'root',
hits: [],
nbHits: 0,
nbPages: 0,
page: 0,
processingTimeMS: 0
} : response.results.shift())
} Thank you for your time and support. This worked perfectly. I'm going to do a couple more refinements then start a discussion thread on this. |
Hadn't considered this approach yet, but yes it should work. The order is always the same. |
🐛 Current behavior
File: /node_modules/instantsearch.js/es/lib/utils/getRefinements
Function getRefinements...
results can be null. This function crashes when it is as there is no guard in the function for this possibility. Adding a guard and returning an empty array (as in, no refinements possible) the error goes away.
🔍 Steps to reproduce
I have no clue how my code is causing a null object to be passed to this function. I just know that it is happening and a straightforward hack stops the nonsense.
Live reproduction
No sandbox should be necessary - and I honestly have no clue on how to reproduce the bug in a simplified example.
💭 Expected behavior
Throwing an unguarded error is a bug. The API should either do the above to recover, or throw a detailed error explaining what I can do in my code to prevent a null object from getting passed into this function.
Package version
instantsearch 4.75.4
Operating system
OSX Sequoia
Browser
Edge Latest
Code of Conduct
The text was updated successfully, but these errors were encountered: