fix(navigation): keep the stack alive when the unwind target is absent - #718
Open
EugeneSusla wants to merge 4 commits into
Open
fix(navigation): keep the stack alive when the unwind target is absent#718EugeneSusla wants to merge 4 commits into
EugeneSusla wants to merge 4 commits into
Conversation
Logging an intake could leave the app on a black screen. Two unwinds use ModalRoute.withName, which removes every route when the named one is not on the stack: Edit Meal's save does pushNamedAndRemoveUntil(mealDetail, withName(addMeal)), and the intake bottom sheet then does popUntil(withName(main)). Reached from the home shortcut's scanner — a stack of main -> scanner with no addMeal below — the first call wipes main, the second finds nothing to stop at and pops the last route too, leaving an empty navigator. Both now stop at the first route as well. After the splash screen replaces itself the first route is the main screen, which is the floor both unwinds were aiming for, so the flows that do have addMeal on the stack behave exactly as before. The intake itself was always written; only the screen after it was lost.
Logging an activity ran the same popUntil(ModalRoute.withName(main)) as the intake sheet, so it emptied the navigator on any stack that had lost the main route. Same predicate, same fix. Also drops the .vscode SDK path that fvm rewrote into the branch: it pins the version a second time next to .fvmrc, and .fvm/flutter_sdk is the symlink fvm keeps current for exactly that reason.
Drives the real EditMealScreen save on the stack a recipe log leaves behind — [main, mealDetail] with no addMealRoute anywhere — and asserts the main screen is still there afterwards. Reverting the call site to ModalRoute.withName(addMealRoute) fails this test: the save removes every route below the new meal detail, so the intake sheet's popUntil(mainRoute) has nothing left to stop at and empties the navigator. The screens either side are stubs. Pushing editMealRoute is four unconditional lines in the meal-detail edit pencil, and standing up MealDetailScreen's seven-usecase bloc would not make the unwind under test any more real; the popUntil that follows it is covered in navigation_predicates_test.dart.
The first cut stubbed both screens either side of Edit Meal, so two of its claims were not actually tested: the `editOnly` assertion only re-read the constructor's own default, and the intake sheet's unwind was re-typed in the test rather than driven through meal_detail_bottom_sheet.dart. Both screens are real now. The test taps the real meal-detail edit pencil, the real Edit Meal save, and the real Add button on the real intake sheet, with only the main screen stubbed — all these unwinds want from it is a route named `main` to survive. Run unchanged against origin/develop (9b2b421) it reproduces the whole bug: the intake is written, every route under the new meal detail is removed, Add's popUntil finds no `main`, and the navigator ends up empty — no Scaffold rendering at all.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
Logging an intake from a recipe leaves the app on a black screen. The
intake itself is saved correctly — only the screen after it is lost.
The two unwinds that run at the end of the logging flow name a route that
is not always on the stack:
edit_meal_screen.dart:911—pushNamedAndRemoveUntil(mealDetailRoute, ModalRoute.withName(addMealRoute))meal_detail_bottom_sheet.dart:319—popUntil(ModalRoute.withName(mainRoute))ModalRoute.withName(x)matches nothing whenxis absent, and bothpopUntilandpushNamedAndRemoveUntilrespond to a predicate that nevermatches by removing every route. An empty navigator renders as a black
screen with nothing to navigate back to.
Reachable chain
recipe_detail_screen.dart:247pushesmealDetailRoute. Nothing onthis path pushed
addMealRoute, so the stack is[main, mealDetail].(
meal_detail_screen.dart:300-311) pusheseditMealRoutewith anEditMealScreenArgumentsthat passes noeditOnly. It defaults tofalse→ the create-and-log save path.edit_meal_screen.dart:911. NoaddMealRoutematches, so every route below the new meal detail is removed —
mainincluded. The stack is now
[mealDetail].meal_detail_bottom_sheet.dart:319) writesthe intake, then runs
popUntil(withName(mainRoute)). No match, so itpops the last remaining route. Empty navigator → black screen.
The other Edit Meal entry points are not affected, which is why this went
unnoticed:
recipes_page.dart:164andcustom_meals_tab.dart:160bothpass
editOnly: trueand thereforepop()instead of unwinding, andadd_meal_screen.dart:530reaches Edit Meal withaddMealRoutegenuinelyon the stack.
The fix
namedRouteOrFirst(routeName)—(route) => route.isFirst || route.settings.name == routeName.isFirstis the right floor because the main screen is the first routeonce the app is running:
splash_screen.dart:67reaches it withpushReplacementNamed, and the onboarding exits(
onboarding_screen.dart:150,:505,onboarding_intro_page_body.dart:220) replace or clear the stack the sameway. So a stack that has no
mainRouteby name still has the main screenat the bottom, and stopping there is what the unwind was aiming for
anyway.
The predicate deliberately does not assert that the first route is main.
Landing on the bottom route beats an empty navigator in every case, and
for the
addMealRoutecall site the first route is legitimately somethingelse.
activity_detail_screen.dart:323carried the identicalpopUntil(withName(mainRoute))and is fixed defensively — it is onlyreachable after step 3 has already removed
main, so it cannot emptythe stack on its own today, but it would break the same way if it ever
could.
Tests
test/features/edit_meal/presentation/edit_meal_unwind_test.dartdrivesthe real chain: the real
MealDetailScreen's edit pencil, the realEditMealScreen's save, and the realMealDetailBottomSheet's Add, onthe
[main, mealDetail]stack a recipe log leaves behind. Only the mainscreen is stubbed — all these unwinds want from it is a route named
mainto survive.
Run unchanged against
developit reproduces the whole bug: the intake iswritten, the navigator ends up empty, and nothing renders at all.
test/core/utils/navigation_predicates_test.dartcovers the predicatedirectly, including a test that pins Flutter's stack-emptying behaviour —
kept on purpose, as the signal for when this helper can be deleted.