From f32ae15f90433e110a4f5661e57e8bab1e268b03 Mon Sep 17 00:00:00 2001
From: Haroen Viaene <hello@haroen.me>
Date: Fri, 6 Dec 2024 15:07:04 +0100
Subject: [PATCH] fix(searchForFacetValues): only call client.search

This signature is the easiest to detect and makes the required signature of the searchClient simpler (only `search`, `getRecommendations` and `addAlgoliaAgent` are now used).

In this PR I also fix the signature of `search` in `SearchClient` type to make this usage fit reality
---
 .../src/algoliasearch.helper.js               |  58 +--
 .../src/requestBuilder.js                     |   1 -
 .../test/spec/algoliasearch.helper/events.js  |   7 +-
 .../searchForFacetValues.js                   | 416 +++++-------------
 .../types/algoliasearch.d.ts                  |  36 +-
 5 files changed, 140 insertions(+), 378 deletions(-)

diff --git a/packages/algoliasearch-helper/src/algoliasearch.helper.js b/packages/algoliasearch-helper/src/algoliasearch.helper.js
index 9adb88fc89..a782ec57ff 100644
--- a/packages/algoliasearch-helper/src/algoliasearch.helper.js
+++ b/packages/algoliasearch-helper/src/algoliasearch.helper.js
@@ -322,21 +322,6 @@ AlgoliaSearchHelper.prototype.searchForFacetValues = function (
   maxFacetHits,
   userState
 ) {
-  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 &&
-    !clientHasInitIndex &&
-    typeof this.client.search !== 'function'
-  ) {
-    throw new Error(
-      'search for facet values (searchable) was called, but this client does not have a function client.searchForFacetValues or client.initIndex(index).searchForFacetValues'
-    );
-  }
-
   var state = this.state.setQueryParameters(userState || {});
   var isDisjunctive = state.isDisjunctiveFacet(facet);
   var algoliaQuery = requestBuilder.getSearchForFacetQuery(
@@ -349,34 +334,15 @@ AlgoliaSearchHelper.prototype.searchForFacetValues = function (
   this._currentNbQueries++;
   // eslint-disable-next-line consistent-this
   var self = this;
-  var searchForFacetValuesPromise;
-  // newer algoliasearch ^3.27.1 - ~4.0.0
-  if (clientHasSFFV) {
-    searchForFacetValuesPromise = this.client.searchForFacetValues([
-      { indexName: state.index, params: algoliaQuery },
-    ]);
-    // algoliasearch < 3.27.1
-  } else if (clientHasInitIndex) {
-    searchForFacetValuesPromise = this.client
-      .initIndex(state.index)
-      .searchForFacetValues(algoliaQuery);
-    // algoliasearch ~5.0.0
-  } else {
-    // @MAJOR only use client.search
-    delete algoliaQuery.facetName;
-    searchForFacetValuesPromise = this.client
-      .search([
-        {
-          type: 'facet',
-          facet: facet,
-          indexName: state.index,
-          params: algoliaQuery,
-        },
-      ])
-      .then(function processResponse(response) {
-        return response.results[0];
-      });
-  }
+
+  var searchForFacetValuesPromise = this.client.search([
+    {
+      type: 'facet',
+      facet: facet,
+      indexName: state.index,
+      params: algoliaQuery,
+    },
+  ]);
 
   this.emit('searchForFacetValues', {
     state: state,
@@ -389,16 +355,16 @@ AlgoliaSearchHelper.prototype.searchForFacetValues = function (
       self._currentNbQueries--;
       if (self._currentNbQueries === 0) self.emit('searchQueueEmpty');
 
-      content = Array.isArray(content) ? content[0] : content;
+      var result = content.results[0];
 
-      content.facetHits.forEach(function (f) {
+      result.facetHits.forEach(function (f) {
         f.escapedValue = escapeFacetValue(f.value);
         f.isRefined = isDisjunctive
           ? state.isDisjunctiveFacetRefined(facet, f.escapedValue)
           : state.isFacetRefined(facet, f.escapedValue);
       });
 
-      return content;
+      return result;
     },
     function (e) {
       self._currentNbQueries--;
diff --git a/packages/algoliasearch-helper/src/requestBuilder.js b/packages/algoliasearch-helper/src/requestBuilder.js
index ba28640454..7938804f47 100644
--- a/packages/algoliasearch-helper/src/requestBuilder.js
+++ b/packages/algoliasearch-helper/src/requestBuilder.js
@@ -434,7 +434,6 @@ var requestBuilder = {
       : state;
     var searchForFacetSearchParameters = {
       facetQuery: query,
-      facetName: facetName,
     };
     if (typeof maxFacetHits === 'number') {
       searchForFacetSearchParameters.maxFacetHits = maxFacetHits;
diff --git a/packages/algoliasearch-helper/test/spec/algoliasearch.helper/events.js b/packages/algoliasearch-helper/test/spec/algoliasearch.helper/events.js
index db8eada81a..8712e0ae56 100644
--- a/packages/algoliasearch-helper/test/spec/algoliasearch.helper/events.js
+++ b/packages/algoliasearch-helper/test/spec/algoliasearch.helper/events.js
@@ -7,9 +7,6 @@ function makeFakeClient() {
     search: jest.fn(function () {
       return new Promise(function () {});
     }),
-    searchForFacetValues: jest.fn(function () {
-      return new Promise(function () {});
-    }),
   };
 }
 
@@ -297,7 +294,7 @@ test(
     helper.on('searchForFacetValues', searchedForFacetValues);
 
     expect(searchedForFacetValues).toHaveBeenCalledTimes(0);
-    expect(fakeClient.searchForFacetValues).toHaveBeenCalledTimes(0);
+    expect(fakeClient.search).toHaveBeenCalledTimes(0);
 
     helper.searchForFacetValues('city', 'NYC');
     expect(searchedForFacetValues).toHaveBeenCalledTimes(1);
@@ -306,7 +303,7 @@ test(
       facet: 'city',
       query: 'NYC',
     });
-    expect(fakeClient.searchForFacetValues).toHaveBeenCalledTimes(1);
+    expect(fakeClient.search).toHaveBeenCalledTimes(1);
   }
 );
 
diff --git a/packages/algoliasearch-helper/test/spec/algoliasearch.helper/searchForFacetValues.js b/packages/algoliasearch-helper/test/spec/algoliasearch.helper/searchForFacetValues.js
index 6606e6d0e0..7e4d1b3e07 100644
--- a/packages/algoliasearch-helper/test/spec/algoliasearch.helper/searchForFacetValues.js
+++ b/packages/algoliasearch-helper/test/spec/algoliasearch.helper/searchForFacetValues.js
@@ -10,77 +10,36 @@ function makeFakeSearchForFacetValuesResponse() {
   };
 }
 
-test('searchForFacetValues calls the client method over the index method', function () {
-  var clientSearchForFacetValues = jest.fn(function () {
-    return Promise.resolve([makeFakeSearchForFacetValuesResponse()]);
-  });
-
-  var indexSearchForFacetValues = jest.fn(function () {
-    return Promise.resolve(makeFakeSearchForFacetValuesResponse());
-  });
-
-  var fakeClient = {
-    searchForFacetValues: clientSearchForFacetValues,
-    initIndex: function () {
-      return {
-        searchForFacetValues: indexSearchForFacetValues,
-      };
-    },
-  };
-
-  var helper = algoliasearchHelper(fakeClient, 'index');
-
-  return helper.searchForFacetValues('facet', 'query', 1).then(function () {
-    expect(clientSearchForFacetValues).toHaveBeenCalledTimes(1);
-    expect(indexSearchForFacetValues).toHaveBeenCalledTimes(0);
-  });
-});
-
-test('searchForFacetValues calls the index method if no client method', function () {
-  var indexSearchForFacetValues = jest.fn(function () {
-    return Promise.resolve(makeFakeSearchForFacetValuesResponse());
-  });
-
+test('searchForFacetValues calls client.search if client.searchForFacets exists', function () {
   var fakeClient = {
     initIndex: function () {
       return {
-        searchForFacetValues: indexSearchForFacetValues,
+        searchForFacetValues: jest.fn(function () {}),
       };
     },
-  };
-
-  var helper = algoliasearchHelper(fakeClient, 'index');
-
-  return helper.searchForFacetValues('facet', 'query', 1).then(function () {
-    expect(indexSearchForFacetValues).toHaveBeenCalledTimes(1);
-  });
-});
-
-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,
+    search: jest.fn(function () {
+      return Promise.resolve({
+        results: [makeFakeSearchForFacetValuesResponse()],
+      });
+    }),
   };
 
   var helper = algoliasearchHelper(fakeClient, 'index');
 
   return helper.searchForFacetValues('facet', 'query', 1).then(function () {
-    expect(clientSearch).toHaveBeenCalledTimes(1);
+    expect(fakeClient.search).toHaveBeenCalledTimes(1);
   });
 });
 
 test('searchForFacetValues resolve with the correct response from client', function () {
   var fakeClient = {
     addAlgoliaAgent: function () {},
-    searchForFacetValues: function () {
-      return Promise.resolve([makeFakeSearchForFacetValuesResponse()]);
+    search: function () {
+      return Promise.resolve({
+        results: [makeFakeSearchForFacetValuesResponse()],
+      });
     },
   };
 
@@ -95,158 +54,14 @@ test('searchForFacetValues resolve with the correct response from client', funct
     });
 });
 
-test('searchForFacetValues resolve with the correct response from initIndex', function () {
+test('searchForFacetValues should search for facetValues with the current state', function () {
   var fakeClient = {
     addAlgoliaAgent: function () {},
-    initIndex: function () {
-      return {
-        searchForFacetValues: function () {
-          return Promise.resolve(makeFakeSearchForFacetValuesResponse());
-        },
-      };
-    },
-  };
-
-  var helper = algoliasearchHelper(fakeClient, 'index');
-
-  return helper
-    .searchForFacetValues('facet', 'query', 1)
-    .then(function (content) {
-      expect(content.exhaustiveFacetsCount).toBe(true);
-      expect(content.facetHits.length).toBe(0);
-      expect(content.processingTimeMS).toBe(3);
-    });
-});
-
-test('index.searchForFacetValues should search for facetValues with the current state', function () {
-  var indexSearchForFacetValues = jest.fn(function () {
-    return Promise.resolve(makeFakeSearchForFacetValuesResponse());
-  });
-
-  var fakeClient = {
-    initIndex: function () {
-      return {
-        searchForFacetValues: indexSearchForFacetValues,
-      };
-    },
-  };
-
-  var helper = algoliasearchHelper(fakeClient, 'index', {
-    highlightPreTag: 'HIGHLIGHT>',
-    highlightPostTag: '<HIGHLIGHT',
-    query: 'iphone',
-  });
-
-  helper.searchForFacetValues('facet', 'query', 75);
-
-  var lastArguments = indexSearchForFacetValues.mock.calls[0][0];
-
-  expect(lastArguments.query).toBe('iphone');
-  expect(lastArguments.facetQuery).toBe('query');
-  expect(lastArguments.facetName).toBe('facet');
-  expect(lastArguments.highlightPreTag).toBe('HIGHLIGHT>');
-  expect(lastArguments.highlightPostTag).toBe('<HIGHLIGHT');
-});
-
-test('index.searchForFacetValues can override the current search state', function () {
-  var indexSearchForFacetValues = jest.fn(function () {
-    return Promise.resolve(makeFakeSearchForFacetValuesResponse());
-  });
-
-  var fakeClient = {
-    initIndex: function () {
-      return {
-        searchForFacetValues: indexSearchForFacetValues,
-      };
-    },
-  };
-
-  var helper = algoliasearchHelper(fakeClient, 'index', {
-    highlightPreTag: 'HIGHLIGHT>',
-    highlightPostTag: '<HIGHLIGHT',
-    query: 'iphone',
-  });
-
-  helper.searchForFacetValues('facet', 'query', 75, {
-    query: undefined,
-    highlightPreTag: 'highlightTag',
-  });
-
-  var lastArguments = indexSearchForFacetValues.mock.calls[0][0];
-
-  expect(lastArguments.hasOwnProperty('query')).toBeFalsy();
-  expect(lastArguments.facetQuery).toBe('query');
-  expect(lastArguments.facetName).toBe('facet');
-  expect(lastArguments.highlightPreTag).toBe('highlightTag');
-  expect(lastArguments.highlightPostTag).toBe('<HIGHLIGHT');
-});
-
-test('client.searchForFacetValues should search for facetValues with the current state', function () {
-  var clientSearchForFacetValues = jest.fn(function () {
-    return Promise.resolve([makeFakeSearchForFacetValuesResponse()]);
-  });
-
-  var fakeClient = {
-    searchForFacetValues: clientSearchForFacetValues,
-  };
-
-  var helper = algoliasearchHelper(fakeClient, 'index', {
-    highlightPreTag: 'HIGHLIGHT>',
-    highlightPostTag: '<HIGHLIGHT',
-    query: 'iphone',
-  });
-
-  helper.searchForFacetValues('facet', 'query', 75);
-
-  var lastArguments = clientSearchForFacetValues.mock.calls[0][0][0];
-
-  expect(lastArguments.indexName).toBe('index');
-  expect(lastArguments.params.query).toBe('iphone');
-  expect(lastArguments.params.facetQuery).toBe('query');
-  expect(lastArguments.params.facetName).toBe('facet');
-  expect(lastArguments.params.highlightPreTag).toBe('HIGHLIGHT>');
-  expect(lastArguments.params.highlightPostTag).toBe('<HIGHLIGHT');
-});
-
-test('client.searchForFacetValues can override the current search state', function () {
-  var clientSearchForFacetValues = jest.fn(function () {
-    return Promise.resolve([makeFakeSearchForFacetValuesResponse()]);
-  });
-
-  var fakeClient = {
-    searchForFacetValues: clientSearchForFacetValues,
-  };
-
-  var helper = algoliasearchHelper(fakeClient, 'index', {
-    highlightPreTag: 'HIGHLIGHT>',
-    highlightPostTag: '<HIGHLIGHT',
-    query: 'iphone',
-  });
-
-  helper.searchForFacetValues('facet', 'query', 75, {
-    query: undefined,
-    highlightPreTag: 'highlightTag',
-  });
-
-  var lastArguments = clientSearchForFacetValues.mock.calls[0][0][0];
-
-  expect(lastArguments.indexName).toBe('index');
-  expect(lastArguments.params.hasOwnProperty('query')).toBeFalsy();
-  expect(lastArguments.params.facetQuery).toBe('query');
-  expect(lastArguments.params.facetName).toBe('facet');
-  expect(lastArguments.params.highlightPreTag).toBe('highlightTag');
-  expect(lastArguments.params.highlightPostTag).toBe('<HIGHLIGHT');
-});
-
-test('client.search should search for facetValues with the current state', function () {
-  var clientSearch = jest.fn(function () {
-    return Promise.resolve({
-      results: [makeFakeSearchForFacetValuesResponse()],
-    });
-  });
-
-  var fakeClient = {
-    search: clientSearch,
+    search: jest.fn(function () {
+      return Promise.resolve({
+        results: [makeFakeSearchForFacetValuesResponse()],
+      });
+    }),
   };
 
   var helper = algoliasearchHelper(fakeClient, 'index', {
@@ -257,26 +72,22 @@ test('client.search should search for facetValues with the current state', funct
 
   helper.searchForFacetValues('facet', 'query', 75);
 
-  var lastArguments = clientSearch.mock.calls[0][0][0];
+  var lastArguments = fakeClient.search.mock.calls[0][0][0];
 
-  expect(lastArguments.indexName).toBe('index');
   expect(lastArguments.params.query).toBe('iphone');
   expect(lastArguments.params.facetQuery).toBe('query');
-  expect(lastArguments.params.hasOwnProperty('facetName')).toBeFalsy();
   expect(lastArguments.facet).toBe('facet');
   expect(lastArguments.params.highlightPreTag).toBe('HIGHLIGHT>');
   expect(lastArguments.params.highlightPostTag).toBe('<HIGHLIGHT');
 });
 
-test('client.search can override the current search state', function () {
-  var clientSearch = jest.fn(function () {
-    return Promise.resolve({
-      results: [makeFakeSearchForFacetValuesResponse()],
-    });
-  });
-
+test('searchForFacetValues can override the current search state', function () {
   var fakeClient = {
-    search: clientSearch,
+    search: jest.fn(function () {
+      return Promise.resolve({
+        results: [makeFakeSearchForFacetValuesResponse()],
+      });
+    }),
   };
 
   var helper = algoliasearchHelper(fakeClient, 'index', {
@@ -290,52 +101,39 @@ test('client.search can override the current search state', function () {
     highlightPreTag: 'highlightTag',
   });
 
-  var lastArguments = clientSearch.mock.calls[0][0][0];
+  var lastArguments = fakeClient.search.mock.calls[0][0][0];
 
-  expect(lastArguments.indexName).toBe('index');
   expect(lastArguments.params.hasOwnProperty('query')).toBeFalsy();
   expect(lastArguments.params.facetQuery).toBe('query');
-  expect(lastArguments.params.hasOwnProperty('facetName')).toBeFalsy();
   expect(lastArguments.facet).toBe('facet');
   expect(lastArguments.params.highlightPreTag).toBe('highlightTag');
   expect(lastArguments.params.highlightPostTag).toBe('<HIGHLIGHT');
 });
 
-test('an error will be thrown if the client does not contain .searchForFacetValues', function () {
-  var fakeClient = {};
-  var helper = algoliasearchHelper(fakeClient, 'index');
-
-  try {
-    helper.searchForFacetValues('facet', 'query');
-  } catch (e) {
-    expect(e.message).toBe(
-      'search for facet values (searchable) was called, but this client does not have a function client.searchForFacetValues or client.initIndex(index).searchForFacetValues'
-    );
-  }
-});
-
 test('isRefined is set for disjunctive facets', function () {
   var fakeClient = {
     addAlgoliaAgent: function () {},
-    searchForFacetValues: function () {
-      return Promise.resolve([
-        {
-          exhaustiveFacetsCount: true,
-          facetHits: [
-            {
-              count: 318,
-              highlighted: '__ais-highlight__K__/ais-highlight__itchenAid',
-              value: 'KitchenAid',
-            },
-            {
-              count: 1,
-              highlighted: 'something',
-              value: 'something',
-            },
-          ],
-          processingTimeMS: 3,
-        },
-      ]);
+    search: function () {
+      return Promise.resolve({
+        results: [
+          {
+            exhaustiveFacetsCount: true,
+            facetHits: [
+              {
+                count: 318,
+                highlighted: '__ais-highlight__K__/ais-highlight__itchenAid',
+                value: 'KitchenAid',
+              },
+              {
+                count: 1,
+                highlighted: 'something',
+                value: 'something',
+              },
+            ],
+            processingTimeMS: 3,
+          },
+        ],
+      });
     },
   };
 
@@ -360,25 +158,27 @@ test('isRefined is set for disjunctive facets', function () {
 test('isRefined is set for conjunctive facets', function () {
   var fakeClient = {
     addAlgoliaAgent: function () {},
-    searchForFacetValues: function () {
-      return Promise.resolve([
-        {
-          exhaustiveFacetsCount: true,
-          facetHits: [
-            {
-              count: 318,
-              highlighted: '__ais-highlight__K__/ais-highlight__itchenAid',
-              value: 'KitchenAid',
-            },
-            {
-              count: 1,
-              highlighted: 'something',
-              value: 'something',
-            },
-          ],
-          processingTimeMS: 3,
-        },
-      ]);
+    search: function () {
+      return Promise.resolve({
+        results: [
+          {
+            exhaustiveFacetsCount: true,
+            facetHits: [
+              {
+                count: 318,
+                highlighted: '__ais-highlight__K__/ais-highlight__itchenAid',
+                value: 'KitchenAid',
+              },
+              {
+                count: 1,
+                highlighted: 'something',
+                value: 'something',
+              },
+            ],
+            processingTimeMS: 3,
+          },
+        ],
+      });
     },
   };
 
@@ -403,25 +203,27 @@ test('isRefined is set for conjunctive facets', function () {
 test('value is escaped when it starts with `-`', function () {
   var fakeClient = {
     addAlgoliaAgent: function () {},
-    searchForFacetValues: function () {
-      return Promise.resolve([
-        {
-          exhaustiveFacetsCount: true,
-          facetHits: [
-            {
-              count: 318,
-              highlighted: 'something',
-              value: 'something',
-            },
-            {
-              count: 1,
-              highlighted: '-20%',
-              value: '-20%',
-            },
-          ],
-          processingTimeMS: 3,
-        },
-      ]);
+    search: function () {
+      return Promise.resolve({
+        results: [
+          {
+            exhaustiveFacetsCount: true,
+            facetHits: [
+              {
+                count: 318,
+                highlighted: 'something',
+                value: 'something',
+              },
+              {
+                count: 1,
+                highlighted: '-20%',
+                value: '-20%',
+              },
+            ],
+            processingTimeMS: 3,
+          },
+        ],
+      });
     },
   };
 
@@ -441,25 +243,27 @@ test('value is escaped when it starts with `-`', function () {
 test('escaped value is marked as refined', function () {
   var fakeClient = {
     addAlgoliaAgent: function () {},
-    searchForFacetValues: function () {
-      return Promise.resolve([
-        {
-          exhaustiveFacetsCount: true,
-          facetHits: [
-            {
-              count: 318,
-              highlighted: 'something',
-              value: 'something',
-            },
-            {
-              count: 1,
-              highlighted: '-20%',
-              value: '-20%',
-            },
-          ],
-          processingTimeMS: 3,
-        },
-      ]);
+    search: function () {
+      return Promise.resolve({
+        results: [
+          {
+            exhaustiveFacetsCount: true,
+            facetHits: [
+              {
+                count: 318,
+                highlighted: 'something',
+                value: 'something',
+              },
+              {
+                count: 1,
+                highlighted: '-20%',
+                value: '-20%',
+              },
+            ],
+            processingTimeMS: 3,
+          },
+        ],
+      });
     },
   };
 
diff --git a/packages/algoliasearch-helper/types/algoliasearch.d.ts b/packages/algoliasearch-helper/types/algoliasearch.d.ts
index 7301f59fbe..5173c05da9 100644
--- a/packages/algoliasearch-helper/types/algoliasearch.d.ts
+++ b/packages/algoliasearch-helper/types/algoliasearch.d.ts
@@ -287,31 +287,27 @@ 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: <T>(
-    requests: Array<{ indexName: string; params: SearchOptions }>
+    requests: Array<
+      | {
+          type?: 'default';
+          indexName: string;
+          params: SearchOptions;
+        }
+      | {
+          type: 'facet';
+          indexName: string;
+          facet: string;
+          params: SearchOptions & {
+            maxFacetHits?: number;
+            facetQuery?: string;
+          };
+        }
+    >
   ) => Promise<SearchResponses<T>>;
   getRecommendations?: <T>(
     requests: RecommendOptions[]
   ) => Promise<RecommendResponses<T>>;
-  searchForFacetValues?: DefaultSearchClient extends {
-    searchForFacetValues: unknown;
-  }
-    ? DefaultSearchClient['searchForFacetValues']
-    : SearchForFacetValuesV5;
-  initIndex?: DefaultSearchClient extends { initIndex: unknown }
-    ? DefaultSearchClient['initIndex']
-    : never;
   addAlgoliaAgent?: DefaultSearchClient['addAlgoliaAgent'];
 }