FED-466: Fix console logs that require formatting

lib/src/util/console_log_formatter.dart

@@ -25,6 +25,8 @@
 @JS()
 library console_log_formatter;
 
+import 'dart:developer';
+
 import 'package:js/js.dart';
 
 @JS('JSON.stringify')
@@ -53,7 +55,9 @@ String format(dynamic f, List<dynamic> arguments) {
           return num.tryParse(arguments[i++].toString()).toString();
         case '%j':
           try {
-            return _jsonStringify([arguments[i++]]);
+            final argToStringify = arguments[i++];
+            debugger();
+            return _jsonStringify(argToStringify);
           } catch (_) {
             return x;
           }

lib/src/util/console_log_utils.dart

@@ -14,9 +14,15 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+@JS()
+library console_log_utils;
+
 import 'dart:async';
+import 'dart:html';
 import 'dart:js';
+import 'dart:js_util';
 
+import 'package:js/js.dart';
 import 'package:meta/meta.dart';
 import 'package:react/react_client/react_interop.dart';
 
@@ -74,6 +80,8 @@ T spyOnConsoleLogs<T>(
   }
 }
 
+get _console => getProperty(window, 'console');
+
 /// Starts spying on console logs, calling [onLog] for each log that occurs until the
 /// returned function (`stopSpying`) is called.
 ///
@@ -91,26 +99,37 @@ void Function() startSpyingOnConsoleLogs({
   @required void Function(String) onLog,
 }) {
   final logTypeToCapture = configuration.logType == 'all' ? ConsoleConfig.types : [configuration.logType];
-  final consoleRefs = <String, JsFunction>{};
+  final consoleRefs = <String, dynamic>{};
+  final consolePropertyDescriptors = <String, dynamic>{};
 
   _resetPropTypeWarningCache();
 
   // Bind to the current zone so the callback isn't called in the top-level zone.
   final boundOnLog = Zone.current.bindUnaryCallback(onLog);
 
   for (final config in logTypeToCapture) {
-    consoleRefs[config] = context['console'][config] as JsFunction;
-    context['console'][config] = JsFunction.withThis((self, [message, arg1, arg2, arg3, arg4, arg5]) {
-      // NOTE: Using console.log or print within this function will cause an infinite
-      // loop when the logType is set to `log`.
-      boundOnLog(format(message?.toString(), [arg1, arg2, arg3, arg4, arg5]));
-      consoleRefs[config].apply([message, arg1, arg2, arg3, arg4, arg5], thisArg: self);
-    });
+    consolePropertyDescriptors[config] = _getOwnPropertyDescriptor(_console, config);
+    consoleRefs[config] = context['console'][config];
+    final newDescriptor = _assign(
+      newObject(),
+      consolePropertyDescriptors[config],
+      jsify({
+        'value': allowInteropCaptureThis((self,
+            [message, arg1 = _undefined, arg2 = _undefined, arg3 = _undefined, arg4 = _undefined, arg5 = _undefined]) {
+          // NOTE: Using console.log or print within this function will cause an infinite
+          // loop when the logType is set to `log`.
+          final args = [arg1, arg2, arg3, arg4, arg5]..removeWhere((arg) => arg == _undefined);
+          boundOnLog(format(message?.toString(), args));
+          consoleRefs[config].apply([message, ...args], thisArg: self);
+        })
+      }),
+    );
+    _defineProperty(_console, config, newDescriptor);
   }
 
   void stopSpying() {
     for (final config in logTypeToCapture) {
-      context['console'][config] = consoleRefs[config];
+      _defineProperty(_console, config, consolePropertyDescriptors[config]);
     }
   }
 
@@ -154,3 +173,25 @@ class ConsoleConfig {
   /// Captures calls to `console.log`, `console.warn`, and `console.error`.
   static const ConsoleConfig all = ConsoleConfig._('all');
 }
+
+const _undefined = Undefined();
+
+class Undefined {
+  const Undefined();
+}
+
+@JS('Object.assign')
+external dynamic _assign(dynamic object, dynamic otherObject, [dynamic anotherObject]);
+
+@JS('Object.getOwnPropertyDescriptor')
+external _PropertyDescriptor _getOwnPropertyDescriptor(dynamic object, String propertyName);
+
+@JS('Object.defineProperty')
+external void _defineProperty(dynamic object, String propertyName, dynamic descriptor);
+
+@JS('Object.prototype.hasOwnProperty')
+external bool _hasOwnProperty(dynamic object, String name);
+
+@JS()
+@anonymous
+class _PropertyDescriptor {}

test/unit/console_log_utils_test.dart

@@ -139,10 +139,10 @@ void main() {
           // the test package.
           final printCalls = recordPrintCalls(() {
             printConsoleLogs(() {
-              callMethod(getProperty(window, 'console'), 'log', ['%s World Number %d! %j', 'Hello']);
+              consoleLog(['%s World Number %d!', 'Hello', 5]);
             });
           });
-          expect(printCalls, ['Hello World Number 5! {"doWeComeInPeace":false} additional']);
+          expect(printCalls, ['Hello World Number 5!']);
         });
 
         test('with too many args', () {
@@ -152,11 +152,12 @@ void main() {
           // the test package.
           final printCalls = recordPrintCalls(() {
             printConsoleLogs(() {
-              callMethod(getProperty(window, 'console'), 'log', [
+              consoleLog([
                 '%s World Number %d! %j',
                 'Hello',
                 5,
-                jsify({'doWeComeInPeace': false})
+                jsify({'doWeComeInPeace': false}),
+                'additional'
               ]);
             });
           });
@@ -170,10 +171,10 @@ void main() {
           // the test package.
           final printCalls = recordPrintCalls(() {
             printConsoleLogs(() {
-              callMethod(getProperty(window, 'console'), 'log', ['%s World Number %d! %j', 'Hello']);
+              consoleLog(['%s World Number %d! %j', 'Hello']);
             });
           });
-          expect(printCalls, ['Hello World Number 5! {"doWeComeInPeace":false} additional']);
+          expect(printCalls, ['Hello World Number %d! %j']);
         });
       });
 
@@ -230,7 +231,16 @@ void main() {
         ];
 
         if (runtimeSupportsPropTypeWarnings()) {
-          expect(logs, unorderedEquals([...expectedLogs, contains('shouldNeverBeNull is necessary.'), contains('⚠️  Warning:')]));
+          expect(
+            logs,
+            unorderedEquals(
+              [
+                ...expectedLogs,
+                contains('shouldNeverBeNull is necessary.'),
+                contains('⚠️  Warning:'),
+              ],
+            ),
+          );
         } else {
           expect(logs, unorderedEquals(expectedLogs));
         }
@@ -267,7 +277,10 @@ void main() {
     if (runtimeSupportsPropTypeWarnings()) {
       group('captures errors correctly', () {
         test('when mounting', () {
-          final logs = recordConsoleLogs(() => rtl.render(Sample({'shouldAlwaysBeFalse': true}) as ReactElement), configuration: ConsoleConfig.error);
+          final logs = recordConsoleLogs(
+            () => rtl.render(Sample({'shouldAlwaysBeFalse': true}) as ReactElement),
+            configuration: ConsoleConfig.error,
+          );
 
           expect(
               logs,
@@ -282,14 +295,17 @@ void main() {
           final view = rtl.render(Sample({'shouldAlwaysBeFalse': true}) as ReactElement);
 
           // Should clear the error from mounting and not create any more
-          final logs =
-              recordConsoleLogs(() => view.rerender(Sample({'shouldNeverBeNull': true}) as ReactElement), configuration: ConsoleConfig.error);
+          final logs = recordConsoleLogs(
+            () => view.rerender(Sample({'shouldNeverBeNull': true}) as ReactElement),
+            configuration: ConsoleConfig.error,
+          );
 
           expect(logs, hasLength(0));
         });
 
         test('with nested components', () {
-          final logs = recordConsoleLogs(() => rtl.render(Sample({}, Sample2({})) as ReactElement), configuration: ConsoleConfig.error);
+          final logs = recordConsoleLogs(() => rtl.render(Sample({}, Sample2({})) as ReactElement),
+              configuration: ConsoleConfig.error);
 
           expect(
               logs,
@@ -300,7 +316,10 @@ void main() {
         });
 
         test('with nested components that are the same', () {
-          final logs = recordConsoleLogs(() => rtl.render(Sample({}, Sample({})) as ReactElement), configuration: ConsoleConfig.error);
+          final logs = recordConsoleLogs(
+            () => rtl.render(Sample({}, Sample({})) as ReactElement),
+            configuration: ConsoleConfig.error,
+          );
 
           expect(
               logs,
@@ -335,7 +354,10 @@ void main() {
         final view = rtl.render(Sample({}) as ReactElement);
 
         // Should clear the previous log and result in there being two
-        final logs = recordConsoleLogs(() => view.rerender(Sample({'addExtraLogAndWarn': true}) as ReactElement), configuration: ConsoleConfig.log);
+        final logs = recordConsoleLogs(
+          () => view.rerender(Sample({'addExtraLogAndWarn': true}) as ReactElement),
+          configuration: ConsoleConfig.log,
+        );
 
         expect(
             logs,
@@ -346,7 +368,10 @@ void main() {
       });
 
       test('with nested components', () {
-        final logs = recordConsoleLogs(() => rtl.render(Sample({}, Sample2({})) as ReactElement), configuration: ConsoleConfig.log);
+        final logs = recordConsoleLogs(
+          () => rtl.render(Sample({}, Sample2({})) as ReactElement),
+          configuration: ConsoleConfig.log,
+        );
 
         if (runtimeSupportsPropTypeWarnings()) {
           expect(
@@ -371,7 +396,10 @@ void main() {
       });
 
       test('with nested components that are the same', () {
-        final logs = recordConsoleLogs(() => rtl.render(Sample({}, Sample({})) as ReactElement), configuration: ConsoleConfig.log);
+        final logs = recordConsoleLogs(
+          () => rtl.render(Sample({}, Sample({})) as ReactElement),
+          configuration: ConsoleConfig.log,
+        );
 
         if (runtimeSupportsPropTypeWarnings()) {
           expect(
@@ -413,7 +441,10 @@ void main() {
         final view = rtl.render(Sample({}) as ReactElement);
 
         // Should clear the previous warnings and result in there being 3
-        final logs = recordConsoleLogs(() => view.rerender(Sample({'addExtraLogAndWarn': true}) as ReactElement), configuration: ConsoleConfig.warn);
+        final logs = recordConsoleLogs(
+          () => view.rerender(Sample({'addExtraLogAndWarn': true}) as ReactElement),
+          configuration: ConsoleConfig.warn,
+        );
 
         expect(
             logs,
@@ -425,13 +456,19 @@ void main() {
       });
 
       test('with nested components', () {
-        final logs = recordConsoleLogs(() => rtl.render(Sample({}, Sample2({})) as ReactElement), configuration: ConsoleConfig.warn);
+        final logs = recordConsoleLogs(
+          () => rtl.render(Sample({}, Sample2({})) as ReactElement),
+          configuration: ConsoleConfig.warn,
+        );
 
         expect(logs, hasLength(6));
       });
 
       test('with nested components that are the same', () {
-        final logs = recordConsoleLogs(() => rtl.render(Sample({}, Sample({})) as ReactElement), configuration: ConsoleConfig.warn);
+        final logs = recordConsoleLogs(
+          () => rtl.render(Sample({}, Sample({})) as ReactElement),
+          configuration: ConsoleConfig.warn,
+        );
 
         expect(logs, hasLength(6));
       });
@@ -566,3 +603,5 @@ class _Sample2Component extends react.Component2 {
 
 // ignore: type_annotate_public_apis
 final Sample2 = react.registerComponent2(() => _Sample2Component());
+
+void consoleLog(List<dynamic> args) => callMethod(getProperty(window, 'console'), 'log', args);