From c1064da21b3e97fc1259ce36078f45cb192e87bd Mon Sep 17 00:00:00 2001 From: chunhtai <47866232+chunhtai@users.noreply.github.com> Date: Wed, 5 Jun 2024 16:17:53 -0700 Subject: [PATCH] Fixes Router transaction to respect operation order (#149763) fixes https://github.com/flutter/flutter/issues/142393 The issue is that if routerdelegate mutate its currentConfiguration outside of Router's workflow, the change will be propagate back to routeinformationprovider at the end of the frame. However if another things trigger router's dependencies change within the same frame. The dependencies will trigger a reparse of the current route which would end up override the currentConfiguration in routerDelegate. This change introduce a transaction system that each operation will be add to the end of the transaction future so everything will be execute inorder --- packages/flutter/lib/src/widgets/router.dart | 5 +- .../flutter/test/widgets/router_test.dart | 78 +++++++++++++++++++ 2 files changed, 81 insertions(+), 2 deletions(-) diff --git a/packages/flutter/lib/src/widgets/router.dart b/packages/flutter/lib/src/widgets/router.dart index 49212d688f7..6c73405247f 100644 --- a/packages/flutter/lib/src/widgets/router.dart +++ b/packages/flutter/lib/src/widgets/router.dart @@ -694,8 +694,9 @@ class _RouterState extends State> with RestorationMixin { // The super.didChangeDependencies may have parsed the route information. // This can happen if the didChangeDependencies is triggered by state // restoration or first build. - if (widget.routeInformationProvider != null && _routeParsePending) { - _processRouteInformation(widget.routeInformationProvider!.value, () => widget.routerDelegate.setNewRoutePath); + final RouteInformation? currentRouteInformation = _routeInformation.value ?? widget.routeInformationProvider?.value; + if (currentRouteInformation != null && _routeParsePending) { + _processRouteInformation(currentRouteInformation, () => widget.routerDelegate.setNewRoutePath); } _routeParsePending = false; _maybeNeedToReportRouteInformation(); diff --git a/packages/flutter/test/widgets/router_test.dart b/packages/flutter/test/widgets/router_test.dart index c3fb19ecd5d..d59bbe7f402 100644 --- a/packages/flutter/test/widgets/router_test.dart +++ b/packages/flutter/test/widgets/router_test.dart @@ -41,6 +41,44 @@ void main() { expect(find.text('update'), findsOneWidget); }); + testWidgets('Router respects update order', (WidgetTester tester) async { + final SimpleRouteInformationProvider provider = SimpleRouteInformationProvider(); + addTearDown(provider.dispose); + provider.value = RouteInformation( + uri: Uri.parse('initial'), + ); + + final MutableRouterDelegate delegate = MutableRouterDelegate(); + addTearDown(delegate.dispose); + + final ValueNotifier notifier = ValueNotifier(0); + await tester.pumpWidget(buildBoilerPlate( + IntInheritedNotifier( + notifier: notifier, + child: Router( + routeInformationProvider: provider, + routeInformationParser: CustomRouteInformationParser( + (RouteInformation information, BuildContext context) { + IntInheritedNotifier.of(context); // create dependency + return information; + }, + ), + routerDelegate: delegate, + ), + ) + )); + expect(find.text('initial'), findsOneWidget); + expect(delegate.currentConfiguration!.uri.toString(), 'initial'); + + delegate.updateConfiguration(RouteInformation(uri: Uri.parse('update'))); + notifier.value = 1; + + // The delegate should still retain the update. + await tester.pumpAndSettle(); + expect(find.text('update'), findsOneWidget); + expect(delegate.currentConfiguration!.uri.toString(), 'update'); + }); + testWidgets('Simple router basic functionality - asynchronized', (WidgetTester tester) async { final SimpleRouteInformationProvider provider = SimpleRouteInformationProvider(); addTearDown(provider.dispose); @@ -1934,3 +1972,43 @@ class RedirectingInformationParser extends RouteInformationParser with ChangeNotifier { + MutableRouterDelegate() { + if (kFlutterMemoryAllocationsEnabled) { + ChangeNotifier.maybeDispatchObjectCreation(this); + } + } + + @override + RouteInformation? currentConfiguration; + + @override + Future setNewRoutePath(RouteInformation configuration) { + currentConfiguration = configuration; + return SynchronousFuture(null); + } + + void updateConfiguration(RouteInformation newConfig) { + currentConfiguration = newConfig; + notifyListeners(); + } + + @override + Future popRoute() { + throw UnimplementedError(); + } + + @override + Widget build(BuildContext context) { + return Text(currentConfiguration?.uri.toString() ?? ''); + } +} + +class IntInheritedNotifier extends InheritedNotifier> { + const IntInheritedNotifier({super.key, required super.notifier, required super.child}); + + static int of(BuildContext context) { + return context.dependOnInheritedWidgetOfExactType()!.notifier!.value; + } +}