Automattic / mongoose

MongoDB object modeling designed to work in an asynchronous environment.
https://mongoosejs.com
MIT License
26.88k stars 3.83k forks source link

Bug in Typescript definitions for Schema.index() #10561

Closed JaredReisinger closed 3 years ago

JaredReisinger commented 3 years ago

Do you want to request a feature or report a bug? bug (in Typescript types)

What is the current behavior? The definition of Schema.index() (at https://github.com/Automattic/mongoose/blob/master/index.d.ts#L1279-L1280) says that the fields argument is of type mongodb.IndexSpecification, but mongoose actually takes its own shorthand syntax for the indexed fields, namely an object containing the indexed fields and the sort order (see https://mongoosejs.com/docs/api/schema.html#schema_Schema-index and https://github.com/Automattic/mongoose/blob/master/lib/schema.js#L1549-L1560).

If the current behavior is a bug, please provide the steps to reproduce.

This is as easy as the following schema definition (as a .ts file):

import mongoose from "mongoose";

export const schema = new mongoose.Schema({ whatever: String });

schema.index({ whatever: 1 });

... you'll get an error on the whatever: 1 in the schema.index() call, stating:

error TS2345: Argument of type '{ whatever: number; }' is not assignable to parameter of type 'IndexSpecification'.

Here's the tsconfig.json file, though it shouldn't matter much:

{
  "extends": "@tsconfig/node14/tsconfig.json",

  "compilerOptions": {
    "rootDir": "./",
    "outDir": "./dist",

    "allowJs": true,
    "checkJs": false,

    "typeRoots": [
      "@types",
      "node_modules/@types"
    ]
  },

  "include": ["**/*"]
}

What is the expected behavior? No error, the fields argument to schema.index() should probably have a type definition similar to SchemaDefinition<>/DocumentDefinition<> that leverages the SchemaDefinitionType to define the allowable keys. Maybe something like

export type IndexDefinition<T> = { [K in keyof T]: 1 | -1 };
// ...

/** Defines an index (most likely compound) for this schema. */
index(fields: IndexDefinition<DocumentDefinition<SchemaDefinitionType>>>, options?: IndexOptions): this;

(The SchemaDefinition<> type might need to be in that chain as well... it's not clear to me what specific value that layer adds.)

What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version. Node.js: 14.16.0 mongoose: 5.13.6 mongodb: irrelevant

JaredReisinger commented 3 years ago

Wow... this was apparently just broken in commit https://github.com/Automattic/mongoose/commit/1c66cc972c90529bd0ffe8496e9de866b92a97fa ("use mongodb's index specification")

thiagokisaki commented 3 years ago

The author of #10538 probably intended to use mongodb driver's IndexSpecification introduced in v4.0.0 of the package (mongodb driver's codebase was rewritten in TypeScript):

Screenshot from 2021-08-09 20-25-28

But the problem is that mongoose v5.x still relies on older versions of mongodb and @types/mongodb which has a different definition for IndexSpecification: Screenshot from 2021-08-09 20-26-48

JaredReisinger commented 3 years ago

Ahhh.... I bet you're right. That's quite a conundrum... Did mongo actually change their API for this (createIndex()), or did they just tweak the type definitions?

thiagokisaki commented 3 years ago

I think they just tweaked the types.

lucasltv commented 3 years ago

Even changing the type to native mongdb, it's not creating the indexes.

Example:

Code: back-office-user model ts — api-v1 2021-08-11 at 10 16 31 AM

Mongoose creating index log. Note that this is creating the index by another way than the field scoped indexes: back-office-user model ts — api-v1 2021-08-11 at 10 17 47 AM

Not reflecting at mongodb: NoSQLBooster for MongoDB 2021-08-11 at 10 18 41 AM

lucasltv commented 3 years ago

For now, reverting to "mongoose": "5.13.5" is working for me.