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

Fix: popToNamed(...), need to break from the while loop when entry found #500

Merged
merged 3 commits into from
Apr 5, 2022

Conversation

Goddchen
Copy link
Contributor

@Goddchen Goddchen commented Apr 4, 2022

We found an issue where our complete history stack is always reset when using popToNamed(...). Turns out that the while loop is missing a break when the actual entry has been found the entries following (including the entry itself) the entry have been removed. Because if we do not break here, we continue with the next while loop iteration, won't find the entry, and remove the last entry until no entries are left and we end up with an empty stack.

@codecov
Copy link

codecov bot commented Apr 4, 2022

Codecov Report

Merging #500 (8edecbe) into master (0c653f8) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #500   +/-   ##
=======================================
  Coverage   96.74%   96.74%           
=======================================
  Files          13       13           
  Lines         861      861           
=======================================
  Hits          833      833           
  Misses         28       28           
Impacted Files Coverage Δ
package/lib/src/beamer_delegate.dart 94.50% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c653f8...8edecbe. Read the comment docs.

@Goddchen Goddchen changed the title Fix: Need to break from the while loop when entry found Fix: popToNamed(...), need to break from the while loop when entry found Apr 5, 2022
Copy link
Owner

@slovnicki slovnicki left a comment

Choose a reason for hiding this comment

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

Hey @Goddchen
Amazing find! Nice, thanks for the PR! 💙

@slovnicki slovnicki merged commit cab91f7 into slovnicki:master Apr 5, 2022
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