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

Update get_it_impl.dart to dispose in reverse order #350

Merged
merged 3 commits into from
Jan 6, 2024

Conversation

bvoq
Copy link
Contributor

@bvoq bvoq commented Nov 21, 2023

There are a few situations, where one instance depends on an instance you created before.
In all of these situations, it makes more sense to dispose of the widgets in reverse order.

lib/get_it_impl.dart Show resolved Hide resolved
@escamoteur
Copy link
Collaborator

escamoteur commented Nov 22, 2023 via email

@bvoq
Copy link
Contributor Author

bvoq commented Jan 3, 2024

Alright I've added tests, one of which fails with the current version but not with this new version.

Minor note: I wouldn't rename the type of factoriesByName, since:
final factoriesByName = <String?, Map<Type, _ServiceFactory<Object, dynamic, dynamic>>>{}; is of type:

Map<String?, Map<Type, _ServiceFactory<Object, dynamic, dynamic>>>

which is not of subtype:

// ignore: prefer_collection_literals
final factoriesByName = LinkedHashMap<String?,LinkedHashMap<Type, _ServiceFactory<Object, dynamic, dynamic>>>();

Without renaming the type, this could be released as a minor update.
If we rename the type, it might break the clients and would require a major version update.

I'm ok with it either way, do you want to edit the CHANGELOG.md with the correct date?

Thanks for looking at the PR!

@bvoq bvoq requested a review from escamoteur January 3, 2024 13:15
@escamoteur
Copy link
Collaborator

Why do you think a change of the type of this this internal data structure might break any apps that use get_it? Change log addition would be fine. Also do we already have a note in the docs pointing out that we now dispose in reverse order?

… in the reverse order in which they were registered
@bvoq
Copy link
Contributor Author

bvoq commented Jan 4, 2024

Why do you think a change of the type of this this internal data structure might break any apps that use get_it?

I was wrong, my bad. I didn't see that it was part of a private class _Scope.
I've changed the type now and made the changelog more explicit.
I've also added information to the docs pointing out that they are disposed in the reverse order.

@escamoteur escamoteur merged commit 6f24d4c into fluttercommunity:master Jan 6, 2024
2 checks passed
@escamoteur
Copy link
Collaborator

thanks a lot for this fine PR, I just published a new version

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.

None yet

2 participants