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(modal): set strong typing in componentInstance getter property #2815
feat(modal): set strong typing in componentInstance getter property #2815
Conversation
@benouat Sorry to bother, any updates on this? |
Any news on this? |
Hello, |
I've even forgotten about this. |
@michaeljota ha, we've forgotten too apparently :) |
@maxokorokov Do you need me to update this? |
@michaeljota if it's not too much trouble to:
I'll merge this in the next release. |
Sure. I will do. |
749df8d
to
ffc129e
Compare
@maxokorokov Done. Thank you. |
ffc129e
to
a4e5a60
Compare
Codecov Report
@@ Coverage Diff @@
## master #2815 +/- ##
=======================================
Coverage 91.07% 91.07%
=======================================
Files 95 95
Lines 2780 2780
Branches 516 516
=======================================
Hits 2532 2532
Misses 189 189
Partials 59 59
Continue to review full report at Codecov.
|
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.
@michaeljota thanks!
I just rolled back your changes in the modal-stack.ts
, as they're not really necessary.
Thanks. |
…ootstrap#2815) This reverts commit f450a7c Fixes ng-bootstrap#3464
Before submitting a pull request, please make sure you have at least performed the following:
This should be mostly compatible with current branch. As latest version of this use Angular 6.1 and Angular itself uses TS2.9, I take the liberty to update those dependencies to latest of each branch to be able to use conditional types.
Using conditional types I manage to strong type the
componentInstance
property only when you open a Component. However by using default types, developers should have the same experience that they currently have, meaning, thecomponentInstance
property is default to any. Regardless that, this could have some breaking scenarios, but please notice that those scenarios should not be working on the first place as this change only typing, and not behavior.Fix: #2479