From 11399e6fd232f005c4e3e8aba842c50110869ae5 Mon Sep 17 00:00:00 2001 From: Haroen Viaene Date: Tue, 10 Sep 2024 11:30:21 +0200 Subject: [PATCH] fix(client): use client.search in sffv for v5 (#6354) * fix(client): use client.search in sffv for v5 v5 has a `searchForFacetValues` function, but it's single-index only. We thus need to detect it and use `client.search` directly. In a future major we can change this and only use `client.search` * hup lint fixed --- .../src/algoliasearch.helper.js | 5 +- .../algoliasearch.helper/pendingSearch.js | 99 +++++++++++++------ .../searchForFacetValues.js | 20 ++++ .../types/algoliasearch.d.ts | 13 ++- .../__tests__/refinement-list.test.tsx | 1 + 5 files changed, 108 insertions(+), 30 deletions(-) diff --git a/packages/algoliasearch-helper/src/algoliasearch.helper.js b/packages/algoliasearch-helper/src/algoliasearch.helper.js index 81f1c6c5bd..3ed7dc444b 100644 --- a/packages/algoliasearch-helper/src/algoliasearch.helper.js +++ b/packages/algoliasearch-helper/src/algoliasearch.helper.js @@ -371,7 +371,10 @@ AlgoliaSearchHelper.prototype.searchForFacetValues = function ( maxFacetHits, userState ) { - var clientHasSFFV = typeof this.client.searchForFacetValues === 'function'; + var clientHasSFFV = + typeof this.client.searchForFacetValues === 'function' && + // v5 has a wrong sffv signature + typeof this.client.searchForFacets !== 'function'; var clientHasInitIndex = typeof this.client.initIndex === 'function'; if ( !clientHasSFFV && diff --git a/packages/algoliasearch-helper/test/spec/algoliasearch.helper/pendingSearch.js b/packages/algoliasearch-helper/test/spec/algoliasearch.helper/pendingSearch.js index 187fa91d0c..00018aa06a 100644 --- a/packages/algoliasearch-helper/test/spec/algoliasearch.helper/pendingSearch.js +++ b/packages/algoliasearch-helper/test/spec/algoliasearch.helper/pendingSearch.js @@ -1,6 +1,7 @@ 'use strict'; var algoliasearch = require('algoliasearch'); +var isV5 = (algoliasearch.apiClientVersion || '')[0] === '5'; algoliasearch = algoliasearch.algoliasearch || algoliasearch; var algoliasearchHelper = require('../../../index'); @@ -71,43 +72,85 @@ test('When searchOnce with promises, hasPendingRequests is true', function (done triggerCb(); }); -test('When searchForFacetValues, hasPendingRequests is true', function (done) { - var client = algoliasearch('dsf', 'dsfdf'); +if (!isV5) { + test('When searchForFacetValues, hasPendingRequests is true (v3, v4)', function (done) { + var client = algoliasearch('dsf', 'dsfdf'); + + let triggerCb; + client.searchForFacetValues = function () { + return new Promise(function (resolve) { + triggerCb = function () { + resolve([ + { + exhaustiveFacetsCount: true, + facetHits: [], + processingTimeMS: 3, + }, + ]); + }; + }); + }; - var triggerCb; - client.searchForFacetValues = function () { - return new Promise(function (resolve) { - triggerCb = function () { - resolve([ - { - exhaustiveFacetsCount: true, - facetHits: [], - processingTimeMS: 3, - }, - ]); - }; + var helper = algoliasearchHelper(client, 'test_hotels-node'); + var countNoMoreSearch = 0; + helper.on('searchQueueEmpty', function () { + countNoMoreSearch += 1; }); - }; - var helper = algoliasearchHelper(client, 'test_hotels-node'); - var countNoMoreSearch = 0; - helper.on('searchQueueEmpty', function () { - countNoMoreSearch += 1; + expect(helper.hasPendingRequests()).toBe(false); + + helper.searchForFacetValues('').then(function () { + expect(helper.hasPendingRequests()).toBe(false); + expect(countNoMoreSearch).toBe(1); + done(); + }); + + expect(helper.hasPendingRequests()).toBe(true); + expect(countNoMoreSearch).toBe(0); + + triggerCb(); }); +} else { + test('When searchForFacetValues, hasPendingRequests is true (v5)', function (done) { + var client = algoliasearch('dsf', 'dsfdf'); + + let triggerCb; + client.search = function () { + return new Promise(function (resolve) { + triggerCb = function () { + resolve({ + results: [ + { + exhaustiveFacetsCount: true, + facetHits: [], + processingTimeMS: 3, + }, + ], + }); + }; + }); + }; - expect(helper.hasPendingRequests()).toBe(false); + var helper = algoliasearchHelper(client, 'test_hotels-node'); + var countNoMoreSearch = 0; + helper.on('searchQueueEmpty', function () { + countNoMoreSearch += 1; + }); - helper.searchForFacetValues('').then(function () { expect(helper.hasPendingRequests()).toBe(false); - expect(countNoMoreSearch).toBe(1); - done(); - }); - expect(helper.hasPendingRequests()).toBe(true); - expect(countNoMoreSearch).toBe(0); + helper.searchForFacetValues('').then(function () { + expect(helper.hasPendingRequests()).toBe(false); + expect(countNoMoreSearch).toBe(1); + done(); + }); - triggerCb(); -}); + expect(helper.hasPendingRequests()).toBe(true); + expect(countNoMoreSearch).toBe(0); + + triggerCb(); + }); +} test('When helper.search(), hasPendingRequests is true', function (done) { var testData = require('../../datasets/SearchParameters/search.dataset')(); diff --git a/packages/algoliasearch-helper/test/spec/algoliasearch.helper/searchForFacetValues.js b/packages/algoliasearch-helper/test/spec/algoliasearch.helper/searchForFacetValues.js index a49a30c899..6606e6d0e0 100644 --- a/packages/algoliasearch-helper/test/spec/algoliasearch.helper/searchForFacetValues.js +++ b/packages/algoliasearch-helper/test/spec/algoliasearch.helper/searchForFacetValues.js @@ -56,6 +56,26 @@ test('searchForFacetValues calls the index method if no client method', function }); }); +test('searchForFacetValues calls client.search if client.searchForFacets exists', function () { + var clientSearch = jest.fn(function () { + return Promise.resolve({ + results: [makeFakeSearchForFacetValuesResponse()], + }); + }); + + var fakeClient = { + searchForFacets: jest.fn(), + searchForFacetValues: jest.fn(), + search: clientSearch, + }; + + var helper = algoliasearchHelper(fakeClient, 'index'); + + return helper.searchForFacetValues('facet', 'query', 1).then(function () { + expect(clientSearch).toHaveBeenCalledTimes(1); + }); +}); + test('searchForFacetValues resolve with the correct response from client', function () { var fakeClient = { addAlgoliaAgent: function () {}, diff --git a/packages/algoliasearch-helper/types/algoliasearch.d.ts b/packages/algoliasearch-helper/types/algoliasearch.d.ts index 6b215f4348..5f9bbfd085 100644 --- a/packages/algoliasearch-helper/types/algoliasearch.d.ts +++ b/packages/algoliasearch-helper/types/algoliasearch.d.ts @@ -306,6 +306,17 @@ export type SupportedLanguage = PickForClient<{ v5: AlgoliaSearch.SupportedLanguage; }>; +// v5 only has the `searchForFacetValues` method in the `search` client, not in `lite`. +// We need to check both clients to get the correct type. +// (this is not actually used in the codebase, but it's here for completeness) +type SearchForFacetValuesV5 = ClientSearchV5 | ClientFullV5 extends { + searchForFacetValues: unknown; +} + ? + | ClientSearchV5['searchForFacetValues'] + | ClientFullV5['searchForFacetValues'] + : never; + export interface SearchClient { search: ( requests: Array<{ indexName: string; params: SearchOptions }> @@ -317,7 +328,7 @@ export interface SearchClient { searchForFacetValues: unknown; } ? DefaultSearchClient['searchForFacetValues'] - : never; + : SearchForFacetValuesV5; initIndex?: DefaultSearchClient extends { initIndex: unknown } ? DefaultSearchClient['initIndex'] : never; diff --git a/packages/instantsearch.js/src/widgets/refinement-list/__tests__/refinement-list.test.tsx b/packages/instantsearch.js/src/widgets/refinement-list/__tests__/refinement-list.test.tsx index 6580c77891..5c18995abd 100644 --- a/packages/instantsearch.js/src/widgets/refinement-list/__tests__/refinement-list.test.tsx +++ b/packages/instantsearch.js/src/widgets/refinement-list/__tests__/refinement-list.test.tsx @@ -1187,6 +1187,7 @@ function createMockedSearchClient() { return Promise.resolve([ createSFFVResponse({ facetHits: + // @ts-ignore for v5, which has a different definition of `searchForFacetValues` requests[0].params.facetQuery === 'query with no results' ? [] : [