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

fix(toggle): ensure facet is present #1613

Merged
merged 1 commit into from
Sep 26, 2018
Merged

fix(toggle): ensure facet is present #1613

merged 1 commit into from
Sep 26, 2018

Conversation

samouss
Copy link
Collaborator

@samouss samouss commented Sep 25, 2018

Summary

This PR fixes an issue when we tried to access the facet values on a SearchResults that don't have the expected facet. We can ensure this with getFacetByName - this method is deprecated on the helper but we don't have a proper alternative yet.

Before

before

After

after

@samouss samouss requested a review from a team September 25, 2018 16:52
@samouss samouss mentioned this pull request Sep 25, 2018
Merged
@algobot
Copy link
Contributor

algobot commented Sep 25, 2018 •
edited
Loading

Deploy preview for react-instantsearch ready!

Built with commit c3b36dd

https://deploy-preview-1613--react-instantsearch.netlify.com

Haroenv
Haroenv approved these changes Sep 26, 2018
View reviewed changes
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.

Would be nice if getfacetvalues would be null or undefined, not throw an error

@samouss
Copy link
Collaborator Author

samouss commented Sep 26, 2018

Yep indeed, or throw only in development. It can be handy to have this information when you develop.

@samouss samouss force-pushed the fix/connect-toggle-refinement branch from 26f5134 to c3b36dd Compare September 26, 2018 08:29
@samouss samouss merged commit e914ff6 into master Sep 26, 2018
@samouss samouss deleted the fix/connect-toggle-refinement branch September 26, 2018 08:39
@Haroenv
Copy link
Contributor

Haroenv commented Sep 26, 2018

Let's find a time later to put a place for these kind of DX errors, but they probably live best in the helper, since it's a shared dependency.

@samouss
Copy link
Collaborator Author

samouss commented Sep 26, 2018

Yes for sure the error about the facet is thrown inside the helper actually.

samouss added a commit that referenced this pull request Sep 26, 2018
@samouss
v5.3.1
13b3526 
<a name="5.3.1"></a>
## [5.3.1](v5.3.0...v5.3.1) (2018-09-26)

### Bug Fixes

* **connector:** ensure canRefine is computed on the transformed items ([#1568](#1568)) ([c95384f](c95384f))
* **toggle:** ensure facet is present ([#1613](#1613)) ([e914ff6](e914ff6))
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Haroenv Haroenv Haroenv approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@samouss @algobot @Haroenv