-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add typegoose recipe #2572
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really only one minor addition, docs look good otherwise :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
To begin, you'll need to install the following dependencies into your [already created NestJS app](/first-steps): | ||
|
||
```typescript | ||
$ npm install --save mongoose @typegoose/typegoose @m8a/nestjs-typegoose |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure we want to link a library that has ~500 weekly downloads in the docs 🤔 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kamilmysliwiec - It's a chicken and egg dilemma. 😁
The @typegoose/typegoose package has 79k downloads a week. That's the popularity we'd want to possibly hook into. And vice-versa of course.
Typegoose is a very good alternative for using Mongoose. The Typegoose package is very well maintained and documented and I take care of the nestjs-typegoose module updates now too.
If more people see the recipe, the more the module will be downloaded. 😉
We've also been promoting Typegoose (mostly me) in the Discord server and the database forum on Discord has had the tag "mongoose/ typegoose" for several months now.
And, if I may be so bold, the Nest team could theoretically take over the nestjs-typegoose module and drop the Mongoose module altogether (sunset it). It would be somewhat less effort from a support standpoint (although I'd still do the support in Discord) because Typegoose's maintainer has his own Discord server. The Typegoose docs are more detailed too. 😀 Just an idea.....
Scott
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Theoretically this recipe could replace the Mongoose recipe too. It's outdated.
https://docs.nestjs.com/recipes/mongodb
Scott
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trying to reinvigorate the decision to get this merged, the actual popularity we'd want to be jumping on is the 2 million downloads a week for Mongoose. Typegoose is a better (I'd say the best) TypeScript wrapper for Mongoose. The developer of Typegoose is really sharp and he even helps fix Mongoose TypeScript issues, when they pop up. The package can help promote Mongoose usage with Nest for sure! 😀
Scott
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd also like to put in that I've been helping in the forum (Discord) a lot and I'm also willing to help continue to support this package, if it became THE official suggested module for using Mongoose with Nest. That means, I'd suggest even taking down the Nest mongoose module. That would save you all time and effort, by putting the support on the community and get Nest users something they can be very successful with using Mongoose with Nest!
Scott
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
N/A
Issue Number: N/A
What is the new behavior?
N/A
Does this PR introduce a breaking change?
Other information
Merry Christmas! 🎄 😁
Scott