fix(createURL): ensure refined value gets removed

[Haroenv]

Summary

If there's only one refinement left, we used to keep uiState purposely as-is, although actually it needs to be cleared from the refinements.

This also has an impact (positive, removing stray refinements) on dispose with the new disposeMode likely, but to investigate.

This also needs to be done and tested for all other refinement widgets.

We possibly also want to remove the refinementList object if it no longer has values (unless routing already deals with empty objects fine)

This behaviour became obvious after #5696 when routing became based on uiState instead of searchParameters

Result

getWidgetUiState removes no-longer-valid values from uiState, instead of leaving the object as-is if there are no refinements.

Original reason for that code was to avoid recreating the object if nothing has changed, but that's not correct. Not sure if this has a perf impact, but I think it's unavoidable

Sandbox that shows the issue for refinementList: https://codesandbox.io/s/algolia-kickstart-userefinementlist-72kryv?file=/src/index.js

FX-2667

[codesandbox-ci]

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 456cb43:

Sandbox	Source
InstantSearch.js	Configuration
react-instantsearch-app	Configuration
example-react-instantsearch-default-theme	Configuration
example-react-instantsearch-next-app-dir-example	Configuration
example-vue-instantsearch-default-theme	Configuration
Algolia Kickstart - useRefinementList	PR

[Haroenv]

does removing this function break any tests?

[dhayab]

Yes, it adds empty values in widget's ui states, which are not expected by tests, and can in some instance show with the default router. I've added this method in each connector for now, but I'd like to see if I can clean these empty refinements from a single location instead.

[Haroenv]

maybe it can be done in the index widget after getUiState has been called, so you only need to iterate once, instead of for every widget (although getWidgetUiState is called in multiple places of course)

[dhayab]

It wasn't feasible to remove empty attributes at a later point in the index widget, as:

• some parts of the call directly use results from connectors' getWidgetUiState()
• the predicates used to determine wether attributes are considered empty differs between connectors

[aymeric-giraudet]

🎉

[sarahdayan]

Looks correct!

Optional, but we could probably factorize all the removeEmptyRefinementsFromUiState functions into one with a variable key (and maybe some more logic to pass for the reduce) and see if this has an impact on the increased bundle size.

[dhayab]

Looks correct!

Optional, but we could probably factorize all the removeEmptyRefinementsFromUiState functions into one with a variable key (and maybe some more logic to pass for the reduce) and see if this has an impact on the increased bundle size.

Tried it initially but I got so many frustrations with types I went back to this state. Ideally the refactored function would accept the connector indexUiState as a generic type which extends a base type, and a predicate argument that returns whether the value is empty or not. But the type intersections for IndexUiState are difficult to work with.