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

'popScope' has different behaivior on develop / release build. #152

Closed
swissonid opened this issue Jan 28, 2021 · 6 comments
Closed

'popScope' has different behaivior on develop / release build. #152

swissonid opened this issue Jan 28, 2021 · 6 comments
Labels
enhancement New feature or request

Comments

@swissonid
Copy link

I do not know if its intentional or not. The assertion gets removed/disabled during the release build. Which can lead to strange behaviour without any meaningful error message. Or just not to pop if you are on the baseScope

@override
  Future<void> popScope() async {
    assert(
        _scopes.length > 1,
        "You are already on the base scope. you can't pop"
        ' this one');
    await _currentScope.dispose();
    await _currentScope.reset(dispose: true);
    _scopes.removeLast();
  }
@escamoteur
Copy link
Collaborator

That is correct. But that's the idea of asserts in general. Do you see it as an error situation that could happen in release that wouldn't be a fatal error that should have been detected during development?

@swissonid
Copy link
Author

I just run in one of that situation mention in issue #153. I was relying on the thrown exception not realising that is was thrown by an assertion. It took me a day to find the bug. Is there any reason why not just prevent to pop the baseScope anyway?

@escamoteur
Copy link
Collaborator

yes, IMHO if that happens something is wrong with your App, so it's better to have an assert.

@swissonid
Copy link
Author

swissonid commented Jan 28, 2021

Ok, let's assume something is wrong in my app. Still would it not be nice if GetIt would prevent the User, in that case, the developer to make so silly mistake as I did? So that the end user gets a better experience. And print a warning or event throw an exception? So we get both, a happy end-user and GetIt can show to the developer he has done something properly wrong.

@escamoteur
Copy link
Collaborator

I'm a big fan of ailing fast :-) But I agree we could change this into an if with an exception instead of the assert, so that you get a meaningful error message

@escamoteur escamoteur added the enhancement New feature or request label Jan 28, 2021
@escamoteur
Copy link
Collaborator

Implemented in V6.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants