fix(types): union SearchForFacetValuesResponse type for multipleQueries

[omikader]

Summary

This PR unions SearchForFacetValuesResponse with SearchResponse<TObject> as possible return types for results in the multipleQueries API.

Context

The MultipleQueriesQuery interface states that you can supply either a default or a facet query. This is also confirmed by the Algolia documentation.

However, the MultipleQueriesResponse definition does not include SearchForFacetValuesResponse as a possible return type. This is confusing because the response data from Algolia does in fact include facet responses when requested (see attached image).

[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 70eaed3:

Sandbox	Source
javascript-client-app	Configuration

[shortcuts]

@Haroenv do you know why we did not made the union in the first place?

[Haroenv]

I don't know the history of this, but I believe initially type didn't exist. We'd need to check what the impact on the types of eg InstantSearch for example is though, as I believe almost everything assumes the type is. default

[Haroenv]

While I feared this was a breaking change, as only MultipleQueriesResponse is changed, this actually does not impact any code used by customers today (only the helper's internals, but that isn't typescript). I'm thus happy with the change after all. Sorry for the delay!

[shortcuts]

While I feared this was a breaking change, as only MultipleQueriesResponse is changed, this actually does not impact any code used by customers today (only the helper's internals, but that isn't typescript). I'm thus happy with the change after all. Sorry for the delay!

good to know! thanks for double checking

[chrisneven]

@Haroenv fyi, it did impact us in a way:

We depend on this typing a lot throughout our codebase

[anjakunkel]

Indeed, the type change has breaking impact, also on us.

But it can be fixed easily by explicitly typing your searchResult to be either SearchForFacetValuesResponse with SearchResponse<TObject>, depending on your input.

[VictorGosse]

Hello,

We are indeed facing the same typing issue after the upgrade to 4.18.0. As suggested above we can explicitly type the searchResult (I guess via a as ?).

Couldn't it be possible to improve the typing here by allowing the type to be sent as a generic so that way it could be defined directly when calling multipleQueries and would be type safe.
eg: client.multipleQueries<SearchResponse<TObject>>(...) would force the return type to be SearchResponse<TObject>.
As there is already a generic on multipleQueries maybe it could be enough to use it: if TObject is defined then it'll return SearchResponse<TObject> otherwise it could be both.

Or even better maybe the return type could be inferred based on the data sent in multipleQueries 🤷‍♂️

Thanks :)

[ranman]

This also broke our search.

How can we import the relevant types from the algoliasearch namespace or do we need to add the @algolia/* packages into the imports?

export const Algolia = {
  async search<T = Record<string, string>[]>(
    query: string,
    params: Record<string, string | number> = {}
  ): Promise<{ hits: T; totalPages: number }> {
    try {
      const queries = BUILDER_MODELS.map((model) => {
        return {
          indexName: model,
          query,
          params: {
            ...params,
            hitsPerPage: HITS_PER_PAGE,
          },
        }
      })

      const res = await client.multipleQueries(queries)

      // Retrieve hits out of results and return
      // @ts-expect-error
      const hits = res?.results?.flatMap((result) => result.hits)
      // @ts-expect-error
      const pages = Math.max(...res.results.map((result) => result.nbPages))
      return { hits: hits as T, totalPages: pages - 1 }
    } catch (e) {
      throw new Error(`Failed to load search data`)
    }
  },
}