-
-
Notifications
You must be signed in to change notification settings - Fork 147
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
Bugfix/179 instance name used to find dep get it dep class #181
Bugfix/179 instance name used to find dep get it dep class #181
Conversation
Please note that this change is not retro compatible. |
Hmm, give me a day to ponder about that :-) |
Hi @escamoteur, probably I've found a way to make it retro-compatible. Can you check it, please? |
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.
thanks a lot for this contribution, I added some comments
…ix/179-instanceName-used-to-find-dep-getItDep-class
README.md
Outdated
@@ -538,8 +538,8 @@ factories/singletons that were registered by name. | |||
class RestService1 implements RestService{} | |||
class RestService2 implements RestService{} | |||
|
|||
getIt.registerSingletonAsync<RestService>(() async => RestService1().init(), instanceName : "restService1"); | |||
getIt.registerSingletonAsync<RestService>(() async => RestService2().init(), instanceName : "restService2"); |
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.
Why did you remove the init()?
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.
Hi, is necessary for this example? if I understood properly the init
method is to "simulate" an operation that takes time, but in this case, I don't think is needed.
Anyway, I can add it again if you want.
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.
No, it is used because any asnycRegistration expects an async factory function which your solution is because is uses the async
keyword, but there is no such thing as an async class constructor that's why it would be a bad example.
the init function to be used like that has to return this
to be usable like that
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 will fix 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.
In the readme we currently have: Automatic using
|
I don't know if you follow me on Twitter but I just tweeted some thoughts of new functionality for get_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.
Awesome, thanks a lot!!!
Thanks to you @escamoteur your job with this lib is awesome. |
Have you tried the get_it_mixin? |
No, I didn't. |
Created a dedicated class to manage the dependency (type + instanceName)