From 63ef0079c360de892096e47830786f3a4fd205d6 Mon Sep 17 00:00:00 2001 From: ahyangnb Date: Sat, 28 Dec 2024 15:09:40 +0800 Subject: [PATCH 1/8] reproduce issue https://github.com/flutter/flutter/issues/150312 --- .../example/lib/stream_listener_router.dart | 314 ++++++++++++++++++ 1 file changed, 314 insertions(+) create mode 100644 packages/go_router/example/lib/stream_listener_router.dart diff --git a/packages/go_router/example/lib/stream_listener_router.dart b/packages/go_router/example/lib/stream_listener_router.dart new file mode 100644 index 000000000000..38079ae8281e --- /dev/null +++ b/packages/go_router/example/lib/stream_listener_router.dart @@ -0,0 +1,314 @@ +import 'dart:async'; +import 'dart:developer'; + +import 'package:flutter/material.dart'; +import 'package:go_router/go_router.dart'; + +void main() { + GoRouter.optionURLReflectsImperativeAPIs = true; + + WidgetsFlutterBinding.ensureInitialized(); + + runApp(const MyApp()); +} + +/// A counter stream that emits a new value when the counter is incremented. +class CounterStream { + int _counter = 0; + + final StreamController _streamController = + StreamController.broadcast(); + + /// The stream that emits a new value when the counter is incremented. + Stream get stateStream => _streamController.stream.asBroadcastStream(); + + /// Increments the counter and emits a new value. + void increment() { + _streamController.sink.add(++_counter); + } +} + +/// A counter stream that emits a new value when the counter is incremented. +final CounterStream counterStream = CounterStream(); + +/// A listener that listens to a stream and refreshes the router when the stream emits a new value. +class StreamListener extends ChangeNotifier { + /// Creates a stream listener. + StreamListener(Stream stream) { + notifyListeners(); + + _subscription = stream.asBroadcastStream().listen((_) { + notifyListeners(); + }); + } + + late final StreamSubscription _subscription; + + @override + void notifyListeners() { + super.notifyListeners(); + log('refreshing the router'); + } + + @override + void dispose() { + _subscription.cancel(); + super.dispose(); + } +} + +/// The main application widget. +class MyApp extends StatefulWidget { + /// Creates the main application widget. + const MyApp({super.key}); + + @override + State createState() => _MyAppState(); +} + +final GlobalKey _rootNavigatorKey = GlobalKey(); + +final GoRouter _router = GoRouter( + initialLocation: '/', + navigatorKey: _rootNavigatorKey, + refreshListenable: StreamListener(counterStream.stateStream), + routes: [ + ShellRoute( + builder: (BuildContext context, GoRouterState state, Widget child) { + return GenericPage(child: child); + }, + routes: [ + GoRoute( + path: '/', + builder: (BuildContext context, GoRouterState state) => + const GenericPage(showPushButton: true, path: 'a'), + routes: [ + GoRoute( + path: 'a', + name: 'a', + builder: (BuildContext context, GoRouterState state) => + const GenericPage(showPushButton: true, path: 'b'), + routes: [ + GoRoute( + path: 'b', + name: 'b', + builder: (BuildContext context, GoRouterState state) => + const GenericPage(showBackButton: true), + ), + ], + ), + ], + ), + ], + ), + ], +); + +class _MyAppState extends State { + late StreamSubscription _stateSubscription; + + /// The current state of the counter. + int _currentState = 0; + + @override + void initState() { + super.initState(); + _stateSubscription = counterStream.stateStream.listen((int state) { + setState(() { + _currentState = state; + log('$_currentState:: "try double place to listen"'); + }); + }); + } + + @override + void dispose() { + _stateSubscription.cancel(); + super.dispose(); + } + + @override + Widget build(BuildContext context) { + return MaterialApp.router( + title: 'Flutter Demo', + theme: ThemeData( + primarySwatch: Colors.blue, + ), + routerConfig: _router, + ); + } +} + +/// A dialog test widget. +class DialogTest extends StatelessWidget { + /// Creates a dialog test widget. + const DialogTest({super.key}); + + @override + Widget build(BuildContext context) { + return Center( + child: Container( + width: 300, + height: 300, + alignment: Alignment.center, + child: Material( + color: Colors.white, + child: Column( + children: + ['Navigator::pop', 'GoRouter::pop'].map((String e) { + return InkWell( + child: SizedBox( + height: 60, + width: 300, + child: Row( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + Text(e), + const Icon(Icons.close), + ], + ), + ), + onTap: () { + if (e == 'GoRouter::pop') { + // WHEN THE USER PRESSES THIS BUTTON, THE URL + // DOESN'T CHANGE, BUT THE SCREEN DOES + counterStream + .increment(); // <- when removing this line the issue is gone + GoRouter.of(context).pop(); + } else { + Navigator.of(context).pop(); + } + }, + ); + }).toList(), + ), + ), + ), + ); + } +} + +/// A generic page that can be used to display a page in the app. +class GenericPage extends StatefulWidget { + /// Creates a generic page. + const GenericPage({ + this.child, + Key? key, + this.showPushButton = false, + this.showBackButton = false, + this.path, + }) : super(key: key ?? const ValueKey('ShellWidget')); + + /// The child widget to be displayed in the page. + final Widget? child; + + /// Whether to show the push button. + final bool showPushButton; + + /// Whether to show the back button. + final bool showBackButton; + + /// The path of the page. + final String? path; + + @override + State createState() => _GenericPageState(); +} + +class _GenericPageState extends State { + late StreamSubscription _stateSubscription; + int _currentState = 0; + final GlobalKey _scaffoldKey = GlobalKey(); + @override + void initState() { + super.initState(); + _stateSubscription = counterStream.stateStream.listen((int state) { + setState(() { + _currentState = state; + }); + }); + } + + @override + void dispose() { + _stateSubscription.cancel(); + super.dispose(); + } + + @override + Widget build(BuildContext context) { + return Scaffold( + key: _scaffoldKey, + appBar: widget.child != null + ? AppBar( + title: Text('Count: $_currentState'), + actions: [ + TextButton( + onPressed: () { + showDialog( + context: context, + builder: (BuildContext context) { + return const DialogTest(); + }, + ); + }, + child: const Text('dialog1'), + ), + TextButton( + onPressed: () { + showModalBottomSheet( + context: context, + builder: (BuildContext context) { + return const DialogTest(); + }, + ); + }, + child: const Text('dialog2'), + ), + TextButton( + onPressed: () { + _scaffoldKey.currentState?.openEndDrawer(); + }, + child: const Text('EndDrawer'), + ), + ], + ) + : null, + endDrawer: const Drawer( + width: 200, + child: DialogTest(), + ), + body: _buildWidget(context), + ); + } + + Widget _buildWidget(BuildContext context) { + if (widget.child != null) { + return widget.child!; + } + + if (widget.showBackButton) { + return TextButton( + onPressed: () { + // WHEN THE USER PRESSES THIS BUTTON, THE URL + // DOESN'T CHANGE, BUT THE SCREEN DOES + counterStream + .increment(); // <- when removing this line the issue is gone + GoRouter.of(context).pop(); + }, + child: const Text('<- Go Back'), + ); + } + + if (widget.showPushButton) { + return TextButton( + onPressed: () { + GoRouter.of(context).goNamed(widget.path!); + }, + child: const Text('Push ->'), + ); + } + + return Text('Current state: $_currentState'); + } +} From b92dc001fd6e96b66d8d1a6a17e914c8f3ba16b9 Mon Sep 17 00:00:00 2001 From: ahyangnb Date: Sat, 28 Dec 2024 15:29:57 +0800 Subject: [PATCH 2/8] fix #150312 --- .../lib/src/information_provider.dart | 18 +++++++++++++++++- packages/go_router/lib/src/parser.dart | 3 +++ packages/go_router/lib/src/router.dart | 4 ++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/packages/go_router/lib/src/information_provider.dart b/packages/go_router/lib/src/information_provider.dart index 400ead006f75..f3019618beec 100644 --- a/packages/go_router/lib/src/information_provider.dart +++ b/packages/go_router/lib/src/information_provider.dart @@ -34,6 +34,9 @@ enum NavigatingType { /// Restore the current match list with /// [RouteInformationState.baseRouteMatchList]. restore, + + /// Pop Current route. + pop, } /// The data class to be stored in [RouteInformation.state] to be used by @@ -49,7 +52,9 @@ class RouteInformationState { this.completer, this.baseRouteMatchList, required this.type, - }) : assert((type == NavigatingType.go || type == NavigatingType.restore) == + }) : assert((type == NavigatingType.go || + type == NavigatingType.restore || + type == NavigatingType.pop) == (completer == null)), assert((type != NavigatingType.go) == (baseRouteMatchList != null)); @@ -227,6 +232,17 @@ class GoRouteInformationProvider extends RouteInformationProvider return completer.future; } + /// Save the pop state to value when remove top-most route. + void popSave(String location, {required RouteMatchList base}) { + _value = RouteInformation( + uri: Uri.parse(location), + state: RouteInformationState( + baseRouteMatchList: base, + type: NavigatingType.pop, + ), + ); + } + RouteInformation _valueInEngine; void _platformReportsNewRouteInformation(RouteInformation routeInformation) { diff --git a/packages/go_router/lib/src/parser.dart b/packages/go_router/lib/src/parser.dart index d1981898a8bb..716447e95c1a 100644 --- a/packages/go_router/lib/src/parser.dart +++ b/packages/go_router/lib/src/parser.dart @@ -216,6 +216,9 @@ class GoRouteInformationParser extends RouteInformationParser { return baseRouteMatchList!.uri.toString() != newMatchList.uri.toString() ? newMatchList : baseRouteMatchList; + case NavigatingType.pop: + // Will do nothing. + return baseRouteMatchList!; } } diff --git a/packages/go_router/lib/src/router.dart b/packages/go_router/lib/src/router.dart index 11f40f505cac..94f9ef004b7c 100644 --- a/packages/go_router/lib/src/router.dart +++ b/packages/go_router/lib/src/router.dart @@ -497,6 +497,10 @@ class GoRouter implements RouterConfig { return true; }()); routerDelegate.pop(result); + routeInformationProvider.popSave( + routerDelegate.currentConfiguration.uri.toString(), + base: routerDelegate.currentConfiguration, + ); } /// Refresh the route. From 614663c086200aae8c31cb7648c9eccb95e2046a Mon Sep 17 00:00:00 2001 From: ahyangnb Date: Thu, 9 Jan 2025 18:58:59 +0800 Subject: [PATCH 3/8] A new way to fix the #6953 issue. --- .../lib/src/information_provider.dart | 18 +----------------- packages/go_router/lib/src/parser.dart | 3 --- packages/go_router/lib/src/router.dart | 9 +++++---- 3 files changed, 6 insertions(+), 24 deletions(-) diff --git a/packages/go_router/lib/src/information_provider.dart b/packages/go_router/lib/src/information_provider.dart index f3019618beec..400ead006f75 100644 --- a/packages/go_router/lib/src/information_provider.dart +++ b/packages/go_router/lib/src/information_provider.dart @@ -34,9 +34,6 @@ enum NavigatingType { /// Restore the current match list with /// [RouteInformationState.baseRouteMatchList]. restore, - - /// Pop Current route. - pop, } /// The data class to be stored in [RouteInformation.state] to be used by @@ -52,9 +49,7 @@ class RouteInformationState { this.completer, this.baseRouteMatchList, required this.type, - }) : assert((type == NavigatingType.go || - type == NavigatingType.restore || - type == NavigatingType.pop) == + }) : assert((type == NavigatingType.go || type == NavigatingType.restore) == (completer == null)), assert((type != NavigatingType.go) == (baseRouteMatchList != null)); @@ -232,17 +227,6 @@ class GoRouteInformationProvider extends RouteInformationProvider return completer.future; } - /// Save the pop state to value when remove top-most route. - void popSave(String location, {required RouteMatchList base}) { - _value = RouteInformation( - uri: Uri.parse(location), - state: RouteInformationState( - baseRouteMatchList: base, - type: NavigatingType.pop, - ), - ); - } - RouteInformation _valueInEngine; void _platformReportsNewRouteInformation(RouteInformation routeInformation) { diff --git a/packages/go_router/lib/src/parser.dart b/packages/go_router/lib/src/parser.dart index 716447e95c1a..d1981898a8bb 100644 --- a/packages/go_router/lib/src/parser.dart +++ b/packages/go_router/lib/src/parser.dart @@ -216,9 +216,6 @@ class GoRouteInformationParser extends RouteInformationParser { return baseRouteMatchList!.uri.toString() != newMatchList.uri.toString() ? newMatchList : baseRouteMatchList; - case NavigatingType.pop: - // Will do nothing. - return baseRouteMatchList!; } } diff --git a/packages/go_router/lib/src/router.dart b/packages/go_router/lib/src/router.dart index 94f9ef004b7c..b5720d26d221 100644 --- a/packages/go_router/lib/src/router.dart +++ b/packages/go_router/lib/src/router.dart @@ -491,16 +491,17 @@ class GoRouter implements RouterConfig { /// /// If the top-most route is a pop up or dialog, this method pops it instead /// of any GoRoute under it. + /// + /// @restore() + /// Ensure that the `value` of `routeInformationProvider` is synced + /// with `routerDelegate.currentConfiguration`. void pop([T? result]) { assert(() { log('popping ${routerDelegate.currentConfiguration.uri}'); return true; }()); routerDelegate.pop(result); - routeInformationProvider.popSave( - routerDelegate.currentConfiguration.uri.toString(), - base: routerDelegate.currentConfiguration, - ); + restore(routerDelegate.currentConfiguration); } /// Refresh the route. From ae86e7a615cb9debe43f5711759317c5aa0a90e0 Mon Sep 17 00:00:00 2001 From: ahyangnb Date: Wed, 22 Jan 2025 10:12:06 +0800 Subject: [PATCH 4/8] Unit test for issues/150312 --- .../example/test/test_stream_listener.dart | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 packages/go_router/example/test/test_stream_listener.dart diff --git a/packages/go_router/example/test/test_stream_listener.dart b/packages/go_router/example/test/test_stream_listener.dart new file mode 100644 index 000000000000..2457d192a0a3 --- /dev/null +++ b/packages/go_router/example/test/test_stream_listener.dart @@ -0,0 +1,41 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:go_router_examples/stream_listener_router.dart'; + +void main() { + // For issue https://github.com/flutter/flutter/issues/150312. + testWidgets('GoRouter.of(context).pop() works', (WidgetTester tester) async { + await tester.pumpWidget(const MyApp()); + + // Navigate to the second page + expect(find.text('Push ->'), findsOneWidget); + await tester.tap(find.text('Push ->')); + await tester.pumpAndSettle(); + + // Navigate to the third page + expect(find.text('Push ->'), findsOneWidget); + await tester.tap(find.text('Push ->')); + await tester.pumpAndSettle(); + + // Verify we are on the second page + expect(find.text('<- Go Back'), findsOneWidget); + await tester.tap(find.text('<- Go Back')); + await tester.pumpAndSettle(); + + // Expect the Count is 1. + expect(find.text('Count: 1'), findsOneWidget); + + // Check if we are pop back to the second page + // and push to the third page again. + expect(find.text('Push ->'), findsOneWidget); + await tester.tap(find.text('Push ->')); + await tester.pumpAndSettle(); + + // Now we try pop again. + expect(find.text('<- Go Back'), findsOneWidget); + await tester.tap(find.text('<- Go Back')); + await tester.pumpAndSettle(); + + // Check count increment. + expect(find.text('Count: 2'), findsOneWidget); + }); +} \ No newline at end of file From cad1f24391b959636413cc8c9dfadc1c5009a942 Mon Sep 17 00:00:00 2001 From: ahyangnb Date: Wed, 22 Jan 2025 14:35:10 +0800 Subject: [PATCH 5/8] =?UTF-8?q?fix"info=20=E2=80=A2=20example/test/test=5F?= =?UTF-8?q?stream=5Flistener.dart:41:2=20=E2=80=A2=20Missing=20a=20newline?= =?UTF-8?q?=20at=20the=20end=20of=20the=20file.=20Try=20adding=20a=20newli?= =?UTF-8?q?ne=20at=20the=20end=20of=20the=20file.=20=E2=80=A2=20eol=5Fat?= =?UTF-8?q?=5Fend=5Fof=5Ffile"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/go_router/example/test/test_stream_listener.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/go_router/example/test/test_stream_listener.dart b/packages/go_router/example/test/test_stream_listener.dart index 2457d192a0a3..a4450334f844 100644 --- a/packages/go_router/example/test/test_stream_listener.dart +++ b/packages/go_router/example/test/test_stream_listener.dart @@ -38,4 +38,4 @@ void main() { // Check count increment. expect(find.text('Count: 2'), findsOneWidget); }); -} \ No newline at end of file +} From 6030676ab7a836c902235056727c30c87d5c25bb Mon Sep 17 00:00:00 2001 From: ahyangnb Date: Wed, 22 Jan 2025 14:48:04 +0800 Subject: [PATCH 6/8] license text used by all first-party files in this repository --- packages/go_router/example/lib/stream_listener_router.dart | 4 ++++ packages/go_router/example/test/test_stream_listener.dart | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/packages/go_router/example/lib/stream_listener_router.dart b/packages/go_router/example/lib/stream_listener_router.dart index 38079ae8281e..8325a0668daf 100644 --- a/packages/go_router/example/lib/stream_listener_router.dart +++ b/packages/go_router/example/lib/stream_listener_router.dart @@ -1,3 +1,7 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + import 'dart:async'; import 'dart:developer'; diff --git a/packages/go_router/example/test/test_stream_listener.dart b/packages/go_router/example/test/test_stream_listener.dart index a4450334f844..eac652ff534f 100644 --- a/packages/go_router/example/test/test_stream_listener.dart +++ b/packages/go_router/example/test/test_stream_listener.dart @@ -1,3 +1,7 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + import 'package:flutter_test/flutter_test.dart'; import 'package:go_router_examples/stream_listener_router.dart'; From 545f133e47b41924524fa4275206f0ef627f4f1e Mon Sep 17 00:00:00 2001 From: ahyangnb Date: Wed, 22 Jan 2025 14:58:47 +0800 Subject: [PATCH 7/8] Version 14.6.5: - Fix issue 150312. --- packages/go_router/CHANGELOG.md | 4 ++++ packages/go_router/pubspec.yaml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index 7134ea8aa4c9..e5c98dcf2971 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,3 +1,7 @@ +## 14.6.5 + +- Fix issue 150312. + ## 14.6.4 - Rephrases readme. diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index ef2d9422b465..7600be29884b 100644 --- a/packages/go_router/pubspec.yaml +++ b/packages/go_router/pubspec.yaml @@ -1,7 +1,7 @@ name: go_router description: A declarative router for Flutter based on Navigation 2 supporting deep linking, data-driven routes and more -version: 14.6.4 +version: 14.6.5 repository: https://github.com/flutter/packages/tree/main/packages/go_router issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+go_router%22 From 99f729348255cf81e6ad6a2ab2d41ce2ebed0ee5 Mon Sep 17 00:00:00 2001 From: ahyangnb Date: Fri, 24 Jan 2025 16:20:31 +0800 Subject: [PATCH 8/8] version 14.7.2: - Fix issue 150312. --- packages/go_router/CHANGELOG.md | 8 ++++---- packages/go_router/pubspec.yaml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index 4266ce651225..9791a17ff3aa 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,3 +1,7 @@ +## 14.7.2 + +- Fix issue 150312. + ## 14.7.1 - Fixes return type of current state getter on `GoRouter` and `GoRouterDelegate` to be non-nullable. @@ -6,10 +10,6 @@ - Adds fragment support to GoRouter, enabling direct specification and automatic handling of fragments in routes. -## 14.6.5 - -- Fix issue 150312. - ## 14.6.4 - Rephrases readme. diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index c020fd4ae31a..204db07f029f 100644 --- a/packages/go_router/pubspec.yaml +++ b/packages/go_router/pubspec.yaml @@ -1,7 +1,7 @@ name: go_router description: A declarative router for Flutter based on Navigation 2 supporting deep linking, data-driven routes and more -version: 14.7.1 +version: 14.7.2 repository: https://github.com/flutter/packages/tree/main/packages/go_router issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+go_router%22