feat: Support creating search indexes for Firestore#10431
feat: Support creating search indexes for Firestore#10431
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for searchConfig in Firestore indexes, encompassing API type definitions, sorting logic, validation, and pretty-printing. The feedback focuses on improving code robustness and adhering to project standards. Specifically, it is recommended to use optional chaining when accessing index fields to prevent potential runtime errors, implement a more deterministic comparison logic for searchConfig instead of relying on JSON.stringify, remove the use of the any type in accordance with the style guide, and add explicit validation for optional configuration fields.
rwhogg
force-pushed
the
search-index-support
branch
2 times, most recently
from
ad5b5ad to
a673eeb
Compare
| }, | ||
| ], | ||
| }; | ||
| const indexWithSearch2: any = { |
There was a problem hiding this comment.
Use a specific type instead of any - is there a reason we can't just use index here?
Applies throughout
| if (lastField.vectorConfig) { | ||
| // lastField is vector field, refer to the second from last field | ||
| const vectorField = lastField; | ||
| if (lastField?.searchConfig || lastField?.vectorConfig) { |
There was a problem hiding this comment.
Now that we have 2 special types, can both show up in the same index? If so, i think this logic might be broken
rwhogg
force-pushed
the
search-index-support
branch
2 times, most recently
from
c5da14f to
8b9c243
Compare
rwhogg
force-pushed
the
search-index-support
branch
from
8b9c243 to
c6250b9
Compare
3 participants
Description
Adds support for creating Firestore search indexes
Scenarios Tested
Tested search configs with and without other existing index field types.
Sample Commands
firebase deploy --only firestore