-
Notifications
You must be signed in to change notification settings - Fork 91
Improve indexes #1918
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Improve indexes #1918
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1918 +/- ##
==========================================
+ Coverage 99.03% 99.92% +0.89%
==========================================
Files 18 18
Lines 1449 1382 -67
Branches 305 294 -11
==========================================
- Hits 1435 1381 -54
+ Misses 14 1 -13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the index type handling to align with the MeiliSearch Rust model by renaming and restructuring several index-related methods and types. Key changes include replacing the legacy index methods (e.g. getRawIndex, deleteIndexIfExists) with new ones (getIndex, deleteIndex) and updating the corresponding tests and documentation to match the new API.
Reviewed Changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
tests/env/typescript-browser/src/index.ts | Updated index creation and fetching methods to use new API. |
tests/env/node/search_example.cjs | Updated deletion and creation of indexes to use the new API methods. |
tests/env/express/public/index.html and headers.html | Replaced getRawInfo and delete with getIndex and deleteIndex calls. |
tests/embedders.test.ts, documents.test.ts, displayed_attributes.test.ts, client.test.ts, batch.test.ts | Updated test files to match method renaming and new argument signatures. |
src/types/*.ts | Renamed and refactored types to align with the updated index model. |
src/meilisearch.ts, src/indexes.ts | Refactored index-related methods and replaced deprecated calls with the new API. |
playgrounds/javascript/src/meilisearch.ts, README.md, .code-samples.meilisearch.yaml | Updated code samples and documentation to reflect the migration changes. |
Comments suppressed due to low confidence (1)
README.md:605
- Verify that all method signature updates in the documentation and code samples completely reflect the migration (e.g. replacing getRawIndex with getIndex and ensuring correct usage of waitTask where applicable) to avoid confusion among users.
client.createIndex({ uid: 'movies', primaryKey: 'id' })
WalkthroughThis update refactors the Meilisearch client and its related codebase to modernize and standardize index management APIs. Method signatures for creating, updating, retrieving, and deleting indexes now use object parameters and return unified types. Deprecated or redundant methods are removed, and corresponding documentation, tests, and type definitions are updated to match the new API structure. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MeiliSearch
participant Index
Client->>MeiliSearch: createIndex({ uid: "movies" })
MeiliSearch-->>Client: EnqueuedTaskPromise
Client->>MeiliSearch: getIndexes()
MeiliSearch-->>Client: IndexViewList
Client->>MeiliSearch: index("movies")
MeiliSearch-->>Client: Index instance
Client->>Index: getIndex()
Index-->>Client: IndexView
Client->>Index: updateIndex({ primaryKey: "id" })
Index-->>Client: EnqueuedTaskPromise
Client->>Index: deleteIndex()
Index-->>Client: EnqueuedTaskPromise
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (8)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/env/node/search_example.cjs (1)
13-13
: Consistency issue: Missing .waitTask() after deleteIndex()The deletion operation should also wait for task completion like the create operation on line 14 does.
- await client.index(indexUid).deleteIndex() + await client.index(indexUid).deleteIndex().waitTask()
🧹 Nitpick comments (3)
src/types/task-and-batch.ts (1)
141-147
: Added generic Results type for better reusabilityA new generic
Results<T>
type has been introduced to represent paginated results in a consistent way. This improves code reusability and standardizes the response format across different endpoints.Consider adding a JSDoc comment for the
Results<T>
type to document its purpose and structure, similar to the documentation provided for other types in this file.+/** + * A generic type for paginated results. + */ type Results<T> = { results: T[]; total: number; limit: number; from: number | null; next: number | null; };tests/index.test.ts (1)
36-43
: Consider testing the timestamp fields more explicitly.While the current destructuring approach works, consider adding explicit type checks for the timestamp fields to ensure they're properly formatted ISO strings or Date objects. This would provide more complete test coverage of the API response structure.
const { createdAt, updatedAt, ...myIndex } = await index.getIndex(); assert.deepEqual(myIndex, { primaryKey, uid: INDEX_UID_ONE, }); assert.typeOf(createdAt, "string"); assert.typeOf(updatedAt, "string"); + assert.match(createdAt, /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d+)?Z$/); + assert.match(updatedAt, /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d+)?Z$/);src/meilisearch.ts (1)
102-108
: Keep endpoint paths consistent & validate input forswapIndexes
Other endpoints (
indexes
,dumps
,snapshots
, …) omit a leading/
.
Staying consistent simplifies any future refactors in the URL builder.A quick guard against an empty array (or malformed payloads) shields the server from pointless calls and provides immediate feedback.
- swapIndexes(swapIndexesPayloads: SwapIndexesPayload[]): EnqueuedTaskPromise { - return this.#httpRequestsWithTask.post({ - path: "/swap-indexes", - body: swapIndexesPayloads, - }); + swapIndexes(swapIndexesPayloads: SwapIndexesPayload[]): EnqueuedTaskPromise { + if (!Array.isArray(swapIndexesPayloads) || swapIndexesPayloads.length === 0) { + throw new Error("swapIndexes payload must contain at least one item.") + } + return this.#httpRequestsWithTask.post({ + path: "swap-indexes", // remove leading slash for consistency + body: swapIndexesPayloads, + }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (41)
.code-samples.meilisearch.yaml
(2 hunks)README.md
(2 hunks)playgrounds/javascript/src/meilisearch.ts
(1 hunks)src/indexes.ts
(82 hunks)src/meilisearch.ts
(2 hunks)src/types/index.ts
(1 hunks)src/types/indexes.ts
(1 hunks)src/types/shared.ts
(1 hunks)src/types/task-and-batch.ts
(4 hunks)src/types/types.ts
(1 hunks)tests/batch.test.ts
(2 hunks)tests/client.test.ts
(3 hunks)tests/displayed_attributes.test.ts
(2 hunks)tests/documents.test.ts
(8 hunks)tests/embedders.test.ts
(1 hunks)tests/env/browser/index.html
(2 hunks)tests/env/express/public/headers.html
(2 hunks)tests/env/express/public/index.html
(2 hunks)tests/env/node/search_example.cjs
(1 hunks)tests/env/typescript-browser/src/index.ts
(2 hunks)tests/env/typescript-node/src/index.ts
(3 hunks)tests/facet_search.test.ts
(1 hunks)tests/facet_search_settings.test.ts
(2 hunks)tests/faceting.test.ts
(3 hunks)tests/filterable_attributes.test.ts
(2 hunks)tests/get_search.test.ts
(2 hunks)tests/index-stats.test.ts
(1 hunks)tests/index.test.ts
(1 hunks)tests/keys.test.ts
(1 hunks)tests/localized_attributes.test.ts
(2 hunks)tests/pagination.test.ts
(2 hunks)tests/prefix_search_settings.test.ts
(2 hunks)tests/search.test.ts
(7 hunks)tests/search_cutoff_ms.test.ts
(2 hunks)tests/searchable_attributes.test.ts
(2 hunks)tests/sortable_attributes.test.ts
(3 hunks)tests/task.test.ts
(2 hunks)tests/token.test.ts
(1 hunks)tests/typed_search.test.ts
(3 hunks)tests/utils/meilisearch-test-utils.ts
(3 hunks)tests/wait_for_task.test.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (12)
tests/faceting.test.ts (1)
src/meilisearch.ts (1)
index
(82-84)
playgrounds/javascript/src/meilisearch.ts (1)
tests/env/esm/src/index.js (1)
client
(4-4)
tests/env/typescript-browser/src/index.ts (2)
tests/env/esm/src/index.js (1)
client
(4-4)src/meilisearch.ts (1)
index
(82-84)
tests/facet_search.test.ts (1)
src/meilisearch.ts (1)
index
(82-84)
tests/task.test.ts (2)
tests/env/esm/src/index.js (1)
client
(4-4)src/meilisearch.ts (1)
index
(82-84)
tests/token.test.ts (1)
tests/env/esm/src/index.js (1)
client
(4-4)
tests/sortable_attributes.test.ts (1)
src/meilisearch.ts (1)
index
(82-84)
tests/env/typescript-node/src/index.ts (2)
tests/env/esm/src/index.js (1)
client
(4-4)src/meilisearch.ts (1)
index
(82-84)
src/types/task-and-batch.ts (2)
src/types/indexes.ts (1)
SwapIndexesPayload
(47-47)src/http-requests.ts (1)
T
(199-264)
tests/index-stats.test.ts (2)
tests/utils/meilisearch-test-utils.ts (2)
getClient
(262-262)assert
(135-138)src/meilisearch.ts (1)
index
(82-84)
src/types/indexes.ts (1)
src/types/shared.ts (1)
PaginationView
(4-9)
src/meilisearch.ts (5)
src/http-requests.ts (1)
T
(199-264)src/types/types.ts (1)
RecordAny
(10-10)src/indexes.ts (1)
Index
(61-1316)src/types/indexes.ts (4)
ListIndexes
(8-11)IndexViewList
(26-26)IndexCreateRequest
(40-40)SwapIndexesPayload
(47-47)src/types/task-and-batch.ts (1)
EnqueuedTaskPromise
(133-139)
🔇 Additional comments (106)
tests/token.test.ts (1)
86-86
: API usage updated correctly for index deletion.The code now properly uses the new
deleteIndex()
method instead of the olddelete()
method, and correctly awaits the task completion withwaitTask()
. This aligns with the broader API refactoring to standardize index manipulation methods.tests/search_cutoff_ms.test.ts (2)
104-104
: Index creation API updated correctly.The code now uses the object parameter format
{ uid: index.uid }
instead of passing the UID string directly, aligning with the renamedIndexCreateRequest
type mentioned in the PR objectives.
135-135
: Index creation API updated correctly.The code now uses the object parameter format
{ uid: index.uid }
instead of passing the UID string directly, aligning with the renamedIndexCreateRequest
type mentioned in the PR objectives.tests/sortable_attributes.test.ts (3)
32-32
: Index creation API updated correctly.The code now uses the object parameter format
{ uid: index.uid }
instead of passing the UID string directly, aligning with the renamedIndexCreateRequest
type mentioned in the PR objectives.
82-82
: Index creation API updated correctly.The code now uses the object parameter format
{ uid: index.uid }
instead of passing the UID string directly, aligning with the renamedIndexCreateRequest
type mentioned in the PR objectives.
113-113
: Index creation API updated correctly.The code now uses the object parameter format
{ uid: index.uid }
instead of passing the UID string directly, aligning with the renamedIndexCreateRequest
type mentioned in the PR objectives.tests/prefix_search_settings.test.ts (2)
62-62
: Proper update to createIndex APIThe code correctly uses the new object parameter format for the createIndex method, which is part of the API refactoring to better align with the MeiliSearch Rust source code model.
94-94
: Properly implemented createIndex API updateThe change correctly implements the new signature format for createIndex, passing an object with a uid property instead of a string parameter.
tests/batch.test.ts (2)
8-8
: Good direct object declaration for indexThe simplification of declaring the index as an object literal follows good JavaScript practices and prepares it to be directly passed to the createIndex method.
19-19
: Correctly uses object parameter for createIndexThe update properly passes the index object to createIndex, aligning with the API refactoring to standardize index creation methods.
tests/keys.test.ts (1)
139-139
: API-compliant index creationThe createIndex call correctly implements the new object parameter format, aligning with the standardized index management API refactoring.
tests/filterable_attributes.test.ts (2)
93-93
: Proper update to createIndex method signatureThis change correctly implements the new object parameter format for createIndex, consistent with the API standardization across the codebase.
124-124
: Consistent implementation of createIndex APIThe update correctly uses the object parameter format for index creation, maintaining consistency with the refactored API design.
tests/searchable_attributes.test.ts (2)
81-81
: Correctly updated to use object parameter for index creation.The method call has been updated to use the new API signature where
createIndex
expects an object parameter with auid
property, instead of a direct string UID. This aligns with the refactored index management API.
112-112
: Correctly updated to use object parameter for index creation.The method call has been updated to use the new API signature where
createIndex
expects an object parameter with auid
property, instead of a direct string UID. This aligns with the refactored index management API.tests/pagination.test.ts (2)
87-87
: Correctly updated to use object parameter for index creation.The method call has been updated to use the new API signature where
createIndex
expects an object parameter with auid
property, instead of a direct string UID. This aligns with the refactored index management API.
118-118
: Correctly updated to use object parameter for index creation.The method call has been updated to use the new API signature where
createIndex
expects an object parameter with auid
property, instead of a direct string UID. This aligns with the refactored index management API.tests/faceting.test.ts (3)
33-33
: Correctly updated to use object parameter for index creation.The method call has been updated to use the new API signature where
createIndex
expects an object parameter with auid
property, instead of a direct string UID. This aligns with the refactored index management API.
92-92
: Correctly updated to use object parameter for index creation.The method call has been updated to use the new API signature where
createIndex
expects an object parameter with auid
property, instead of a direct string UID. This aligns with the refactored index management API.
121-121
: Correctly updated to use object parameter for index creation.The method call has been updated to use the new API signature where
createIndex
expects an object parameter with auid
property, instead of a direct string UID. This aligns with the refactored index management API.tests/wait_for_task.test.ts (1)
22-22
: Correctly updated to use object parameter for index creation.The method call has been updated to use the new API signature where
createIndex
expects an object parameter with auid
property, instead of a direct string UID. This aligns with the refactored index management API.tests/env/node/search_example.cjs (1)
14-14
: LGTM: Properly updated to new API formatThe createIndex call has been correctly updated to use the new object parameter format, and it properly chains waitTask() for task completion.
tests/localized_attributes.test.ts (2)
111-111
: LGTM: Properly updated to new API formatThe createIndex call has been correctly updated to use the new object parameter format with the uid property, and it properly chains waitTask() for task completion.
142-142
: LGTM: Properly updated to new API formatThe createIndex call has been correctly updated to use the new object parameter format with the uid property, and it properly chains waitTask() for task completion.
tests/search.test.ts (7)
129-130
: LGTM: Properly updated to new API formatThe createIndex calls have been correctly updated to use the new object parameter format with the uid property. In this case, waitTask() isn't chained since it's not needed for the specific test scenario.
295-295
: LGTM: Properly updated to new API formatThe createIndex call has been correctly updated to use the new object parameter format with the uid property.
373-373
: LGTM: Properly updated to new API formatThe createIndex call has been correctly updated to use the new object parameter format with the uid property.
1250-1250
: LGTM: Properly updated method nameThe index deletion method has been correctly renamed from delete() to deleteIndex() to better reflect its purpose, following the API changes outlined in the PR.
1263-1263
: LGTM: Properly updated to new API formatThe createIndex call has been correctly updated to use the new object parameter format with the uid property, and it properly chains waitTask() for task completion.
1292-1292
: LGTM: Properly updated to new API formatThe createIndex call has been correctly updated to use the new object parameter format with the uid property.
1366-1366
: LGTM: Properly updated to new API formatThe createIndex call has been correctly updated to use the new object parameter format with the uid property, and it properly chains waitTask() for task completion.
tests/embedders.test.ts (1)
61-61
: LGTM: Properly updated to new API formatThe createIndex call has been correctly updated to use the new object parameter format with the uid property, and it properly chains waitTask() for task completion.
tests/typed_search.test.ts (3)
116-117
: API signature update for createIndexThe code has been updated to use the new index creation API which now accepts an object with
uid
property instead of a string parameter. This aligns with the objective to standardize index-related methods across the codebase.
396-396
: Index deletion method renamed for improved consistencyChanged from using
.delete()
to.deleteIndex()
on the index instance. This provides a clearer API by explicitly stating the operation being performed.
410-410
: API signature update for createIndexUpdated to use the new object-based parameter style with the
uid
property, consistent with other index creation calls in the file.tests/task.test.ts (2)
34-34
: API signature update for createIndexModified to use the new object parameter format with a
uid
property instead of passing the index UID as a string. This change aligns with the refactored index API.
95-95
: API signature update for createIndexUpdated index creation call to use the object parameter format with
uid
property, consistent with the changes throughout the codebase.tests/displayed_attributes.test.ts (2)
77-77
: API signature update for createIndexChanged index creation to use the new object parameter format with a
uid
property for the "Search" permission test suite, consistent with the updated API.
109-109
: API signature update for createIndexUpdated index creation call for the "No" permission test suite to use the new object parameter format, maintaining consistency across the codebase.
tests/facet_search_settings.test.ts (2)
62-62
: API signature update for createIndexChanged index creation to use the new object parameter format with a
uid
property for the "Search" permission test suite, consistent with the refactored index API.
94-94
: API signature update for createIndexUpdated index creation call for the "No" permission test suite to use the new object parameter format, maintaining consistency across the codebase.
src/types/index.ts (1)
1-2
: LGTM: Updated exports to reflect new type organization.The changes properly export the new index-related types from the dedicated module and update the path format to a consistent hyphenated style for task and batch types.
tests/env/express/public/headers.html (2)
26-26
: Updated API usage with object parameter pattern.The method call has been properly updated to match the new API signature, using an object with a
uid
property instead of a direct string parameter.
43-43
: Updated to new index deletion API.The index deletion method has been properly changed to use the new
deleteIndex()
method and includes proper task handling withwaitTask()
.tests/env/browser/index.html (3)
26-26
: Updated index creation to use object parameter pattern.The method call has been properly updated to match the new API signature, using an object with a
uid
property instead of a direct string parameter, and includes proper task handling.
30-30
: Updated to use the new index information retrieval method.The method to retrieve index information has been properly updated from the deprecated
getRawInfo()
to the newgetIndex()
method, aligning with the API changes.
44-44
: Updated to new index deletion API.The index deletion method has been properly changed to use the new
deleteIndex()
method name and includes proper task handling withwaitTask()
.src/types/types.ts (2)
7-7
: LGTM: Updated import path to consistent format.The import path for
WaitOptions
has been updated to use the hyphenated format, ensuring consistency across the codebase.
1-1108
:✅ Verification successful
Verify that index-related types have been moved to the new dedicated module.
The import path has been updated, but I notice that the PR objectives mentioned removing several index-related types (like
IndexOptions
,IndexObject
, etc.) from this file and moving them to a dedicated module. Let's verify that these types are properly defined in the new module.
🏁 Script executed:
#!/bin/bash # Description: Verify that index-related types have been moved from types.ts to the dedicated indexes.ts file. # Check that the indexes.ts file exists and contains the expected types echo "Checking that the indexes.ts file exists and contains the expected types:" grep -e "IndexView" -e "IndexViewList" -e "ListIndexes" -e "UpdateIndexRequest" -e "IndexCreateRequest" src/types/indexes.ts # Verify that no references to old types remain in the codebase echo "Verifying that no references to old types remain in the codebase:" rg "IndexOptions|IndexObject|IndexesQuery|IndexesResults" --type tsLength of output: 949
Index-related types have been moved to
src/types/indexes.ts
- Confirmed
ListIndexes
,IndexView
,IndexViewList
,UpdateIndexRequest
, andIndexCreateRequest
are defined insrc/types/indexes.ts
.- No remaining references to the old types (
IndexOptions
,IndexObject
,IndexesQuery
,IndexesResults
) were found in the codebase.No further action required.
tests/utils/meilisearch-test-utils.ts (4)
3-3
: Added Task type import to support new task validation functionality.The Task type import is now included alongside Config to support the new
isTaskSuccessful
assertion helper.
82-82
: Updated to use new index API method name.The method call has been updated from
getRawIndexes()
togetIndexes()
to align with the refactored API structure described in the PR objectives.
86-86
: Updated index deletion to use the new method name.The index deletion operation now uses
deleteIndex()
instead ofdelete()
, reflecting the standardized naming convention in the refactored API.
130-133
: Added helpful task status validation utility.The new
isTaskSuccessful
helper method provides a convenient way to assert that a task has completed successfully, checking both for null error and a "succeeded" status.tests/get_search.test.ts (2)
87-88
: Updated index creation to use new object parameter signature.The
createIndex
method calls now use an object with auid
property instead of a direct string parameter, aligning with the refactored API structure.
549-549
: Updated index deletion to use the new method name.The index deletion operation now uses
deleteIndex()
instead ofdelete()
, reflecting the standardized naming convention in the refactored API.tests/env/express/public/index.html (3)
29-29
: Updated index creation to use new object parameter signature.The
createIndex
method call now uses an object with auid
property instead of a direct string parameter, aligning with the refactored API structure.
33-33
: Updated to use new index information retrieval method.The method call has been updated from
getRawInfo()
togetIndex()
to align with the refactored API structure described in the PR objectives.
51-51
: Updated index deletion to use the new method name.The index deletion operation now uses
deleteIndex()
instead ofdelete()
, reflecting the standardized naming convention in the refactored API.tests/env/typescript-browser/src/index.ts (3)
1-1
: Simplified imports by removing unnecessary type.Removed the explicit import of
IndexObject
type, which has been renamed toIndexView
according to the PR objectives.
17-17
: Updated to use new index listing method.The method call has been updated from
getRawIndexes()
togetIndexes()
to align with the refactored API structure described in the PR objectives.
19-19
: Removed explicit type annotation in mapping function.The type annotation for the
index
parameter in the mapping function has been removed, as it previously referenced the now-renamedIndexObject
type.playgrounds/javascript/src/meilisearch.ts (2)
13-13
: Updated index deletion to use new API patternThe code now uses the more explicit chain
client.index(indexUid).deleteIndex().waitTask()
instead of the deprecateddeleteIndexIfExists(indexUid)
method. This change aligns with the PR objective of removing index manipulation methods that don't directly take an index UID parameter.
15-15
: Updated index creation to use object parameterThe code now uses
createIndex({ uid: indexUid })
instead of passing the UID directly as a string parameter. This change follows the new API convention that standardizes method signatures using object parameters, as described in the PR objectives.tests/documents.test.ts (7)
35-35
: Updated index creation to use object parameterThe code now uses
createIndex({ uid: indexNoPk.uid })
instead of passing the UID directly as a string parameter, aligning with the updated API.
37-39
: Updated index creation with primaryKey to use object parameterThe code now uses an object with
uid
andprimaryKey
properties for index creation, following the standardized API pattern outlined in the PR objectives.
414-414
: Updated method to get index informationChanged from using the deprecated method
fetchInfo()
to the newgetIndex()
method, which follows the refactored API design.
427-427
: Updated method to get index informationChanged from using the deprecated method
fetchInfo()
to the newgetIndex()
method, which follows the refactored API design.
443-443
: Updated method to get index informationChanged from using the deprecated method
fetchInfo()
to the newgetIndex()
method, which follows the refactored API design.
461-461
: Updated method to get index informationChanged from using the deprecated method
fetchInfo()
to the newgetIndex()
method, which follows the refactored API design.
563-563
: Updated index creation to use object parameterThe code now uses
createIndex({ uid: indexPk.uid })
instead of passing the UID directly as a string parameter, maintaining consistency with the updated API.src/types/shared.ts (2)
3-4
: Renamed and documented type to align with Rust implementationRenamed
CursorResults<T>
toPaginationView<T>
and added a JSDoc reference to the corresponding Rust typemeilisearch::routes::PaginationView
. This change directly aligns with the PR objective of refactoring types to match the Rust source code model.
5-7
: Updated pagination propertiesChanged pagination properties by replacing
from
andnext
with anoffset: number
property. This modification is part of the standardization effort to more closely match the Rust implementation's pagination model.tests/index-stats.test.ts (2)
8-11
: Using the new deleteIndex API in test cleanupThe test correctly uses the new API pattern
ms.index(INDEX_UID).deleteIndex().waitTask()
for index deletion in the cleanup code, consistent with the refactored index API.
13-40
: Well-structured test for index statisticsThis test properly validates the
getStats
method on an index, including verification of important statistics like document count, field distribution, and database size. The test follows good practices by:
- Setting up test data with unique fields
- Waiting for and verifying task completion
- Making specific assertions about the returned statistics
The implementation aligns with the overall API refactoring objectives of the PR.
tests/env/typescript-node/src/index.ts (4)
30-31
: API usage updated to follow new pattern for index operationsThe code now correctly uses the new index manipulation pattern where:
- Index-specific operations are performed on an index instance obtained via
client.index(indexUid)
- Index creation now accepts an object parameter with a
uid
propertyThese changes align with the PR's objective to refactor index handling to match the MeiliSearch Rust source code model.
34-38
: Method renamed fromgetRawIndexes
togetIndexes
The code now uses the renamed
getIndexes()
method instead of the deprecatedgetRawIndexes()
, correctly implementing the migration described in the PR objectives.
46-46
: Simplified index mapping operationThe code properly uses the results directly without needing to specify the return type, which is consistent with the removal of
IndexObject
type in favor ofIndexView
.
65-65
: Consistent use of new index deletion patternThe code correctly uses the refactored pattern for deleting an index, which aligns with the PR's goal of removing index manipulation methods from the
MeiliSearch
class that don't directly take an index UID parameter..code-samples.meilisearch.yaml (6)
27-27
: Updated method name fromgetRawInfo
togetIndex
The code sample correctly uses the renamed
getIndex()
method instead of the deprecatedgetRawInfo()
, which aligns with the API changes outlined in the PR migration guide.
31-31
: Updated index creation to use object parameterThe code sample now properly uses an object parameter with
uid
andprimaryKey
properties instead of passing them as separate arguments, reflecting the standardized method signature.
33-33
: Updated index update method to use index instanceThe code correctly uses the new pattern where index-specific operations are performed on an index instance, and the method name has been updated to
updateIndex
.
35-35
: Updated index deletion to use index instanceThe code sample now uses the index instance method
deleteIndex()
instead of the client-level method, which is consistent with the overall refactoring approach.
405-407
: Consistent usage of updated index instance methodThe code example correctly uses the index instance and the renamed
updateIndex
method with an object parameter, maintaining consistency with other similar operations.
409-409
: Consistent index creation with object parameterThis code sample maintains consistency with the updated index creation approach using an object with
uid
andprimaryKey
properties.src/types/task-and-batch.ts (4)
1-2
: Updated imports with cleaner structureThe imports now correctly use the new types structure, importing
SwapIndexesPayload
from "./indexes.js" instead of using a locally defined type. This aligns with the PR's goal of refactoring index types to match the Rust source code model.
110-110
: Updated type reference for swaps propertyThe
swaps
property now correctly uses the importedSwapIndexesPayload[]
type instead of the oldIndexSwap[]
type, ensuring consistency with the new type system.
154-154
: Simplified TasksResults type definitionThe
TasksResults
type now correctly uses the new genericResults<Task>
type, simplifying the type definition and ensuring consistency with other similar types.
200-200
: Simplified BatchesResults type definitionSimilar to
TasksResults
, theBatchesResults
type now uses the genericResults<Batch>
type, maintaining consistency in how paginated results are typed throughout the codebase.tests/client.test.ts (4)
11-11
: Simplified imports by removing unused index-related typesThe imports have been properly updated to remove index-related types that are no longer used in this file after the removal of index management tests.
244-244
: Updated index creation to use object parameterThe test correctly uses the new index creation pattern with an object parameter
{ uid: "test" }
instead of a string.
265-265
: Consistent usage of updated index creation patternThis test also correctly uses the new index creation pattern with an object parameter, maintaining consistency throughout the test file.
271-271
: Updated to use new index access patternThe test now correctly uses
client.index("test")
to get an index instance rather than the deprecatedclient.getIndex("test")
method, aligning with the new API design.src/types/indexes.ts (1)
1-48
: Well-organized type definitions with clear documentation links.The new type definitions are well-structured and properly documented with links to the Meilisearch API documentation and references to the corresponding Rust implementation. The types align well with the PR's objective of better matching the Rust source code model.
Each type has a clear purpose:
ListIndexes
for pagination parametersIndexView
replaces the previousIndexObject
IndexViewList
for paginated resultsUpdateIndexRequest
andIndexCreateRequest
for mutation operationsSwapIndexesPayload
for index swapping operationsThe types are properly exported and have well-defined properties with correct types.
tests/index.test.ts (1)
1-100
: Tests appropriately updated to match the new API.The test suite has been effectively simplified to focus on core index lifecycle operations, using a consistent pattern for testing creation, retrieval, updating, deletion, and swapping of indexes. The tests properly use the new method signatures and patterns, such as:
ms.createIndex({ uid })
instead of the old signatureindex.getIndex()
replacing the deprecatedgetRawInfo()
index.updateIndex()
andindex.deleteIndex()
methodsThe cleanup in
afterAll
is a good practice to ensure tests don't leave artifacts behind.src/indexes.ts (6)
61-65
: Good encapsulation of the uid property.Converting the
uid
from a public property to a private field with a getter improves encapsulation and prevents accidental mutation. This is a good practice to ensure the integrity of the index identity.
76-78
: Constructor simplified correctly.The constructor has been simplified by removing the optional
primaryKey
parameter, which is appropriate since index properties are now fetched via thegetIndex()
method rather than being stored in the instance.
194-197
: Well-defined replacement for getRawInfo().The new
getIndex()
method provides a cleaner API for fetching index information. The method is properly documented with a link to the Meilisearch API documentation and returns the newIndexView
type.
199-205
: Method renaming improves clarity.Renaming
update
toupdateIndex
makes the method's purpose more explicit and consistent with other method names. The parameter type has been updated to use the newUpdateIndexRequest
type, which aligns with the Rust model.
207-210
: Method renaming improves clarity.Renaming
delete
todeleteIndex
makes the method's purpose more explicit and consistent with other method names. This change helps avoid confusion with JavaScript's built-indelete
operator.
1-1316
: All API paths updated consistently.All HTTP request paths have been updated to use the private
#uid
field, ensuring consistent encapsulation throughout the codebase. This is a comprehensive change that touches all methods in the class.README.md (5)
598-598
: Updated method signature aligns with code changes.The
getIndexes
method signature has been updated to use the newListIndexes
type and returnIndexViewList
, matching the implementation changes.
605-605
: Updated method signature aligns with code changes.The
createIndex
method signature has been updated to accept anIndexCreateRequest
object, correctly reflecting the new implementation that uses object parameters instead of separate arguments.
617-617
: Updated API reference for index retrieval.The method for getting index information has been correctly updated to use
getIndex()
returning anIndexView
, replacing the previousgetRawIndex
.
623-623
: Updated method signature for updateIndex.The method signature for updating an index has been correctly updated to use
updateIndex
with an optionalUpdateIndexRequest
parameter.
629-629
: Updated method signature for deleteIndex.The method signature for deleting an index has been correctly updated to use
deleteIndex
, replacing the previousdelete
method.src/meilisearch.ts (1)
94-100
: Add a quick client-side check for theuid
field before firing the request
createIndex
blindly forwards the payload to the HTTP layer.
Fail-fast validation keeps the error surface closer to the caller and avoids an unnecessary network round-trip (saves latency & bandwidth and produces clearer stack traces).createIndex(indexCreateRequest: IndexCreateRequest): EnqueuedTaskPromise { + if (!indexCreateRequest?.uid) { + throw new Error( + "Index creation payload must include a non-empty `uid` property." + ) + } return this.#httpRequestsWithTask.post({ path: "indexes", body: indexCreateRequest, }) }[ suggest_optional_refactor ]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/meilisearch.ts (1)
1-377
: 💡 Verification agent🧩 Analysis chain
Verify that removed methods are correctly documented in migration guide
The PR objectives mention several removed methods (
getIndex
,getRawIndex
,getRawIndexes
,updateIndex
,deleteIndex
,deleteIndexIfExists
). These removals align with the PR's goal of simplifying the API and removing redundancy.
🏁 Script executed:
#!/bin/bash # Check if removed methods are properly documented in migration guide # Look for migration guide files echo "Looking for migration guide files:" fd --type file migration_guide -x echo "Found: {}" # Check if the migration guide contains the removed method names echo "\nChecking if removed methods are documented in migration guides:" methods=("getIndex" "getRawIndex" "getRawIndexes" "updateIndex" "deleteIndex" "deleteIndexIfExists") for method in "${methods[@]}"; do echo "\nSearching for documentation of removed method: $method" rg "$method" --context 2 --glob "*.md" done # Also check if README or other documentation was updated echo "\nChecking if README or other documentation was updated:" rg -i "migration|api changes|breaking change" --glob "*.md" --context 2Length of output: 2367
Add a migration guide and update outdated docs
It looks like there isn’t a dedicated migration guide in the repo, and the README still shows calls to methods that were removed (
getIndex
,updateIndex
,deleteIndex
, etc.). We need to:
- Create a
MIGRATION.md
(or similar) detailing the removal of:
getIndex
→ useclient.index(uid).getRawInfo()
/getStats()
getRawIndex
/getRawIndexes
updateIndex
→ useclient.index(uid).update()
deleteIndex
→ useclient.index(uid).delete()
deleteIndexIfExists
→ useclient.index(uid).deleteIfExists()
- In README.md, remove or replace these outdated examples:
client.index(uid: string).getIndex(): Promise<IndexView> client.index(uid: string).updateIndex(...): EnqueuedTaskPromise client.index(uid: string).deleteIndex(): EnqueuedTaskPromise- Reference the migration guide in README to help users upgrade.
Please add this documentation before merging.
🧹 Nitpick comments (1)
src/meilisearch.ts (1)
95-101
: Improved index creation APIThe
createIndex
method now accepts a singleIndexCreateRequest
object parameter instead of separate parameters, which follows modern API design patterns and provides better extensibility.Consider adding a code sample in the JSDoc to demonstrate how to use this method with the new parameter structure.
/** {@link https://www.meilisearch.com/docs/reference/api/indexes#create-an-index} */ createIndex(indexCreateRequest: IndexCreateRequest): EnqueuedTaskPromise { + /** + * @example + * ```ts + * // Create an index with only a UID + * client.createIndex({ uid: 'movies' }); + * + * // Create an index with UID and primaryKey + * client.createIndex({ uid: 'movies', primaryKey: 'movie_id' }); + * ``` + */ return this.#httpRequestsWithTask.post({ path: "indexes", body: indexCreateRequest, }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (5)
src/meilisearch.ts
(2 hunks)src/types/index.ts
(1 hunks)tests/documents.test.ts
(8 hunks)tests/embedders.test.ts
(1 hunks)tests/search.test.ts
(7 hunks)
✅ Files skipped from review due to trivial changes (2)
- tests/search.test.ts
- tests/documents.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/embedders.test.ts
- src/types/index.ts
🔇 Additional comments (4)
src/meilisearch.ts (4)
19-19
: Types renamed for improved consistencyThe type renames align with the PR objective to make index-related types match the MeiliSearch Rust source code model, improving consistency across the codebase.
Also applies to: 27-29
78-81
: Documentation updated to reflect refactored index APIThe updated JSDoc comments correctly describe the
index
method's purpose and return value, which is part of the new API structure.
87-93
: Simplified index listing implementationThe
getIndexes
method has been properly refactored to return raw index data (IndexViewList
) directly from the API rather than transforming results intoIndex
instances. This aligns with the goal of separating index details from index instances.
104-108
: Updated type for swapIndexes methodThe
swapIndexesPayload
parameter type has been correctly updated to match the renamed type, maintaining consistency with other renamed types in this PR.
Pull Request
What does this PR do?
meilisearch
rust source code model as much as possibleIndex
class that don't directly take an index UID parameter, and vice versa forMeiliSearch
classIndex
instances from queried datadeleteIndexIfExists
, this is a flawed function that is no longer required, because the base delete function doesn't throw if the index doesn't exist, so it has the same effectMigration
getRawIndex
->getIndex
IndexObject
->IndexView
getRawIndexes
->getIndexes
IndexesResults<IndexObject[]>
->IndexViewList
IndexesQuery
->ListIndexes
IndexOptions
->IndexCreateRequest
These are hardly useful, but here's the migration anyway:
Summary by CodeRabbit
New Features
Refactor
createIndex({ uid })
,getIndex()
,deleteIndex()
), replacing various legacy and redundant methods.Bug Fixes
Documentation
Tests
Chores