Skip to content
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions packages/go_router/test/on_enter_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -898,6 +898,60 @@ void main() {
expect(find.text('Start'), findsOneWidget);
});

// Regression test for https://github.com/flutter/flutter/issues/179370.
// A SYNCHRONOUS onEnter that returns `Block.then(() => router.go(...))`
// on the initial route must still run the `then` callback so the
// navigation redirect occurs. Previously the sync path resolved the
// SynchronousFuture inline, causing the deferred microtask to run
// before the Router had finished the initial navigation, which broke
// the redirect that the user added the `async` keyword to work around.
testWidgets(
'Synchronous onEnter Block.then(router.go) redirects from initial route',
(WidgetTester tester) async {
late GoRouter localRouter;
localRouter = GoRouter(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The localRouter variable is redundant. You can assign the GoRouter instance directly to the group-level router variable, which is consistent with the implementation pattern used in other tests within this file.

Suggested change
late GoRouter localRouter;
localRouter = GoRouter(
router = GoRouter(

initialLocation: '/dashboard',
// Note: deliberately NOT marked `async`.
onEnter:
(
BuildContext context,
GoRouterState current,
GoRouterState next,
GoRouter goRouter,
) {
// Mirrors the exact predicate from #179370: the user keys
// off `current.uri.path`, which on the initial parse equals
// `nextState.uri.path` because the router has no committed
// configuration yet.
if (current.uri.path == '/dashboard') {
return Block.then(() => goRouter.go('/login'));
}
return const Allow();
},
routes: <RouteBase>[
GoRoute(
path: '/dashboard',
builder: (_, __) =>
const Scaffold(body: Center(child: Text('Dashboard'))),
),
GoRoute(
path: '/login',
builder: (_, __) =>
const Scaffold(body: Center(child: Text('Login'))),
),
],
);
router = localRouter;

await tester.pumpWidget(MaterialApp.router(routerConfig: localRouter));
await tester.pumpAndSettle();

expect(localRouter.state.uri.path, equals('/login'));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Update these lines to use the group-level router variable directly and remove the redundant localRouter assignment to maintain consistency with the rest of the test suite.

Suggested change
router = localRouter;
await tester.pumpWidget(MaterialApp.router(routerConfig: localRouter));
await tester.pumpAndSettle();
expect(localRouter.state.uri.path, equals('/login'));
await tester.pumpWidget(MaterialApp.router(routerConfig: router));
await tester.pumpAndSettle();
expect(router.state.uri.path, equals('/login'));

expect(find.text('Login'), findsOneWidget);
expect(find.text('Dashboard'), findsNothing);
},
);

testWidgets('Should call onException when exceptions thrown in onEnter callback', (
WidgetTester tester,
) async {
Expand Down