Improve Connection Typings #11418
Improve Connection Typings #11418
Conversation
| collection?: string, | ||
| options?: CompileModelOptions | ||
| ): U; | ||
| model<T>(name: string, schema?: Schema<T>, collection?: string, options?: CompileModelOptions): Model<T>; |
There was a problem hiding this comment.
Question to all:
shouldnt we do here:
model<T extends AnyObject = AnyObject>
With the current typings we are forcing a dev to type .model(). I think better DX is to make it optional for the dev.
| * and [transactions](http://thecodebarbarian.com/a-node-js-perspective-on-mongodb-4-transactions.html). | ||
| */ | ||
| startSession(options: mongodb.ClientSessionOptions | undefined | null, callback: Callback<mongodb.ClientSession>): void; | ||
| startSession(callback: Callback<mongodb.ClientSession>): void; |
There was a problem hiding this comment.
I added this, because in startSession we check if first argument is function and if so use it as callback.
| */ | ||
| readonly readyState: ConnectionStates; | ||
|
|
||
| /** Sets the value of the option `key`. */ |
There was a problem hiding this comment.
here we had the information that set is the same as connection.options[key] = value.
The typings do not expose the options attribute, so I removed this remark here and the same for get
| * - 3 = disconnecting | ||
| * - 99 = uninitialized | ||
| */ | ||
| readonly readyState: ConnectionStates; |
There was a problem hiding this comment.
using connectionStates instead of number
| * The host name portion of the URI. If multiple hosts, such as a replica set, | ||
| * this will contain the first host name in the URI | ||
| */ | ||
| readonly host: string; |
There was a problem hiding this comment.
please give me feedback regarding the readonly on host, user, pass etc.
Is it wrong? Is there actually a use case for having them writeable?
If I am wrong, then I would like to fix it before we have regression reports
There was a problem hiding this comment.
I think readonly should be fine, modifying host is a noop.
| schema?: Schema<T, U, TQueryHelpers>, | ||
| collection?: string, | ||
| options?: CompileModelOptions | ||
| ): U; |
There was a problem hiding this comment.
ReturnType U seems odd. Should it be Model or does U should extend from Model?
There was a problem hiding this comment.
U should extend from Model, but I believe we removed that for perf reasons in #10349. U extends Model causes perf issues for the TS compiler
This PR extracts the Connection Typings
With the first commit I extract the typings from Connection from index.d.ts into a new Connection.d.ts.
With the second commit I do the actual improvements.
The order of the function overloads are according to Typescript Do's and Don'ts, so basically by most arity to least arity.
Attributes like pass, user, models etc. were set to readonly. nobody should use them directly for manipulating the internals of mongoose. If there is some reason to modify them, a developer should use the corresponding methods.
Just check the second commit to see relevant changes.
Also added typings tests.