-
Notifications
You must be signed in to change notification settings - Fork 64
Create index.d.ts for Typescript #10
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
Conversation
|
Thanks! Wondering how to test this (eg w/ VSCode) and publish to npm? |
|
|
||
| interface Options { ttl: number } | ||
|
|
||
| export abstract class MongoDataSource<T extends mongoose.Document, TContext = any> extends DataSource { |
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.
| export abstract class MongoDataSource<T extends mongoose.Document, TContext = any> extends DataSource { | |
| export abstract class MongoDataSource<T extends mongoose.Document, TContext = any> extends DataSource<TContext> { |
DataSource is also a generic type, which optionally takes in the context type, which is what we should do imo.
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.
Also, I don't think the collection type makes these typings compatible with the native mongodb connector, which has a generic type for collection
| constructor(collection: mongoose.Collection | mongoose.Model<T>) | ||
| public initialize(config: DataSourceConfig<TContext>): void | ||
| protected findOneById(id: string, { ttl }?: Options): T | null | ||
| protected findManyByIds(ids: string[], { ttl }?: Options): (T| null)[] |
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.
The ids are supposed to be ObjectIds, at least for the native driver. Can you confirm @lorensr ?
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.
|
Hi guys, Any progress on this feature..? We'd love to use this in one of our projects and for that we need the Typescript support. Thanks in advance. |
|
@gayansanjeewa we're waiting on either @yoonhoGo to make the suggested changes and someone to test it, or for someone to create another PR with the changes |
|
Merged #13, released in |

No description provided.