[go_router] Avoid logging when debugLogDiagnostics is false (#4875)

Fixes flutter/flutter#134187

Author: ValentinVignal

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`.

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);

packages/go_router/lib/src/configuration.dart

@@ -32,7 +32,7 @@ class RouteConfiguration {
             routes, <GlobalKey<NavigatorState>>[navigatorKey])) {
     assert(_debugCheckStatefulShellBranchDefaultLocations(routes));
     _cacheNameToPath('', routes);
-    log.info(debugKnownRoutes());
+    log(debugKnownRoutes());
   }
 
   static bool _debugCheckPath(List<RouteBase> routes, bool isTopLevel) {
@@ -234,7 +234,7 @@ class RouteConfiguration {
     Map<String, dynamic> queryParameters = const <String, dynamic>{},
   }) {
     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<RouteMatchList> redirections) {

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<LogRecord>? _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

packages/go_router/lib/src/parser.dart

@@ -89,7 +89,7 @@ class GoRouteInformationParser extends RouteInformationParser<RouteMatchList> {
       // 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(

packages/go_router/lib/src/router.dart

@@ -161,7 +161,7 @@ class GoRouter implements RouterConfig<RouteMatchList> {
     );
 
     assert(() {
-      log.info('setting initial location $initialLocation');
+      log('setting initial location $initialLocation');
       return true;
     }());
   }
@@ -338,13 +338,13 @@ class GoRouter implements RouterConfig<RouteMatchList> {
   /// 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<RouteMatchList> {
   ///   it as the same page. The page key will be reused. This will preserve the
   ///   state and not run any page animation.
   Future<T?> push<T extends Object?>(String location, {Object? extra}) async {
-    log.info('pushing $location');
+    log('pushing $location');
     return routeInformationProvider.push<T>(
       location,
       base: routerDelegate.currentConfiguration,
@@ -409,7 +409,7 @@ class GoRouter implements RouterConfig<RouteMatchList> {
   ///   state and not run any page animation.
   Future<T?> pushReplacement<T extends Object?>(String location,
       {Object? extra}) {
-    log.info('pushReplacement $location');
+    log('pushReplacement $location');
     return routeInformationProvider.pushReplacement<T>(
       location,
       base: routerDelegate.currentConfiguration,
@@ -448,7 +448,7 @@ class GoRouter implements RouterConfig<RouteMatchList> {
   /// * [pushReplacement] which replaces the top-most page of the page stack but
   ///   always uses a new page key.
   Future<T?> replace<T>(String location, {Object? extra}) {
-    log.info('replace $location');
+    log('replace $location');
     return routeInformationProvider.replace<T>(
       location,
       base: routerDelegate.currentConfiguration,
@@ -486,7 +486,7 @@ class GoRouter implements RouterConfig<RouteMatchList> {
   /// of any GoRoute under it.
   void pop<T extends Object?>([T? result]) {
     assert(() {
-      log.info('popping ${routerDelegate.currentConfiguration.uri}');
+      log('popping ${routerDelegate.currentConfiguration.uri}');
       return true;
     }());
     routerDelegate.pop<T>(result);
@@ -495,7 +495,7 @@ class GoRouter implements RouterConfig<RouteMatchList> {
   /// Refresh the route.
   void refresh() {
     assert(() {
-      log.info('refreshing ${routerDelegate.currentConfiguration.uri}');
+      log('refreshing ${routerDelegate.currentConfiguration.uri}');
       return true;
     }());
     routeInformationProvider.notifyListeners();

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
 

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<void, LogRecord>((LogRecord r) {}, count: 2));
+    final StreamSubscription<LogRecord> subscription = logger.onRecord.listen(
+      expectAsync1<void, LogRecord>((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<LogRecord> subscription =
+          Logger.root.onRecord.listen(
+        expectAsync1((LogRecord data) {}, count: 0),
+      );
+      addTearDown(subscription.cancel);
+      GoRouter(
+        routes: <RouteBase>[
+          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<String> logs = <String>[];
+      Logger.root.onRecord.listen(
+        (LogRecord event) => logs.add(event.message),
+      );
+      GoRouter(
+        debugLogDiagnostics: true,
+        routes: <RouteBase>[
+          GoRoute(
+            path: '/',
+            builder: (_, GoRouterState state) => const Text('home'),
+          ),
+        ],
+      );
+
+      expect(
+        logs,
+        const <String>[
+          'Full paths for routes:\n  => /\n',
+          'setting initial location null'
+        ],
+      );
+    },
+  );
 }