Closed omikader closed 1 year ago
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 70eaed39cc8297dac7fc8ee312842cc38fb7b713:
Sandbox | Source |
---|---|
javascript-client-app | Configuration |
@Haroenv do you know why we did not made the union in the first place?
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
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
@Haroenv fyi, it did impact us in a way:
We depend on this typing a lot throughout our codebase
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.
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 :)
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`)
}
},
}
Summary
This PR unions
SearchForFacetValuesResponse
withSearchResponse<TObject>
as possible return types for results in themultipleQueries
API.Context
The
MultipleQueriesQuery
interface states that you can supply either adefault
or afacet
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).