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

Add TypeScript declarations #13

Merged
merged 4 commits into from
Mar 25, 2020
Merged

Add TypeScript declarations #13

merged 4 commits into from
Mar 25, 2020

Conversation

9at8
Copy link
Collaborator

@9at8 9at8 commented Mar 25, 2020

@lorensr I'm not sure why npm install triggered a massive change in the lock file.

~/src/github.com/apollo-datasource-mongodb types
❯ npm --version
6.14.3

~/src/github.com/apollo-datasource-mongodb types
❯ node --version
v12.16.1

I have a project that I'm working on (it's private), and I'm using mongodb collections in that project. It works to the best of my knowledge.

Any feedback is appreciated! 😃

TContext
> {
protected collection: Collection<TData>
protected model: Model<TData>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is only sometimes defined—should it be model? ?

Copy link
Collaborator Author

@9at8 9at8 Mar 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we change this to model?, then in a project which uses mongoose, devs would have to do this.model?.someMethod or this.model && this.model.someMethod.

As seen on line 14, this is going to conditionally have type MongooseModel<T> (if the class is specialized with a mongoose collection) or never if the constructor is specialized with a mongo collection)

Maybe I should change line 14 to this:

  export type Model<T> = T extends Document ? MongooseModel<T> : undefined

Copy link
Collaborator Author

@9at8 9at8 Mar 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After this change ^

this.model will have type:

  • MongooseModel<T> if constructor is passed a MongooseModel<T>
  • undefined if we're using a MongoCollection

What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds more accurate than never? whatever you think best

@lorensr lorensr merged commit 36cc9e7 into GraphQLGuide:master Mar 25, 2020
@lorensr
Copy link
Member

lorensr commented Mar 25, 2020

Thanks! Released in 0.2.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants