search

fluttercommunity / get_it

Get It - Simple direct Service Locator that allows to decouple the interface from a concrete implementation and to access the concrete implementation from everywhere in your App. Maintainer: @escamoteur
https://pub.dev/packages/get_it
MIT License
1.36k stars 148 forks source link

popScopesTill broken in version 8 #384

Closed kuhnroyal closed 1 month ago

kuhnroyal commented 1 month ago

Hello @escamoteur

The changes in commit 7d2d4bb7d403b39af631f04e2e0dbb8419646f39 lead to popScopeTill(inclusive: true) always popping all scopes until the base scope. The while loop exit condition does not trigger when inclusive == true.

kuhnroyal commented 1 month ago

I added tests in https://github.com/fluttercommunity/get_it/pull/385 for now

escamoteur commented 1 month ago

Could you determine where the problem is? Am 23. Okt. 2024, 16:09 +0100 schrieb Peter Leibiger @.***>:

I added tests in #385 for now — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

kuhnroyal commented 1 month ago

Yea, I think I fixed it.

while (hasScope(scopeName) && inclusive
        ? (poppedScopeName != scopeName)
        : (nextScopeToPop.name != scopeName))

This lead to the else branch running even if hasScope returned false.

Additionally I found a secondary problem if the passed scopeName is the top scope and inclusive == false which would always have popped the first scope because of the used do() while loop.

Changes are on the same PR.

kuhnroyal commented 1 month ago

Fixed by #385

kuhnroyal commented 1 month ago

@escamoteur Can you please cut a release when you get a chance, thanks!

escamoteur commented 1 month ago

Sure, I was hoping to include the latest 2 PRs one is already merged. If the other doesn't come in this afternoon I will publish it anyway Am 28. Okt. 2024, 12:28 +0000 schrieb Peter Leibiger @.***>:

@escamoteur Can you please cut a release when you get a chance, thanks! — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

escamoteur commented 1 month ago

done Nachricht von @.*** am Oct 28, 2024, 1:30 PM +0100:

Sure, I was hoping to include the latest 2 PRs one is already merged. If the other doesn't come in this afternoon I will publish it anyway Am 28. Okt. 2024, 12:28 +0000 schrieb Peter Leibiger @.***>:

@escamoteur Can you please cut a release when you get a chance, thanks! — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

kuhnroyal commented 1 month ago

Thanks a lot!