RangeInput fails to refine search when either min or max is empty

[dotboris]

Bug 🐞

What is the current behavior?

Here are the steps to reproduce:

1. Open https://vue-instantsearch.netlify.app/stories/?selectedKind=ais-range-input&selectedStory=default&full=0&addons=1&stories=1&panelRight=1&addonPanel=storybooks%2Fstorybook-addon-knobs
2. Put 20 in the max textbox
3. Hit apply

Note that the results are not updated.

Make a sandbox with the current behavior

I have managed to reproduce this on the sandbox from the documentation here: https://vue-instantsearch.netlify.app/stories/?selectedKind=ais-range-input&selectedStory=default&full=0&addons=1&stories=1&panelRight=1&addonPanel=storybooks%2Fstorybook-addon-knobs

What is the expected behavior?

The results should get updated

Does this happen only in specific situations?

This is pretty hard to reproduce. Sometimes it works and sometimes it doesn't. What I find to be consistent is testing this right after the component loads.

I've done some digging and this happens only when this.values.min or this.values.max of RangeInput is set to null. This is the case when the component is first loaded but it can happen in other cases. The possible cases for those two values are in the following code

vue-instantsearch/src/components/RangeInput.vue

Lines 120 to 128 in 9bb117c

	values() {
	const [minValue, maxValue] = this.state.start;
	const { min: minRange, max: maxRange } = this.state.range;
	return {
	min: minValue !== -Infinity && minValue !== minRange ? minValue : null,
	max: maxValue !== Infinity && maxValue !== maxRange ? maxValue : null,
	};
	},

What is the proposed solution?

The solution is really a question of architecture. There are two libs at play here (instantsearch.js and vue-instantsearch) and either of them could be changed to fix this.

Fixing instantsearch.js

instantsearch.js does not fully handle the "empty" values in connectRange. Specifically, it handles undefined and '' but not null. This can be seen here: https://github.com/algolia/instantsearch.js/blob/5209bdb9189e7cbbf9514b62fde55f923b2b3273/src/connectors/range/connectRange.js#L123-L127

When either nextMin or nextMax is set to undefined or '', the code thinks that the user is trying to reset either the upper or lower bound of the range. This is the right behavior and works as expected.

When either is set to null the code does not think that the user is trying to reset part of the filter. Instead, it tries to set the value to null. Because the code supports strings as arguments, it ends up trying to parse null as a number which leads to NaN. That NaN value ends up failing one of the validity checks in the code and the refine function does nothing.

One solution to this is to change the refine function in instantsearch.js's connectRange so that it considers null as a possible empty value treating it like undefined or ''.

Fixing vue-instantsearch

vue-instantsearch does not respect the contract established by connectRange. From what I've described above, connectRange does not handle null. It only handles undefined and '' as empty values.

The fix would be to change this.values in RangeInput to fallback on undefined if it can't find a proper value for either min or max. The code in question is here:

vue-instantsearch/src/components/RangeInput.vue

Lines 120 to 128 in 9bb117c

	values() {
	const [minValue, maxValue] = this.state.start;
	const { min: minRange, max: maxRange } = this.state.range;
	return {
	min: minValue !== -Infinity && minValue !== minRange ? minValue : null,
	max: maxValue !== Infinity && maxValue !== maxRange ? maxValue : null,
	};
	},

The fix would be to replace those nulls with undefineds.

What is the version you are using?

I'm running vue-instantsearch@v3.1.0. The full list of algolia packages that I'm using with their matching version are:

$  yarn list --pattern 'algolia|instantsearch'
yarn list v1.22.4
├─ @algolia/cache-browser-local-storage@4.4.0
├─ @algolia/cache-common@4.4.0
├─ @algolia/cache-in-memory@4.4.0
├─ @algolia/client-account@4.4.0
├─ @algolia/client-analytics@4.4.0
├─ @algolia/client-common@4.4.0
├─ @algolia/client-recommendation@4.4.0
├─ @algolia/client-search@4.4.0
├─ @algolia/logger-common@4.4.0
├─ @algolia/logger-console@4.4.0
├─ @algolia/requester-browser-xhr@4.4.0
├─ @algolia/requester-common@4.4.0
├─ @algolia/requester-node-http@4.4.0
├─ @algolia/transporter@4.4.0
├─ algoliasearch-helper@3.2.2
├─ algoliasearch@4.4.0
├─ instantsearch.js@4.7.0
└─ vue-instantsearch@3.1.0
✨  Done in 0.99s.

[yannickcr]

Thanks for your very precise report 👍 , we'll going to push a fix ASAP.