Skip to content
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

feat(NODE-6044): add detailed types for SearchIndexDescription #4052

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

noseworthy
Copy link

Description

Adds more detailed type definitions for SearchIndexDescription and SearchIndexDefinition based on the documentation found at Create an Atlas Search Index.

The existing type:

export interface SearchIndexDescription {
  name?: string;
  definition: Document;
}

allows users to use any object for the definition property. Search index definitions are fairly complicated and having more strict types defined for this property will make it easier to use the createSearchIndex() and updateSearchIndex() apis.

What is changing?

Added full types for defining a search index definition.

Is there new documentation needed for these changes?

I don't think any new hand-written documentation is necessary, but new tsdoc documentation should be generated to capture the new types.

What is the motivation for this change?

Using the createSearchIndex() and updateSearchIndex() apis when the definition is just specified to be a bson.Document is a bit unwieldy. The search index definitions can be fairly complicated, and it's very helpful to have TypeScript help validate the definitions.

Release Highlight

Fill in title or leave empty for no highlight

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@W-A-James
Copy link
Contributor

Hi @noseworthy thanks so much for all your work on this PR. We're currently reviewing this and will get back to you soon.

@W-A-James
Copy link
Contributor

Hi @noseworthy! Thanks for your patience and thanks again for all your hard work on this PR. After reviewing it with the team, we've decided that accepting these changes into the driver isn't the right choice for us at the moment. Taking on the burden of keeping these types accurate as the Atlas Search product continues to be developed is not something we can commit to at this time.
The driver also might not be the right place for these changes as the type definitions would then be bound to specific releases of the Node driver. This is something we generally try to avoid as we put a lot of work into having our driver be compatible and easy to use across all our currently supported server versions.
We will leave this PR open for consideration later down the line.

@nbbeeken nbbeeken added External Submission PR submitted from outside the team tracked-in-jira Ticket filed in MongoDB's Jira system labels Mar 28, 2024
@noseworthy
Copy link
Author

Hi @noseworthy! Thanks for your patience and thanks again for all your hard work on this PR. After reviewing it with the team, we've decided that accepting these changes into the driver isn't the right choice for us at the moment. Taking on the burden of keeping these types accurate as the Atlas Search product continues to be developed is not something we can commit to at this time. The driver also might not be the right place for these changes as the type definitions would then be bound to specific releases of the Node driver. This is something we generally try to avoid as we put a lot of work into having our driver be compatible and easy to use across all our currently supported server versions. We will leave this PR open for consideration later down the line.

No worries, @W-A-James . Thanks for taking a look and considering it. We're really excited to start using the Atlas Search features more now that we can run it locally for our dev environments but I found definitions for the indices really complicated. I was hoping that this would've been able to make it a bit more approachable than a simple bson.Document declaration. But I totally understand the burden of maintaining these types. Like I said, they're complicated and I'm sure with vector search changing frequently too it'd be a huge pain.

Hopefully as the product evolves we can get better typescript support for some of these features 🤞. Thanks for all the team's work on the driver!

@W-A-James W-A-James removed their assignment Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Submission PR submitted from outside the team tracked-in-jira Ticket filed in MongoDB's Jira system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants