GraphQLGuide / apollo-datasource-mongodb

Apollo data source for MongoDB
MIT License
285 stars 64 forks source link

Typescript declaration for `this` object #82

Open danielsyang opened 3 years ago

danielsyang commented 3 years ago

Hello everybody this is my first time using this library and I'm wondering if there's a way to define the return type of the collection object instead of it returning any

image

Thanks in advance

lorensr commented 3 years ago

I'm not sure—looks like it should be a Collection? https://github.com/GraphQLGuide/apollo-datasource-mongodb/blob/master/index.d.ts#L39

danielsyang commented 3 years ago

Yes! I was testing and checking all the attributes of the this.collection object and it looks like it's exactly the same type as Collection https://mongodb.github.io/node-mongodb-native/api-generated/collection.html# I couldn't find a way to get it working besides casting (this.collection as MongoCollection)

Funnily enough, when using mongoose everything is very well defined, everything works like a charm. :)

paulriley commented 2 years ago

I've just run into this. The underlying problem seems to be that the code here creates an unintended dependency on mongoose. If you don't have mongoose installed in the project, Document and MongooseCollection both resolve to any, which means

  export type Collection<T, U = MongoCollection<T>> = T extends Document
    ? MongooseCollection
    : U

is treated the same as

  export type Collection<T, U = Collection<T>> = T extends any
    ? any
    : U

Obviously, T extends any is always true and thus Collection<TData> resolves to any.

lorensr commented 2 years ago

If you don't have mongoose installed in the project, Document and MongooseCollection both resolve to any, which means

In 0.5.3, mongoose is a dependency, if that helps

GeorgGroenendaal commented 2 years ago

I have mongoose and and apollo-datasource-mongodb 0.5.4 installed and I am still have issues with type declarations for this.model. Right now I am solving it by adding const model = this.model as Model<ActualDataType>.

Is there a better way of doing this?

paulriley commented 2 years ago

@GeorgGroenendaal Bit of a punt, but is your problem that ActualDataType isn't derived from Mongoose Document as per Issue 78?

I've tried to rejig the types to solve both issues but it's difficult. I think the underlying problem is a need for separation of concerns. The Mongoose datasource needs to be in a separate class, perhaps a separate package, from the MongoClient one. Just to make the typings work.

But, giving it a lot of thought while using this package in anger for a couple of months, I'm beginning to conclude that a better solution might be a Data Source that works with any Data Loader and separate Data Loaders for MongoClient and Mongoose. Planning to give that a try soon, if I don't find one.