Automattic / mongoose

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

Typescript Issues - How do we report issues and move forward #9620

Closed thaoula closed 3 years ago

thaoula commented 3 years ago

Hi @vkarpov15,

I mentioned yesterday on #9606 that after upgrading to the latest version of mongoose i have over 230 compilations errors due the new typescript definition.

We use mongoose to define the schema and indexes and are mostly using lean(), find* and aggregation to get the data. We just want the JSON and do not really use mongoose methods for saving etc.

I have created a summary of the issues we experiencing with the new typescript definition. I am sorry I removed your issue template just wasn't sure how you wanted these issues reported.

At the moment we are forced to downgrade to the previous version of mongoose and have to revert to using @types/mongoose otherwise we cannot build the application.

I am sure you will be received quite a lot of issues regarding typescript over the next few days. So

Summary of Issues

Missing / Incorrect types:

**Missing / Broken Methods and Properties**
- Cannot use set on mongoose (import * as mongoose from 'mongoose';) -  
`Property 'set' does not exist on type 'typeof import("mongoose")'`
- findById: 
`Property 'findById' does not exist on type 'Model<Document>'`
- deleteMany:
` Property 'deleteMany' does not exist on type 'JobQueueJobModel' which is a Model`
- deleteOne
`Property 'deleteOne' does not exist on type 'Model<ISchedulerRunSheetDocument>'`
- create: No overload matches this call.

const deadJob = await this.deadQueue.create({ payload: job.payload, tries: job.tries});

- collection no longer available on document - 
`Property 'collection' does not exist on type 'Model<Document>'`
- Model.findOneAndUpdate: 
`Type '{ new: boolean; select: { workOrders: { $elemMatch: { _id: string; }; }; site: number; }; arrayFilters: { 'order._id': { $eq: string; }; }[]; }' has no properties in common with type **'QueryOptions'.**`
- find(query) seems to only return a single record. 

Previously we could do this -
    const existing = await this.document
        .find(query)
        .lean()
        .exec() as TimesheetEntry[];

Now we get the following error - 

`Conversion of type 'Pick<Pick<_LeanDocument<ITimesheetEntryDocument>, "_id" | "active" | "createdBy" | "createdAt" | "updatedAt" | "status" | "jobId" | "source" | ... 8 more ... | "updatedBy">, "_id" | ... 15 more ... | "updatedBy">' to type 'TimesheetEntry[]' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first`

**Missing Options**
- Object literal may only specify known properties, and 'arrayFilters' does not exist in type 'QueryOptions'
`arrayFilters: [{ 'elem.document._id': { $eq: document._id } }]`
- Type '{ new: boolean; }' has no properties in common with type 'QueryOptions'

Thanks
Tarek
UncleVic commented 3 years ago

I don't understand why you don't like strong typings. In last version 5.11 we've include index.d.ts to inside package. Most of types declared as any. That case break hole idea of strong typings in TypeScript.

For example Schema & SchemaDefenition: If use the generics, you can control schema's params by a type. constructor(definition?: SchemaDefinition<R>, options?: SchemaOptions); type SchemaDefinition<T = any> = { [P in keyof T]-?: SchemaTypeOpts<T[P]> | Schema | SchemaType; };

It solution doesn't brake previous users cod and get strong types for using later.

Can you don't include types definition to mongoose package? It'll be better for TypeScripts developers. Or... Yon can spend more time and create correct types definition where any will be used where it need not anywhere

thaoula commented 3 years ago

Hi @UncleVic,

Are you responding to me? If so, I am sure I did not say I did not like strong typings. What I did say was that I was using a previous version of mongoose and the @types/mongoose typescript definitions.

Our codebase has many strongly typed Mongoose models and a lots of code that is working just fine on the previous version.

Now with 5.11 we have built-in typings that are not compatible / different to @types/mongoose. Code, that previously compiled just fine, now generates 232 errors.

So like many others .. we have been forced to downgrade so that we can continue using mongoose and @types/mongoose. Otherwise we cannot build our apps and we cannot keep developing.

Furthermore, if everyone just downgraded then no one can help fix these issues.

Finally, it's not clear which errors are bugs in the type definitions and which errors require refactoring. According to the docs many of the items above should not causes errors.

I just summarised the 10 or so items that are the bulk of our 232 compile errors and then downgraded to continue working.

Also, my suggestion to seperate the typing from the project (Temporarily) is to allow people to upgrade to 5.11 (which may contain bugs and features people needs) whilst allow teams like ours to continue using @types/mongoose. At least until we can collectively resolve these typing issues.

Regards, Tarek

acrilex1 commented 3 years ago

Hello,

I just opened an issue on DefinitelyTyped in order to move forward and phase-out or make the types compatible. See https://github.com/DefinitelyTyped/DefinitelyTyped/issues/49902.

One of the big problems here is the fact that types were not back-ported to v3 and v4. Therefore, the package in @types cannot simply be added in notNeededPackages.json.

huineng commented 3 years ago

I took the hard pain and updated as much as i could. Some of the issues are already being fixed . My real concern is the part mentioned above

Conversion of type 'Pick<Pick<_LeanDocument<ITimesheetEntryDocument>, "_id" | "active" | "createdBy" | "createdAt" | "updatedAt" | "status" | "jobId" | "source" | ... 8 more ... | "updatedBy">, "_id" | ... 15 more ... | "updatedBy">' to type 'TimesheetEntry[]' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first

for this i don't find an easy solution where it was very simply yesterday

const doc: IConfigmapSchema | null = await ConfigmapsModel.findOne({ selector }, {}).lean().exec();
vkarpov15 commented 3 years ago

@huineng thanks for the detailed list of issues, feel free to report any others that come up in a similar manner.

Regarding why const doc: IConfigmapSchema | null = await ConfigmapsModel.findOne({ selector }, {}).lean().exec(); no longer works correctly, we added a notion of a LeanDocument so we didn't just return any from a lean query. LeanDocument picks all the properties from ITimesheetEntryDocument that aren't in Document. What's the relationship between IConfigmapSchema and ITimesheetEntryDocument in your code?

huineng commented 3 years ago

well for some reason with the 5.11.2 you just pushed , i don't see that error anymore on findOne, so that's good news, but not sure why it now works again

i do still have a problem with find , i tried to add LeanDocument but it's an array

const d1: LeanDocument<IConnSchema> = await ConnModel.find(filter)
            .collation({ locale: 'en' }) // insensitive sorting
            .skip(body.first || 0)
            .limit(body.rows || 0)
            .sort({ alias: 1 })
            .lean()
            .exec();

and without it i get (const d1: IConnSchema[] = await ConnModel.find(filter)

Type 'Pick<Pick<_LeanDocument<IConnModel>, "_id" | "alias" | "annotations" | "cert" | "description" | "jdbc" | "owner" | "password" | "project" | "idtype" | "type" | "user" | "expires">, "_id" | ... 11 more ... | "expires">' is missing the following properties from type 'IConnSchema[]': length, pop, push, concat, and 31 more.ts(2740)

so i'm not sure how to handle document arrays

vkarpov15 commented 3 years ago

Try LeanDocument<IConnSchema>[] ?

huineng commented 3 years ago

thanks, many combinations tried, the problem is when adding lean() with find

const d1: LeanDocument<IConnSchema[]>

yields

const d1: Pick<Pick<IConnSchema[], number | "length" | "toString" | "toLocaleString" | "pop" | "push" | "concat" | "join" | "reverse" | "shift" | "slice" | "sort" | "splice" | "unshift" | ... 21 more ... | "turn">, "length" | ... 33 more ... | "turn">
Type 'Pick<Pick<_LeanDocument<IConnModel>, "_id" | "alias" | "annotations" | "cert" | "description" | "jdbc" | "owner" | "password" | "project" | "idtype" | "type" | "user" | "expires">, "_id" | ... 11 more ... | "expires">' is missing the following properties from type 'Pick<Pick<IConnSchema[], number | "length" | "toString" | "toLocaleString" | "pop" | "push" | "concat" | "join" | "reverse" | "shift" | "slice" | "sort" | "splice" | "unshift" | ... 21 more ... | "turn">, "length" | ... 33 more ... | "turn">': length, pop, push, concat, and 29 more.
const d1: LeanDocument<IConnSchema>[]
onst d1: Pick<Pick<_LeanDocument<IConnSchema>, "user" | "_id" | "alias" | "annotations" | "cert" | "description" | "jdbc" | "owner" | "password" | "project" | "idtype" | "type" | "expires">, "user" | ... 11 more ... | "expires">[]
Type 'Pick<Pick<_LeanDocument<IConnModel>, "_id" | "alias" | "annotations" | "cert" | "description" | "jdbc" | "owner" | "password" | "project" | "idtype" | "type" | "user" | "expires">, "_id" | ... 11 more ... | "expires">' is missing the following properties from type 'Pick<Pick<_LeanDocument<IConnSchema>, "_id" | "alias" | "annotations" | "cert" | "description" | "jdbc" | "owner" | "password" | "project" | "idtype" | "type" | "user" | "expires">, "_id" | ... 11 more ... | "expires">[]': length, pop, push, concat, and 31 more.ts(2740)

this works but without lean

const d1: IConnSchema[] = await ConnModel.find(filter)
            .collation({ locale: 'en' }) // insensitive sorting
            .skip(body.first || 0)
            .limit(body.rows || 0)
            .sort({ alias: 1 })
            .exec();
vkarpov15 commented 3 years ago

@huineng this should be fixed by 1bc2de836fdba733caf5518a5dca2340421ccdfc, check out the test example: https://github.com/Automattic/mongoose/commit/1bc2de836fdba733caf5518a5dca2340421ccdfc#diff-494edc8f12da8cc9e2e7e82778d4d5f05e39131258fe20dae5bc3dd4de7d4d04 .

I'll close this issue, please open a new issue if you have more trouble :+1;

huineng commented 3 years ago

thanks, but this is not published yet ... any idea when i can see this in a release ?

vkarpov15 commented 3 years ago

v5.11.3 was just shipped, let me know if you have any more issues :+1:

huineng commented 3 years ago

all good..thanks !