diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index 3dc0e2fff3..c41a7d2fb3 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,3 +1,12 @@ +## 9.0.0 + +- **BREAKING CHANGE**: + - Removes GoRouter.location and GoRouter.canPop. Use GoRouterState.of().location and + Navigator.of().canPop instead. + - GoRouter does not `extends` ChangeNotifier. + - [Migration guide](https://flutter.dev/go/go-router-v9-breaking-changes) +- Reduces excessive rebuilds due to inherited look up. + ## 8.2.0 - Adds onException to GoRouter constructor. diff --git a/packages/go_router/README.md b/packages/go_router/README.md index 9beccbd72a..0e07a75517 100644 --- a/packages/go_router/README.md +++ b/packages/go_router/README.md @@ -37,6 +37,7 @@ See the API documentation for details on the following topics: - [Error handling](https://pub.dev/documentation/go_router/latest/topics/Error%20handling-topic.html) ## Migration guides +- [Migrating to 9.0.0](https://flutter.dev/go/go-router-v9-breaking-changes). - [Migrating to 8.0.0](https://flutter.dev/go/go-router-v8-breaking-changes). - [Migrating to 7.0.0](https://flutter.dev/go/go-router-v7-breaking-changes). - [Migrating to 6.0.0](https://flutter.dev/go/go-router-v6-breaking-changes) diff --git a/packages/go_router/lib/src/information_provider.dart b/packages/go_router/lib/src/information_provider.dart index 0f61e742bc..e63ddf439f 100644 --- a/packages/go_router/lib/src/information_provider.dart +++ b/packages/go_router/lib/src/information_provider.dart @@ -133,6 +133,14 @@ class GoRouteInformationProvider extends RouteInformationProvider RouteInformation get value => _value; RouteInformation _value; + @override + // TODO(chunhtai): remove this ignore once package minimum dart version is + // above 3. + // ignore: unnecessary_overrides + void notifyListeners() { + super.notifyListeners(); + } + void _setValue(String location, Object state) { final bool shouldNotify = _value.location != location || _value.state != state; diff --git a/packages/go_router/lib/src/misc/inherited_router.dart b/packages/go_router/lib/src/misc/inherited_router.dart index bd426d12f5..c41b4df231 100644 --- a/packages/go_router/lib/src/misc/inherited_router.dart +++ b/packages/go_router/lib/src/misc/inherited_router.dart @@ -11,13 +11,13 @@ import '../router.dart'; /// /// Used for to find the current GoRouter in the widget tree. This is useful /// when routing from anywhere in your app. -class InheritedGoRouter extends InheritedNotifier { +class InheritedGoRouter extends InheritedWidget { /// Default constructor for the inherited go router. const InheritedGoRouter({ required super.child, required this.goRouter, super.key, - }) : super(notifier: goRouter); + }); /// The [GoRouter] that is made available to the widget tree. final GoRouter goRouter; @@ -27,4 +27,7 @@ class InheritedGoRouter extends InheritedNotifier { super.debugFillProperties(properties); properties.add(DiagnosticsProperty('goRouter', goRouter)); } + + @override + bool updateShouldNotify(covariant InheritedWidget oldWidget) => false; } diff --git a/packages/go_router/lib/src/router.dart b/packages/go_router/lib/src/router.dart index 745fc3a014..26338ceebf 100644 --- a/packages/go_router/lib/src/router.dart +++ b/packages/go_router/lib/src/router.dart @@ -62,7 +62,7 @@ typedef GoExceptionHandler = void Function( /// {@category Deep linking} /// {@category Error handling} /// {@category Named routes} -class GoRouter extends ChangeNotifier implements RouterConfig { +class GoRouter implements RouterConfig { /// Default constructor to configure a GoRouter with a routes builder /// and an error page builder. /// @@ -152,7 +152,6 @@ class GoRouter extends ChangeNotifier implements RouterConfig { builderWithNav: (BuildContext context, Widget child) => InheritedGoRouter(goRouter: this, child: child), ); - routerDelegate.addListener(_handleStateMayChange); assert(() { log.info('setting initial location $initialLocation'); @@ -296,34 +295,9 @@ class GoRouter extends ChangeNotifier implements RouterConfig { @override late final GoRouteInformationParser routeInformationParser; - /// Gets the current location. - // TODO(chunhtai): deprecates this once go_router_builder is migrated to - // GoRouterState.of. - String get location => _location; - String _location = '/'; - /// Returns `true` if there is at least two or more route can be pop. bool canPop() => routerDelegate.canPop(); - void _handleStateMayChange() { - final String newLocation; - if (routerDelegate.currentConfiguration.isNotEmpty && - routerDelegate.currentConfiguration.matches.last - is ImperativeRouteMatch) { - newLocation = (routerDelegate.currentConfiguration.matches.last - as ImperativeRouteMatch) - .matches - .uri - .toString(); - } else { - newLocation = routerDelegate.currentConfiguration.uri.toString(); - } - if (_location != newLocation) { - _location = newLocation; - notifyListeners(); - } - } - /// Get a location from route name and parameters. /// This is useful for redirecting to a named location. String namedLocation( @@ -488,7 +462,7 @@ class GoRouter extends ChangeNotifier implements RouterConfig { /// of any GoRoute under it. void pop([T? result]) { assert(() { - log.info('popping $location'); + log.info('popping ${routerDelegate.currentConfiguration.uri}'); return true; }()); routerDelegate.pop(result); @@ -497,7 +471,7 @@ class GoRouter extends ChangeNotifier implements RouterConfig { /// Refresh the route. void refresh() { assert(() { - log.info('refreshing $location'); + log.info('refreshing ${routerDelegate.currentConfiguration.uri}'); return true; }()); routeInformationProvider.notifyListeners(); @@ -507,27 +481,25 @@ class GoRouter extends ChangeNotifier implements RouterConfig { /// /// This method throws when it is called during redirects. static GoRouter of(BuildContext context) { - final InheritedGoRouter? inherited = - context.dependOnInheritedWidgetOfExactType(); + final GoRouter? inherited = maybeOf(context); assert(inherited != null, 'No GoRouter found in context'); - return inherited!.goRouter; + return inherited!; } /// The current GoRouter in the widget tree, if any. /// /// This method returns null when it is called during redirects. static GoRouter? maybeOf(BuildContext context) { - final InheritedGoRouter? inherited = - context.dependOnInheritedWidgetOfExactType(); + final InheritedGoRouter? inherited = context + .getElementForInheritedWidgetOfExactType() + ?.widget as InheritedGoRouter?; return inherited?.goRouter; } - @override + /// Disposes resource created by this object. void dispose() { routeInformationProvider.dispose(); - routerDelegate.removeListener(_handleStateMayChange); routerDelegate.dispose(); - super.dispose(); } String _effectiveInitialLocation(String? initialLocation) { diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index 77370874a6..f977eb9984 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: 8.2.0 +version: 9.0.0 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 diff --git a/packages/go_router/test/extension_test.dart b/packages/go_router/test/extension_test.dart index df12172074..48a93a6144 100644 --- a/packages/go_router/test/extension_test.dart +++ b/packages/go_router/test/extension_test.dart @@ -5,6 +5,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:go_router/go_router.dart'; +import 'package:go_router/src/match.dart'; void main() { group('replaceNamed', () { @@ -37,7 +38,10 @@ void main() { final GoRouter router = await createGoRouter(tester); await tester.tap(find.text('Settings')); await tester.pumpAndSettle(); - expect(router.location, '/page-0/settings?search=notification'); + final ImperativeRouteMatch routeMatch = router + .routerDelegate.currentConfiguration.last as ImperativeRouteMatch; + expect(routeMatch.matches.uri.toString(), + '/page-0/settings?search=notification'); }); }); } diff --git a/packages/go_router/test/go_router_test.dart b/packages/go_router/test/go_router_test.dart index 1d4b059f03..c2dcfdb44c 100644 --- a/packages/go_router/test/go_router_test.dart +++ b/packages/go_router/test/go_router_test.dart @@ -258,42 +258,6 @@ void main() { expect(find.byType(DummyScreen), findsOneWidget); }); - testWidgets('can access GoRouter parameters from builder', - (WidgetTester tester) async { - final List routes = [ - GoRoute(path: '/', redirect: (_, __) => '/1'), - GoRoute( - path: '/:id', - builder: (BuildContext context, GoRouterState state) { - return Text(GoRouter.of(context).location); - }), - ]; - - final GoRouter router = await createRouter(routes, tester); - expect(find.text('/1'), findsOneWidget); - router.go('/123?id=456'); - await tester.pumpAndSettle(); - expect(find.text('/123?id=456'), findsOneWidget); - }); - - testWidgets('can access GoRouter parameters from error builder', - (WidgetTester tester) async { - final List routes = [ - GoRoute(path: '/', builder: dummy), - ]; - - final GoRouter router = await createRouter(routes, tester, - errorBuilder: (BuildContext context, GoRouterState state) { - return Text(GoRouter.of(context).location); - }); - router.go('/123?id=456'); - await tester.pumpAndSettle(); - expect(find.text('/123?id=456'), findsOneWidget); - router.go('/1234?id=456'); - await tester.pumpAndSettle(); - expect(find.text('/1234?id=456'), findsOneWidget); - }); - testWidgets('repeatedly pops imperative route does not crash', (WidgetTester tester) async { // Regression test for https://github.com/flutter/flutter/issues/123369. @@ -616,7 +580,11 @@ void main() { // NOTE: match the lower case, since location is canonicalized to match the // path case whereas the location can be any case; so long as the path // produces a match regardless of the location case, we win! - expect(router.location.toLowerCase(), loc.toLowerCase()); + expect( + router.routerDelegate.currentConfiguration.uri + .toString() + .toLowerCase(), + loc.toLowerCase()); expect(matches, hasLength(1)); expect(find.byType(FamilyScreen), findsOneWidget); @@ -1680,7 +1648,8 @@ void main() { return state.matchedLocation == '/login' ? null : '/login'; }); - expect(router.location, '/login'); + expect( + router.routerDelegate.currentConfiguration.uri.toString(), '/login'); expect(redirected, isTrue); redirected = false; @@ -1688,7 +1657,8 @@ void main() { await sendPlatformUrl('/dummy', tester); await tester.pumpAndSettle(); - expect(router.location, '/login'); + expect( + router.routerDelegate.currentConfiguration.uri.toString(), '/login'); expect(redirected, isTrue); }); @@ -1716,11 +1686,12 @@ void main() { return state.location; }); - expect(router.location, '/'); + expect(router.routerDelegate.currentConfiguration.uri.toString(), '/'); // Directly set the url through platform message. await sendPlatformUrl('/dummy', tester); await tester.pumpAndSettle(); - expect(router.location, '/dummy'); + expect( + router.routerDelegate.currentConfiguration.uri.toString(), '/dummy'); }); testWidgets('top-level redirect w/ named routes', @@ -1756,7 +1727,8 @@ void main() { ? null : state.namedLocation('login'), ); - expect(router.location, '/login'); + expect( + router.routerDelegate.currentConfiguration.uri.toString(), '/login'); }); testWidgets('route-level redirect', (WidgetTester tester) async { @@ -1784,7 +1756,8 @@ void main() { final GoRouter router = await createRouter(routes, tester); router.go('/dummy'); await tester.pump(); - expect(router.location, '/login'); + expect( + router.routerDelegate.currentConfiguration.uri.toString(), '/login'); }); testWidgets('top-level redirect take priority over route level', @@ -1826,7 +1799,8 @@ void main() { await sendPlatformUrl('/dummy', tester); await tester.pumpAndSettle(); - expect(router.location, '/login'); + expect( + router.routerDelegate.currentConfiguration.uri.toString(), '/login'); expect(redirected, isTrue); }); @@ -1860,7 +1834,8 @@ void main() { final GoRouter router = await createRouter(routes, tester); router.go('/dummy'); await tester.pump(); - expect(router.location, '/login'); + expect( + router.routerDelegate.currentConfiguration.uri.toString(), '/login'); }); testWidgets('multiple mixed redirect', (WidgetTester tester) async { @@ -1890,7 +1865,7 @@ void main() { state.matchedLocation == '/dummy1' ? '/dummy2' : null); router.go('/dummy1'); await tester.pump(); - expect(router.location, '/'); + expect(router.routerDelegate.currentConfiguration.uri.toString(), '/'); }); testWidgets('top-level redirect loop', (WidgetTester tester) async { @@ -2012,7 +1987,7 @@ void main() { tester, initialLocation: '/dummy', ); - expect(router.location, '/'); + expect(router.routerDelegate.currentConfiguration.uri.toString(), '/'); }); testWidgets('top-level redirect state', (WidgetTester tester) async { @@ -2314,7 +2289,8 @@ void main() { await sendPlatformUrl('/dummy/dummy2', tester); await tester.pumpAndSettle(); - expect(router.location, '/other'); + expect( + router.routerDelegate.currentConfiguration.uri.toString(), '/other'); }); }); @@ -2340,7 +2316,8 @@ void main() { tester, initialLocation: '/dummy', ); - expect(router.location, '/dummy'); + expect( + router.routerDelegate.currentConfiguration.uri.toString(), '/dummy'); }); testWidgets('initial location with extra', (WidgetTester tester) async { @@ -2366,7 +2343,8 @@ void main() { initialLocation: '/dummy', initialExtra: 'extra', ); - expect(router.location, '/dummy'); + expect( + router.routerDelegate.currentConfiguration.uri.toString(), '/dummy'); expect(find.byKey(const ValueKey('extra')), findsOneWidget); }); @@ -2389,7 +2367,7 @@ void main() { tester, initialLocation: '/dummy', ); - expect(router.location, '/'); + expect(router.routerDelegate.currentConfiguration.uri.toString(), '/'); }); testWidgets( @@ -2465,7 +2443,7 @@ void main() { final RouteMatchList matches = router.routerDelegate.currentConfiguration; - expect(router.location, loc); + expect(router.routerDelegate.currentConfiguration.uri.toString(), loc); expect(matches.matches, hasLength(1)); expect(find.byType(FamilyScreen), findsOneWidget); expect(matches.pathParameters['fid'], fid); @@ -2495,7 +2473,7 @@ void main() { final RouteMatchList matches = router.routerDelegate.currentConfiguration; - expect(router.location, loc); + expect(router.routerDelegate.currentConfiguration.uri.toString(), loc); expect(matches.matches, hasLength(1)); expect(find.byType(FamilyScreen), findsOneWidget); expect(matches.uri.queryParameters['fid'], fid); @@ -2729,11 +2707,11 @@ void main() { await tester.pumpAndSettle(); final RouteMatchList matches = router.routerDelegate.currentConfiguration; - expect(router.location, loc); expect(matches.matches, hasLength(2)); expect(find.byType(PersonScreen), findsOneWidget); final ImperativeRouteMatch imperativeRouteMatch = matches.matches.last as ImperativeRouteMatch; + expect(imperativeRouteMatch.matches.uri.toString(), loc); expect(imperativeRouteMatch.matches.pathParameters['fid'], fid); expect(imperativeRouteMatch.matches.pathParameters['pid'], pid); }); @@ -2799,7 +2777,7 @@ void main() { await tester.pumpAndSettle(); RouteMatchList matches = router.routerDelegate.currentConfiguration; - expect(router.location, loc); + expect(router.routerDelegate.currentConfiguration.uri.toString(), loc); expect(matches.matches, hasLength(4)); expect(find.byType(PersonScreen), findsOneWidget); expect(matches.pathParameters['fid'], fid); @@ -2861,7 +2839,8 @@ void main() { router.routerDelegate.currentConfiguration.matches; expect(matches, hasLength(1)); - expectLocationWithQueryParams(router.location); + expectLocationWithQueryParams( + router.routerDelegate.currentConfiguration.uri.toString()); expect( tester.widget(find.byType(DummyScreen)), isA().having( @@ -2912,7 +2891,8 @@ void main() { router.routerDelegate.currentConfiguration.matches; expect(matches, hasLength(1)); - expectLocationWithQueryParams(router.location); + expectLocationWithQueryParams( + router.routerDelegate.currentConfiguration.uri.toString()); expect( tester.widget(find.byType(DummyScreen)), isA().having( @@ -2962,7 +2942,8 @@ void main() { router.routerDelegate.currentConfiguration.matches; expect(matches, hasLength(1)); - expectLocationWithQueryParams(router.location); + expectLocationWithQueryParams( + router.routerDelegate.currentConfiguration.uri.toString()); expect( tester.widget(find.byType(DummyScreen)), isA().having( diff --git a/packages/go_router/test/inherited_test.dart b/packages/go_router/test/inherited_test.dart index 1b7fb3f86d..62b7b541c9 100644 --- a/packages/go_router/test/inherited_test.dart +++ b/packages/go_router/test/inherited_test.dart @@ -25,7 +25,7 @@ void main() { expect(shouldNotify, false); }); - test('updates when goRouter changes', () { + test('does not update even when goRouter changes', () { final GoRouter oldGoRouter = GoRouter( routes: [ GoRoute( @@ -46,7 +46,7 @@ void main() { oldGoRouter: oldGoRouter, newGoRouter: newGoRouter, ); - expect(shouldNotify, true); + expect(shouldNotify, false); }); }); diff --git a/packages/go_router/test/rebuild_test.dart b/packages/go_router/test/rebuild_test.dart new file mode 100644 index 0000000000..4617ef0131 --- /dev/null +++ b/packages/go_router/test/rebuild_test.dart @@ -0,0 +1,67 @@ +// 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/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:go_router/go_router.dart'; + +import 'test_helpers.dart'; + +void main() { + testWidgets('GoRouter.push does not trigger unnecessary rebuilds', + (WidgetTester tester) async { + final List routes = [ + GoRoute( + path: '/', builder: (BuildContext context, __) => const HomePage()), + GoRoute( + path: '/1', + builder: (BuildContext context, __) { + return ElevatedButton( + onPressed: () { + context.push('/1'); + }, + child: const Text('/1'), + ); + }), + ]; + + await createRouter(routes, tester); + expect(find.text('/'), findsOneWidget); + // first build + expect(HomePage.built, isTrue); + + HomePage.built = false; + // Should not be built again afterward. + await tester.tap(find.text('/')); + await tester.pumpAndSettle(); + expect(find.text('/1'), findsOneWidget); + expect(HomePage.built, isFalse); + + await tester.tap(find.text('/1')); + await tester.pumpAndSettle(); + expect(find.text('/1'), findsOneWidget); + expect(HomePage.built, isFalse); + + await tester.tap(find.text('/1')); + await tester.pumpAndSettle(); + expect(find.text('/1'), findsOneWidget); + expect(HomePage.built, isFalse); + }); +} + +class HomePage extends StatelessWidget { + const HomePage({super.key}); + + static bool built = false; + @override + Widget build(BuildContext context) { + built = true; + return ElevatedButton( + onPressed: () { + context.push('/1'); + }, + child: const Text('/'), + ); + } +}