Skip to content

Added hasScope api to easily verify if the scope already pushed before #327

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

Merged
merged 1 commit into from
May 26, 2023

Conversation

mondoktamas
Copy link
Contributor

In some cases, I need to check if the scope that I'm trying to push has already been pushed before.
There is an option to do it by setting the onScopeChanged callback and managing the scope changes in a stack somewhere in my sources, but as GetIt already manages the list of scopes, I believe the new hasScope() API can be a better way to check it.

@mondoktamas
Copy link
Contributor Author

@escamoteur could you please take a look?

@escamoteur
Copy link
Collaborator

Hmm, my first reaction is 'if you don't know if you have already pushed that scope then something in your code is wrong' using the existence of a scope to store information feels wrong to me.
that said why don't you just check if a certain type that is registered in that sope is already registered isRegistered is your friend for that?

@mondoktamas
Copy link
Contributor Author

mondoktamas commented May 26, 2023

In my case it's important to know when I handle a deep link, so before I push the routes I need to build the scopes properly.
But probably I can use the isRegistered as well. It just seems to me more natural to check if the scope has been pushed then check some modules if registered, modules can be registered in different scopes.

Anyway, thanks for the reply. If you feel this PR does not make sense then i'll close it

@escamoteur escamoteur merged commit ebfaee6 into fluttercommunity:master May 26, 2023
@escamoteur
Copy link
Collaborator

Yeah, deeplinks can make is difficult and I don't see a reason not to add it. will wait for another fix before pushing to pub

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