diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index b96164d182e..28ee780a10e 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,3 +1,7 @@ +## 11.1.2 + +- Fixes a bug where the known routes and initial route were logged even when `debugLogDiagnostics` was set to `false`. + ## 11.1.1 - Fixes a missing `{@end-tool}` doc directive tag for `GoRoute.name`. diff --git a/packages/go_router/lib/src/builder.dart b/packages/go_router/lib/src/builder.dart index 0f4e4ab8845..e3e116fe758 100644 --- a/packages/go_router/lib/src/builder.dart +++ b/packages/go_router/lib/src/builder.dart @@ -445,17 +445,17 @@ class RouteBuilder { final Element? elem = context is Element ? context : null; if (elem != null && isMaterialApp(elem)) { - log.info('Using MaterialApp configuration'); + log('Using MaterialApp configuration'); _pageBuilderForAppType = pageBuilderForMaterialApp; _errorBuilderForAppType = (BuildContext c, GoRouterState s) => MaterialErrorScreen(s.error); } else if (elem != null && isCupertinoApp(elem)) { - log.info('Using CupertinoApp configuration'); + log('Using CupertinoApp configuration'); _pageBuilderForAppType = pageBuilderForCupertinoApp; _errorBuilderForAppType = (BuildContext c, GoRouterState s) => CupertinoErrorScreen(s.error); } else { - log.info('Using WidgetsApp configuration'); + log('Using WidgetsApp configuration'); _pageBuilderForAppType = pageBuilderForWidgetApp; _errorBuilderForAppType = (BuildContext c, GoRouterState s) => ErrorScreen(s.error); diff --git a/packages/go_router/lib/src/configuration.dart b/packages/go_router/lib/src/configuration.dart index 618e583b7df..4c9c018ed80 100644 --- a/packages/go_router/lib/src/configuration.dart +++ b/packages/go_router/lib/src/configuration.dart @@ -32,7 +32,7 @@ class RouteConfiguration { routes, >[navigatorKey])) { assert(_debugCheckStatefulShellBranchDefaultLocations(routes)); _cacheNameToPath('', routes); - log.info(debugKnownRoutes()); + log(debugKnownRoutes()); } static bool _debugCheckPath(List routes, bool isTopLevel) { @@ -234,7 +234,7 @@ class RouteConfiguration { Map queryParameters = const {}, }) { assert(() { - log.info('getting location for name: ' + log('getting location for name: ' '"$name"' '${pathParameters.isEmpty ? '' : ', pathParameters: $pathParameters'}' '${queryParameters.isEmpty ? '' : ', queryParameters: $queryParameters'}'); @@ -492,7 +492,7 @@ class RouteConfiguration { _addRedirect(redirectHistory, newMatch, previousLocation); return newMatch; } on GoException catch (e) { - log.info('Redirection exception: ${e.message}'); + log('Redirection exception: ${e.message}'); return _errorRouteMatchList(previousLocation, e); } } @@ -522,7 +522,7 @@ class RouteConfiguration { redirects.add(newMatch); - log.info('redirecting to $newMatch'); + log('redirecting to $newMatch'); } String _formatRedirectionHistory(List redirections) { diff --git a/packages/go_router/lib/src/logging.dart b/packages/go_router/lib/src/logging.dart index ccbb09b7f87..3f39e4dc804 100644 --- a/packages/go_router/lib/src/logging.dart +++ b/packages/go_router/lib/src/logging.dart @@ -4,22 +4,35 @@ import 'dart:async'; import 'dart:developer' as developer; + import 'package:flutter/foundation.dart'; import 'package:logging/logging.dart'; /// The logger for this package. -final Logger log = Logger('GoRouter'); +@visibleForTesting +final Logger logger = Logger('GoRouter'); + +/// Whether or not the logging is enabled. +bool _enabled = false; + +/// Logs the message if logging is enabled. +void log(String message) { + if (_enabled) { + logger.info(message); + } +} StreamSubscription? _subscription; /// Forwards diagnostic messages to the dart:developer log() API. void setLogging({bool enabled = false}) { _subscription?.cancel(); + _enabled = enabled; if (!enabled) { return; } - _subscription = log.onRecord.listen((LogRecord e) { + _subscription = logger.onRecord.listen((LogRecord e) { // use `dumpErrorToConsole` for severe messages to ensure that severe // exceptions are formatted consistently with other Flutter examples and // avoids printing duplicate exceptions diff --git a/packages/go_router/lib/src/parser.dart b/packages/go_router/lib/src/parser.dart index 41f6ef875cf..32860014f78 100644 --- a/packages/go_router/lib/src/parser.dart +++ b/packages/go_router/lib/src/parser.dart @@ -89,7 +89,7 @@ class GoRouteInformationParser extends RouteInformationParser { // TODO(chunhtai): remove this ignore and migrate the code // https://github.com/flutter/flutter/issues/124045. // ignore: deprecated_member_use - log.info('No initial matches: ${routeInformation.location}'); + log('No initial matches: ${routeInformation.location}'); } return debugParserFuture = _redirect( diff --git a/packages/go_router/lib/src/router.dart b/packages/go_router/lib/src/router.dart index 534581abef3..44a95f387e6 100644 --- a/packages/go_router/lib/src/router.dart +++ b/packages/go_router/lib/src/router.dart @@ -161,7 +161,7 @@ class GoRouter implements RouterConfig { ); assert(() { - log.info('setting initial location $initialLocation'); + log('setting initial location $initialLocation'); return true; }()); } @@ -338,13 +338,13 @@ class GoRouter implements RouterConfig { /// Navigate to a URI location w/ optional query parameters, e.g. /// `/family/f2/person/p1?color=blue` void go(String location, {Object? extra}) { - log.info('going to $location'); + log('going to $location'); routeInformationProvider.go(location, extra: extra); } /// Restore the RouteMatchList void restore(RouteMatchList matchList) { - log.info('restoring ${matchList.uri}'); + log('restoring ${matchList.uri}'); routeInformationProvider.restore( matchList.uri.toString(), matchList: matchList, @@ -376,7 +376,7 @@ class GoRouter implements RouterConfig { /// it as the same page. The page key will be reused. This will preserve the /// state and not run any page animation. Future push(String location, {Object? extra}) async { - log.info('pushing $location'); + log('pushing $location'); return routeInformationProvider.push( location, base: routerDelegate.currentConfiguration, @@ -409,7 +409,7 @@ class GoRouter implements RouterConfig { /// state and not run any page animation. Future pushReplacement(String location, {Object? extra}) { - log.info('pushReplacement $location'); + log('pushReplacement $location'); return routeInformationProvider.pushReplacement( location, base: routerDelegate.currentConfiguration, @@ -448,7 +448,7 @@ class GoRouter implements RouterConfig { /// * [pushReplacement] which replaces the top-most page of the page stack but /// always uses a new page key. Future replace(String location, {Object? extra}) { - log.info('replace $location'); + log('replace $location'); return routeInformationProvider.replace( location, base: routerDelegate.currentConfiguration, @@ -486,7 +486,7 @@ class GoRouter implements RouterConfig { /// of any GoRoute under it. void pop([T? result]) { assert(() { - log.info('popping ${routerDelegate.currentConfiguration.uri}'); + log('popping ${routerDelegate.currentConfiguration.uri}'); return true; }()); routerDelegate.pop(result); @@ -495,7 +495,7 @@ class GoRouter implements RouterConfig { /// Refresh the route. void refresh() { assert(() { - log.info('refreshing ${routerDelegate.currentConfiguration.uri}'); + log('refreshing ${routerDelegate.currentConfiguration.uri}'); return true; }()); routeInformationProvider.notifyListeners(); diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index 746150e0fdd..c4d4154fdd9 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: 11.1.1 +version: 11.1.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 diff --git a/packages/go_router/test/logging_test.dart b/packages/go_router/test/logging_test.dart index d6e278610fb..3ae24561751 100644 --- a/packages/go_router/test/logging_test.dart +++ b/packages/go_router/test/logging_test.dart @@ -2,17 +2,70 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:async'; + +import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:go_router/go_router.dart'; import 'package:go_router/src/logging.dart'; import 'package:logging/logging.dart'; void main() { test('setLogging does not clear listeners', () { - log.onRecord - .listen(expectAsync1((LogRecord r) {}, count: 2)); + final StreamSubscription subscription = logger.onRecord.listen( + expectAsync1((LogRecord r) {}, count: 2), + ); + addTearDown(subscription.cancel); + setLogging(enabled: true); - log.info('message'); + logger.info('message'); setLogging(); - log.info('message'); + logger.info('message'); }); + + testWidgets( + 'It should not log anything the if debugLogDiagnostics is false', + (WidgetTester tester) async { + final StreamSubscription subscription = + Logger.root.onRecord.listen( + expectAsync1((LogRecord data) {}, count: 0), + ); + addTearDown(subscription.cancel); + GoRouter( + routes: [ + GoRoute( + path: '/', + builder: (_, GoRouterState state) => const Text('home'), + ), + ], + ); + }, + ); + + testWidgets( + 'It should not log the known routes and the initial route if debugLogDiagnostics is true', + (WidgetTester tester) async { + final List logs = []; + Logger.root.onRecord.listen( + (LogRecord event) => logs.add(event.message), + ); + GoRouter( + debugLogDiagnostics: true, + routes: [ + GoRoute( + path: '/', + builder: (_, GoRouterState state) => const Text('home'), + ), + ], + ); + + expect( + logs, + const [ + 'Full paths for routes:\n => /\n', + 'setting initial location null' + ], + ); + }, + ); }