draft: fix(common): change configurable module class type to support promise
PR Checklist
Please check if your PR fulfills the following requirements:
- [X] The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
- [X] Tests for the changes have been added (for bug fixes / features)
- [X] Docs have been added / updated (for bug fixes / features)
PR Type
What kind of change does this PR introduce?
- [X] Bugfix
- [ ] Feature
- [ ] Code style update (formatting, local variables)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] CI related changes
- [ ] Other... Please describe:
What is the current behavior?
Configurable modules built with the ConfigurableModuleBuilder can't have async register or registerAsync methods
Issue Number: #10226
What is the new behavior?
Configurable modules built with the ConfigurableModuleBuilder can have async register or registerAsync methods
Does this PR introduce a breaking change?
- [ ] Yes
- [X] No
Other information
Ah, shoot, this gets more tricky when it comes to spreading the result. I'll need to play around with this more to see if we can get this extension to really work. I think there should be a way to do this, make the function optionally async, but it's gonna be weird on the Typescript side of things
Strange that it fails the build was not modified this file:
🤔
@Tony133 is makes sense because we use a ...super.forRoot() and ...super.forRootAsync() and Typescript isn't able to immediately guarantee that the return of T | Promise<T> is not going to be that Promise<T>. We know better, because we aren't doing anything async in the function, but if we want to support the possibility of the methods returning a Promise<T> then we need to figure out some way to appease Typescript here. I'd rather not resort to an as T assertion, but that may be the solution.
If that is the solution though, this could be seen as a breaking change, because if anyone else is already doing that same kind of spread then their build will fail when we publish this change.
Very interesting, thank you very much for the explanation 👍
Is this PR ready for review 🤔 ? @jmcdo29
No. There's still a type issue that I'm trying to work back out