Skip to content

Commit fbe783b

Browse files
Merge #1256
1256: Allow query in search options r=bidoubiwa a=bidoubiwa Fixes: #1198 closes: #1212 Currently when using the search method: ```ts index('xxx').search(query: string, options: SearchParams = {}, config?: Partial<Request>) ``` the `options` parameter complying to the SearchParams type does not accept the `q` parameter in typescript. Which has made a few user having to make a work-around annoying: ```js const query = params.q delete params.q await index.client(query, params) ``` Initially having the `query` as a first parameter has been chosen to make a search without options more simple ([algolia does the same](https://www.algolia.com/doc/api-reference/api-methods/search/#examples)). Nonetheless, the most common use case would use the second `options` parameter. In `not typescript` the user can add `q` to the searchParams. The `q` is overwritten by the first search parameter. We have three choices to consider: ### 1. Remove the query parameter ```ts index('xxx').search(options: SearchParams = {}, config?: Partial<Request>) ``` example: ```js index.search({ q: "test" }) ``` ### 2. Allow query in options but does not overwrite the first params **current behavior** the first parameter not be overwritten by the searchParams: example: ```js index.search("hello", { q: "world" }) // query requested will be `hello` ``` This means that if a user wants to use the option object, they have to still specify the query as the first parameter to avoid a `null` or `undedined` to overwrite the `q` in the options object: ```js const options = { q = "hello" } index.search(options.q, options) // query is hello index.search(null, options) // query is null ``` ### 3. Allow query in options and overwrites first params the first parameter would be overwritten by the searchParams: example: ```js index.search("hello", { q: "world" }) // query requested will be `world` ``` This one lets the user ignore the first parameter ```js index.search(null, { q: "world" }) // query requested will be `world` ``` Co-authored-by: Charlotte Vermandel <[email protected]>
2 parents 902002c + 20a9acd commit fbe783b

File tree

4 files changed

+38
-4
lines changed

4 files changed

+38
-4
lines changed

src/indexes.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ class Index<T = Record<string, any>> {
8383

8484
return await this.httpRequest.post(
8585
url,
86-
removeUndefinedFromObject({ ...options, q: query }),
86+
removeUndefinedFromObject({ q: query, ...options }),
8787
undefined,
8888
config
8989
)

src/types/types.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ export type Crop = {
6666
cropMarker?: string
6767
}
6868

69-
export type SearchParams = Pagination &
69+
export type SearchParams = Query &
70+
Pagination &
7071
Highlight &
7172
Crop & {
7273
filter?: Filter
@@ -76,8 +77,6 @@ export type SearchParams = Pagination &
7677
matches?: boolean
7778
}
7879

79-
export type SearchRequest = SearchParams & Query
80-
8180
// Search parameters for searches made with the GET method
8281
// Are different than the parameters for the POST method
8382
export type SearchRequestGET = Pagination &

tests/search.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,32 @@ describe.each([
9393
expect(response.hits.length).toEqual(2)
9494
})
9595

96+
test(`${permission} key: Search with query in searchParams overwriting query`, async () => {
97+
const client = await getClient(permission)
98+
const response = await client
99+
.index(index.uid)
100+
.search('other', { q: 'prince' })
101+
102+
expect(response).toHaveProperty('hits', expect.any(Array))
103+
expect(response).toHaveProperty('offset', 0)
104+
expect(response).toHaveProperty('limit', 20)
105+
expect(response).toHaveProperty('processingTimeMs', expect.any(Number))
106+
expect(response).toHaveProperty('query', 'prince')
107+
expect(response.hits.length).toEqual(2)
108+
})
109+
110+
test(`${permission} key: Search with query in searchParams overwriting null query`, async () => {
111+
const client = await getClient(permission)
112+
const response = await client.index(index.uid).search(null, { q: 'prince' })
113+
114+
expect(response).toHaveProperty('hits', expect.any(Array))
115+
expect(response).toHaveProperty('offset', 0)
116+
expect(response).toHaveProperty('limit', 20)
117+
expect(response).toHaveProperty('processingTimeMs', expect.any(Number))
118+
expect(response).toHaveProperty('query', 'prince')
119+
expect(response.hits.length).toEqual(2)
120+
})
121+
96122
test(`${permission} key: Basic phrase search`, async () => {
97123
const client = await getClient(permission)
98124
const response = await client

tests/typed_search.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,15 @@ describe.each([
113113
expect(response.query === 'prince').toBeTruthy()
114114
})
115115

116+
test(`${permission} key: Search with query in searchParams`, async () => {
117+
const client = await getClient(permission)
118+
const response = await client
119+
.index(index.uid)
120+
.search('other', { q: 'prince' }) // ensures `q` is a valid field in SearchParams type
121+
122+
expect(response).toHaveProperty('query', 'prince')
123+
})
124+
116125
test(`${permission} key: Search with options`, async () => {
117126
const client = await getClient(permission)
118127
const response = await client

0 commit comments

Comments
 (0)