Skip to content
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

Conversation

n3wtron
Copy link
Contributor

@n3wtron n3wtron commented Apr 24, 2021

Created a dedicated class to manage the dependency (type + instanceName)

@n3wtron
Copy link
Contributor Author

n3wtron commented Apr 24, 2021

Please note that this change is not retro compatible.
dependsOn attribute now need a GetItDep class instead of a Type

@escamoteur
Copy link
Collaborator

Hmm, give me a day to ponder about that :-)

@n3wtron
Copy link
Contributor Author

n3wtron commented Apr 26, 2021

Hi @escamoteur, probably I've found a way to make it retro-compatible. Can you check it, please?

Copy link
Collaborator

@escamoteur escamoteur left a 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

lib/get_it.dart Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
test/async_test.dart Outdated Show resolved Hide resolved
@n3wtron n3wtron closed this Apr 29, 2021
@n3wtron n3wtron reopened this Apr 29, 2021
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");
Copy link
Collaborator

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()?

Copy link
Contributor Author

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.

Copy link
Collaborator

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will fix it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@n3wtron n3wtron requested a review from escamoteur April 30, 2021 16:48
@escamoteur
Copy link
Collaborator

In the readme we currently have:

Automatic using dependsOn

In case that this services have to be initialized in a certain order because they depend on that other services are already ready to be used you can use the dependsOn parameter of registerFactoryAsync. If you have a non async Singleton that depends on other Singletons, there is registerSingletonWithDependencies. In the following example, DbService depends on ConfigService, and AppModel depends on ConfigService and RestService

  getIt.registerSingletonAsync<ConfigService>(() async {
    final configService = ConfigService();
    await configService.init();
    return configService;
  });
  getIt.registerSingletonAsync<RestService>(() async => RestService().init());
  getIt.registerSingletonAsync<DbService>(createDbServiceAsync,
      dependsOn: [ConfigService]);
  getIt.registerSingletonWithDependencies<AppModel>(
      () => AppModelImplmentation(),
      dependsOn: [ConfigService, DbService, RestService]);

When using dependsOn you ensure that the registration waits with creating its singleton on the completion of the type defined in dependsOn.

could you add there how to use the new InitDependency ?

@escamoteur
Copy link
Collaborator

I don't know if you follow me on Twitter but I just tweeted some thoughts of new functionality for get_it:
https://twitter.com/ThomasBurkhartB/status/1388539107756883971?s=20
What do you think?

Copy link
Collaborator

@escamoteur escamoteur left a 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!!!

@escamoteur escamoteur merged commit 15944ac into fluttercommunity:null_safety May 1, 2021
@n3wtron
Copy link
Contributor Author

n3wtron commented May 1, 2021

Thanks to you @escamoteur your job with this lib is awesome.

@escamoteur
Copy link
Collaborator

Have you tried the get_it_mixin?

@n3wtron
Copy link
Contributor Author

n3wtron commented May 1, 2021

No, I didn't.
I will take a look and try it.
thank you

@n3wtron n3wtron deleted the bugfix/179-instanceName-used-to-find-dep-getItDep-class branch May 2, 2021 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants