search

justinfagnani / dado

An experimental dependency injection container for Dart
BSD 3-Clause "New" or "Revised" License
9 stars 6 forks source link

Initial implementation of inject.dart support (BindingAnnotation) #8

Closed Dreckr closed 11 years ago

Dreckr commented 11 years ago

This patch makes dado use inject's BindingAnnotation to recognize metadata that define bindings (now only BindingAnnotation metadata is used by dado as opposed to any metadata). Named class has been included as an easy to use BindingAnnotation.

Probably you guys have already done this internally, but in case you didn't here it is.

Actually, I made this to ask you if I may contribute to this project. May I proceed with the implementation of @inject?

sethladd commented 11 years ago

Hi Dreckr, Can you sign the CLA? We need your sig on record before we can merge any code. https://developers.google.com/open-source/cla/individual

The authors of this package still need to review this code, but it doesn't hurt to get started on the CLA. Thanks!

Dreckr commented 11 years ago

Hey, Seth. Done! Should I wait for an email confirmation?

sethladd commented 11 years ago

I don't think you'll get any email response. Thanks for signing!

justinfagnani commented 11 years ago

Thanks for the pull request Diego!

This is obviously along the lines of what I was thinking when I made the inject package, but I've had a couple doubts since then. Mainly, since annotations are so easy to create in Dart, I'm not sure it's a good idea to force binding annotations to implement an interface. That'll prevent using Strings or existing annotations as binding annotations.

One possible solution is to pull in all the annotations and instead of just constructing a Key based on all the annotations and looking for an exact match, search through the registered Keys looking for the best match. Match could be defined as a Key with all annotations present at the inject site, and ties are broken by the larger number of annotations in the key. This has some downsides where if there's a more generic binding, say, non-annotated, and an annotated binding, then any inject sites will be satisfied with the generic binding even if they have what should be binding annotations. In Guice there would be an error, and that's probably helpful.

Here's this thing right now though, that makes this all academic: We can't access parameter metadata annotations via mirrors right now, so there's no good way to use binding annotations no matter what solution we choose. Injector.getInstance() works, but that should rarely be used. The relevant bug is http://dartbug.com/11418

So... I'm not sure about his pull request. The code looks good though.

But to answer your question about contributing, the answer is definitely yes! @inject would be a good one to solve. As would method injection. Interested?

Cheers! Justin

Dreckr commented 11 years ago

Got it. Thanks for answering so fast. I'll close this for now.

May I talk to you on Hangout so we can discuss some solution? Also, method injection would be really useful, when I have some free time I'll take a look at it.

justinfagnani commented 11 years ago

Yes, let's schedule a hangout next week. Also, let's detail method injection over on its issue before implementing it. Thanks Diego!