feat: alllow specifying directives as bindings#1498
feat: alllow specifying directives as bindings#1498pkozlowski-opensource wants to merge 1 commit into
Conversation
|
@tbosch would you mind reviewing this one for me? |
There was a problem hiding this comment.
Can you add a tests where you create a binding such as: bind(PublicAPI).toClass(PrivateImpl) and then have a directive which injects
class SomeDecorator {
constructor(@Parent() api:PublicAP) {
expect(api instanceof PrivateImpl).toBe(true);
}
}<div public-api>
<div child-decorator></div>
</div>583fc07 to
75406a7
Compare
75406a7 to
444008a
Compare
|
@mhevery I've added a test you've been mentioning but while doing so I've realized that this is probably not representing the use-case described by @ajoslin . There are 2 differences I can see:
@ajoslin could you please verify if this commit fixes your use-case or not? |
|
@pkozlowski-opensource Can we merge this anyways? I need it for another issue... |
|
@tbosch merging this won't break anything so in this sense I think we can. But I still believe that this doesn't address the original issue and might be confusing (we read meta-data from one class and use a different class as impl, even if impl has different meta-data). I would say that we can merge this if we don't mind reviewing the original issue / introducing breaking changes in the near future. |
|
Ok, merged, but leaving the original issue open so that we don't forget... |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Closes #709