-
Notifications
You must be signed in to change notification settings - Fork 73
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
Updates to NestJS v8 Fixes #433 #434
base: master
Are you sure you want to change the base?
Conversation
Hey, sorry for the late reply. I will try to get this reviewed over the weekend. |
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 have a few questions - I also need to fix the test runner.
src/typegoose-core.module.ts
Outdated
@@ -66,7 +66,7 @@ export class TypegooseCoreModule implements OnApplicationShutdown { | |||
} = typegooseModuleOptions; | |||
return mongoose.createConnection(uri, typegooseOptions); | |||
}, | |||
inject: [TYPEGOOSE_MODULE_OPTIONS] // inject output of async config creator | |||
inject: [ TYPEGOOSE_MODULE_OPTIONS ] // inject output of async config creator |
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 these formatting changes from prettier, if not I might rerun it?
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.
Yeah. I'll fix those. Not even sure how it happened tbh.
Scott
@smolinari Do you need a re-review? |
I will. Typegoose v8 just went final. I need to update that dep. Edit. Ok. Updated. 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.
LGTM - assuming tests are green.
Hey Kyle, I missed your "any updates" and to be honest, I'm not sure anymore where this stands. Aaaannnd, Mongoose has moved to version 6 and with it there are breaking changes. What should we do? Or what can I do? I'm going to work on fixing the current issue with mongoose 6. But, what else should I do? And should I do it through another PR, or continue with this one? I'd imagine, this package would also need to be moved up to 8.0? Scott |
Forgot to ping you above. @kpfromer Scott |
Hey @smolinari @kpfromer, any updates on this PR? |
@duongdev - I've just created my own fork. I haven't updated it in a few months though either. So, also not up-to-date. But, it should be working. Let me know if you have any issues on my fork in Github. https://www.npmjs.com/package/@m8a/nestjs-typegoose Scott |
Hi @smolinari your repo show a 404 on Github. Is it private ? |
@GwendalBroudin-Emoko - No. It's just somewhere else. I've updated the package and also the repo link (sort of). Scott |
I hope this is ok. I'm not the most experienced at this. Any critique (and I'm sure there will be some) is welcome. I'm learning.
Fixes #433
Scott