Add generic support to Collection and it's methods #16315

beary · 2017-05-04T11:35:50Z

Please fill in this template.

Use a meaningful title for the pull request. Include the name of the package modified.
Test the change in your own code. (Compile and run.)
Follow the advice from the readme.
Avoid common mistakes.
Run npm run lint package-name (or tsc if no tslint.json is present).

Select one of these and delete the others:

If changing an existing definition:

Provide a URL to documentation or source code which provides context for the suggested changes: <http://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-3.html>
Increase the version number in the header if appropriate.
If you are making substantial changes, consider adding a tslint.json containing { "extends": "dslint/dt.json" }.

TypeScript support default generic since version 2.3, So I add generic to collection. When create a collection by DB.collection()orDB.createCollection(), developer could specify a scheme, and when do some query, the result can be static analysd.

These changes don't break the old cold.

eg:

import { MongoClient } from 'mongodb'

interface User {
  name: string
  gender: number
}

interface SomeType {
  // ...
}


MongoClient.connect('...').then(DB => {
  // It's also support old code
  DB
    .collection('User')
    .find()
    .toArray()
    .then(arr => {
      arr.forEach(user => {
        // user: any
      })
    })

  DB
    .collection<User>('User')
    .find()
    .toArray()
    .then(arr => {
      arr.forEach(user => {
        console.log(user.gender) // user: User
      })
    })

  DB
    .collection<User>('User')
    .aggregate<SomeType>([])
    .toArray()
    .then(arr => {
      arr.forEach(item => {
        // item: SomeType
      })
    })

  DB
    .collection<User>('User')
    .findOne({})
    .then(user => {
      // user: User
    })

  DB
    .collection<User>('User')
    .findOne<SomeType>({}, { fields: {/* ... */ } }) // If pass fields option, could specify generic to `findOne()`
    .then(user => {
      // user: SomeType
    })
})

TypeScript support default generic since version 2.3, so add generic to Collection.

dt-bot · 2017-05-04T11:35:51Z

types/mongodb/index.d.ts

to author (@CaselIT). Could you review this PR?
👍 or 👎?

Checklist

pass the Travis CI test?

beary · 2017-05-04T13:19:15Z

I checked logs, it says there are some syntax error. But there is no problem on my code, so strange... 😢
My versions:
typescript: 2.3.2
vscode: code-insiders_1.12.0-1493823801_amd64.deb

# test logs
../mongodb/index.d.ts(184,24): error TS1005: ',' expected.
../mongodb/index.d.ts(185,24): error TS1005: ',' expected.
../mongodb/index.d.ts(186,24): error TS1005: ',' expected.
../mongodb/index.d.ts(195,30): error TS1005: ',' expected.
../mongodb/index.d.ts(196,30): error TS1005: ',' expected.
../mongodb/index.d.ts(197,30): error TS1005: ',' expected.
../mongodb/index.d.ts(239,30): error TS1005: ',' expected.
../mongodb/index.d.ts(240,30): error TS1005: ',' expected.
../mongodb/index.d.ts(241,30): error TS1005: ',' expected.
../mongodb/index.d.ts(424,17): error TS1005: ',' expected.
../mongodb/index.d.ts(425,17): error TS1005: ',' expected.
../mongodb/index.d.ts(466,12): error TS1005: ',' expected.
../mongodb/index.d.ts(469,15): error TS1005: ',' expected.
../mongodb/index.d.ts(470,15): error TS1005: ',' expected.

CaselIT · 2017-05-04T17:00:15Z

@beary the build fails because of microsoft/dtslint#31. Shortly DT will start supporting features that require the latest version of TS about a month after its release.

Otherwise LGTM 👍

beary · 2017-05-05T02:02:00Z

Thanks for your response @CaselIT

ghost

Needs a // TypeScript Version: 2.3 comment on line 5.

ghost · 2017-06-01T15:05:54Z

types/mongodb/index.d.ts

    /** @deprecated */
-    find(query: Object, fields?: Object, skip?: number, limit?: number, timeout?: number): Cursor<any>;
+    find<T = TSchema>(query: Object, fields?: Object, skip?: number, limit?: number, timeout?: number): Cursor<T>;


Is T ever not TSchema?

Yes, When fields parameter was passed in, the results would not have some properties which TSchema has. This is an old api http://mongodb.github.io/node-mongodb-native/1.4/api-generated/collection.html#find

Sorry, my expression was puzzled. Actually I means T always same as TSchema, but if developer pass fields parameter, they can specify T according their fields

yuit · 2017-06-06T17:44:26Z

@beary if you could add the // TypeScript Version: 2.3 it should resolve your build failure and I can promptly merge the PR. Thanks for your contribution!

ghost · 2017-06-07T02:01:04Z

types/mongodb/index.d.ts


 // Documentation : http://mongodb.github.io/node-mongodb-native/2.2/api/

+// TypeScript Version: 2.3


Old location was correct.
Packages that depend on mongo will also have to be updated to have this in their header. #16198 is doing this.

hinell · 2017-06-07T08:02:31Z

@beary I had the same problem when have tried to migrate mongodb module to 2.3 ts version.
Seems like if you want to solve this error you have to add ts 2.3 header to all these modules 😐 :

The following packages had errors: acl, agenda, connect-mongo, easy-jsend, express-brute-mongo,
gridfs-stream, mongoose, mongoose-auto-increment, mongoose-deep-populate, mongoose-mock,
mongoose-paginate, mongoose-promise, mongoose-sequence, passport-local-mongoose

CaselIT · 2017-06-07T08:08:20Z

@beary @hinell @andy-ms I'll be doing the same thing for this other PR #16198 where it's still missing a couple of definitions. Maybe its best to merge this after the other one, to avoid duplicating the work?

hinell · 2017-06-07T08:18:31Z

@CaselIT Are you going to do it manually? If so It is pretty a lot of job there. I think it would be more better or quicker if somebody would write script that automatically resets all the headers in specified modules.

Maybe its best to merge this after the other one, to avoid duplicating the work?

Yep, it is.

beary · 2017-06-07T08:27:38Z

@hinell Thanks, @CaselIT add // TypeScript Version 2.3 to all those modules on #16198 , I'll waiting for his PR was merged.

@CaselIT No problem. I can do my work after your PR was merged.

hinell · 2017-06-07T08:31:42Z

@CaselIT At first I didn't understand you. See my post above again pls. 😄

CaselIT · 2017-06-07T16:34:47Z

@hinell Yes I did it manually. It's not too much work

beary · 2017-06-08T13:07:46Z

@CaselIT I merged #16198 and resolved the conflicts, mind taking one more look 😉

CaselIT

Apart from the note I've left it looks good. 👍

CaselIT · 2017-06-08T19:04:15Z

types/mongodb/index.d.ts

@@ -743,9 +741,9 @@ export interface DeleteWriteOpResultObject {
 }

 //http://mongodb.github.io/node-mongodb-native/2.1/api/Collection.html#~findAndModifyWriteOpResult
-export interface FindAndModifyWriteOpResultObject {
+export interface FindAndModifyWriteOpResultObject<TSchema> {


I think it's better to set <TSchema=Default>

beary · 2017-06-07T02:31:30Z

types/mongodb/index.d.ts


 // Documentation : http://mongodb.github.io/node-mongodb-native/2.2/api/

+// TypeScript Version: 2.3


beary · 2017-06-08T02:55:08Z

types/mongodb/index.d.ts

@@ -443,7 +443,7 @@ export interface FSyncOptions {
 }

 // Documentation : http://mongodb.github.io/node-mongodb-native/2.1/api/Collection.html
-export interface Collection {
+export interface Collection<TSchema> {


Collection was created by DB.collection() and other methods of DB, So I add default type argument to those methods but Collection interface. But the test was failed, I'll and a Default type argument to Collection interface.

beary · 2017-06-09T01:51:23Z

@CaselIT Thanks for remind, default type argument for FindAndModifyWriteOpResultObject was added. I realize sometimes people would use the interface as a type annotations, the default type argument is necessary.

beary · 2017-06-11T04:23:19Z

@yuit Thank you!

beary added 3 commits May 4, 2017 18:13

Add default generic to Collection

920d2d7

TypeScript support default generic since version 2.3, so add generic to Collection.

Add generic to collection

22b3831

Add generic to findOneAnd*** functions

b0d31d8

mhegazy added the Revision needed This PR needs code changes before it can be merged. label May 4, 2017

ghost reviewed Jun 1, 2017

View reviewed changes

mhegazy added the TypeScript 2.3 label Jun 1, 2017

yuit added The Travis CI build failed and removed The Travis CI build failed labels Jun 6, 2017

beary added 2 commits June 7, 2017 09:40

Add "// TypeScript Version: 2.3"'

68153fb

Move ts version comment to end of the header

f421bc7

ghost reviewed Jun 7, 2017

View reviewed changes

Move ts version comment to correct location

e33c2ab

beary added 2 commits June 8, 2017 10:47

Merge branch 'master' into master

a511933

Add default type argument to Collection interface

1f3527b

CaselIT reviewed Jun 8, 2017

View reviewed changes

beary commented Jun 9, 2017

View reviewed changes

Add default generic to FindAndModify... interface

424873b

yuit merged commit 15ef67b into DefinitelyTyped:master Jun 11, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add generic support to Collection and it's methods #16315

Add generic support to Collection and it's methods #16315

beary commented May 4, 2017 •

edited

dt-bot commented May 4, 2017

beary commented May 4, 2017

CaselIT commented May 4, 2017

beary commented May 5, 2017

ghost left a comment

ghost Jun 1, 2017

beary Jun 1, 2017

beary Jun 4, 2017

yuit commented Jun 6, 2017

ghost Jun 7, 2017

beary Jun 7, 2017

hinell commented Jun 7, 2017 •

edited

CaselIT commented Jun 7, 2017 •

edited

hinell commented Jun 7, 2017 •

edited

beary commented Jun 7, 2017

hinell commented Jun 7, 2017

CaselIT commented Jun 7, 2017

beary commented Jun 8, 2017

CaselIT left a comment

CaselIT Jun 8, 2017

beary Jun 7, 2017

beary Jun 8, 2017

beary commented Jun 9, 2017

beary commented Jun 11, 2017


		// Documentation : http://mongodb.github.io/node-mongodb-native/2.2/api/

		// TypeScript Version: 2.3

Add generic support to Collection and it's methods #16315

Add generic support to Collection and it's methods #16315

Conversation

beary commented May 4, 2017 • edited

dt-bot commented May 4, 2017

beary commented May 4, 2017

CaselIT commented May 4, 2017

beary commented May 5, 2017

ghost left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuit commented Jun 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hinell commented Jun 7, 2017 • edited

CaselIT commented Jun 7, 2017 • edited

hinell commented Jun 7, 2017 • edited

beary commented Jun 7, 2017

hinell commented Jun 7, 2017

CaselIT commented Jun 7, 2017

beary commented Jun 8, 2017

CaselIT left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

beary commented Jun 9, 2017

beary commented Jun 11, 2017

beary commented May 4, 2017 •

edited

hinell commented Jun 7, 2017 •

edited

CaselIT commented Jun 7, 2017 •

edited

hinell commented Jun 7, 2017 •

edited