Skip to content

Commit

Permalink
fix(Configure): prevent aroundRadius="all" throwing (#6193)
Browse files Browse the repository at this point in the history
* fix(Configure): prevent aroundRadius="all" throwing

Because React props are not writable, behaviour inside _parseNumbers > merge in the helper requires the configure props to be writable.

fixes #6136

This bug is reproducible by simply rendering `<Configure aroundRadius="all" />` or any other numeric (https://github.com/algolia/instantsearch/blob/d1aa720c8e4e1aad2d7b64e385a29b258240c7df/packages/algoliasearch-helper/src/SearchParameters/index.js#L239-L251) key as a non-number string.

* chore(bundlesize): update limit

This seems to be needed because the location of the babel helpers needs to change to support this new usage of spread.

I don't see any rhyme or reason to why it actually happens or why that has such a large impact for just moved code, but the babel helpers clearly are already very inefficient (many duplicates of the same helper functions) that this probably is fixed if we take a look later to improve this.
  • Loading branch information
Haroenv committed May 14, 2024
1 parent 6d477df commit cf5a326
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 2 deletions.
2 changes: 1 addition & 1 deletion bundlesize.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
},
{
"path": "packages/react-instantsearch-core/dist/umd/ReactInstantSearchCore.min.js",
"maxSize": "47.5 kB"
"maxSize": "48.5 kB"
},
{
"path": "packages/react-instantsearch/dist/umd/ReactInstantSearch.min.js",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { UseConfigureProps } from '../connectors/useConfigure';
export type ConfigureProps = UseConfigureProps;

export function Configure(props: ConfigureProps) {
useConfigure(props, { $$widgetType: 'ais.configure' });
useConfigure({ ...props }, { $$widgetType: 'ais.configure' });

return null;
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,45 @@ describe('Configure', () => {
]);
});
});

test('does not break for aroundRadius="all"', async () => {
const searchClient = createSearchClient({});

/**
* Inside the helper, aroundRadius is one of the "numeric" parameters. This means
* that it goes through the flow of _parseNumbers when a SearchParameters object is
* created.
*
* The _parseNumbers function is responsible for converting the value of aroundRadius
* to a number. However if it is set to "all", it should not be converted to a number.
*
* Due to this logic, the props get copied to the search parameters object as is. This
* could be problematic if we use the React `props` object directly in the helper, as
* its keys are not writable.
*
* This test exists to ensure that the helper does not break when aroundRadius is set
* to "all".
*
* For the bug report, see: https://github.com/algolia/instantsearch/issues/6136, and
* a relevant part of the code being edited: https://github.com/algolia/instantsearch/pull/6011
*/

render(
<InstantSearch indexName="indexName" searchClient={searchClient}>
<Configure aroundRadius="all" />
</InstantSearch>
);

await waitFor(() => {
expect(searchClient.search).toHaveBeenCalledTimes(1);
expect(searchClient.search).toHaveBeenCalledWith([
{
indexName: 'indexName',
params: expect.objectContaining({
aroundRadius: 'all',
}),
},
]);
});
});
});

0 comments on commit cf5a326

Please sign in to comment.