nestjs / mongoose

Mongoose module for Nest framework (node.js) 🍸
https://nestjs.com
MIT License
528 stars 118 forks source link

Throw error on invalid @Prop typings or converting them implictly #656

Closed amirqasemi74 closed 3 years ago

amirqasemi74 commented 3 years ago

I'm submitting a...


[ ] Regression 
[ ] Bug report
[*] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Today I was working with this library and encountered a bug in my code. it took a complete day to solve it. that error started to happen when i upgraded to @nestjs/mongoose@7.1.0 from @nestjs/mongoose@7.0.2. @kamilmysliwiec you have fixed a bug in version @nestjs/mongoose@7.0.3 which caused to Mongo throw error due my incorrect usage of @Prop typings.

the change link: https://github.com/nestjs/mongoose/commit/8868c08331dadd7daf9c0a54fbab5fad518f15e9#diff-ecba12c3cf4c1a852167bbececea7cf59ed3566f7e618020d292d1a5cf83d6b4R31.

The error which mongo was showing to me was not good and does not relevant to my bad usage of @Props. But i think we can improve usability of @Props by Throw Error on bad explicit typing or converting types.

My code with bad usage of @Prop

@Schema()
class Salary {
    @Prop()
     type: string
}

const SalarySchema = SchemaFactory.createForClass(Salary)

class Candidate {
    @Prop(SalarySchema)
     proposedSalay: Salary;
}

the right one is:

@Schema()
class Salary {
    @Prop()
     type: string
}

const SalarySchema = SchemaFactory.createForClass(Salary)

class Candidate {
    @Prop({type: SalarySchema})
     proposedSalay: Salary;
}

So i think on @Prop(SalarySchema) library should throw error or convert in to @Prop({type: SalarySchema}) for sub-schema s in mongo...

also the current code of Prop decorator doesn't handle this usage

// wrong
@Schema()
class Salary {
    @Prop(String)
     type: string | null
}

// correct
@Schema()
class Salary {
    @Prop({type: String})
     type: string | null
}
kamilmysliwiec commented 3 years ago

Fixed in 7.1.1