Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FED-466: Fix console logs that require formatting #52

Merged
merged 14 commits into from
Oct 20, 2022
Merged
76 changes: 76 additions & 0 deletions lib/src/util/console_log_formatter.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// This code was adapted to Dart from
// https://github.com/nodejs/node-v0.x-archive/blob/master/lib/util.js
//
// Copyright Joyent, Inc. and other Node contributors.
//
// Permission is hereby granted, free of charge, to any person obtaining a
// copy of this software and associated documentation files (the
// "Software"), to deal in the Software without restriction, including
// without limitation the rights to use, copy, modify, merge, publish,
// distribute, sublicense, and/or sell copies of the Software, and to permit
// persons to whom the Software is furnished to do so, subject to the
// following conditions:
//
// The above copyright notice and this permission notice shall be included
// in all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
// USE OR OTHER DEALINGS IN THE SOFTWARE.

@JS()
library console_log_formatter;

import 'dart:developer';

import 'package:js/js.dart';

@JS('JSON.stringify')
external String _jsonStringify(dynamic object);

final _formatRegExp = RegExp('%[sdj%]');

/// A doc comment
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You want me to close this PR? 😛

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🖕 get out of here friday greg its wednesday.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is worse than no doc comment lol

String format(dynamic f, List<dynamic> arguments) {
if (f is! String) {
return [f, ...arguments].join(' ');
}

var str = '';
if (f is String) {
var i = 0;
final len = arguments.length;
str += f.replaceAllMapped(_formatRegExp, (m) {
final x = m[0];
if (x == '%%') return '%';
if (i >= len) return x;
switch (x) {
case '%s':
return arguments[i++].toString();
case '%d':
return num.tryParse(arguments[i++].toString()).toString();
case '%j':
try {
final argToStringify = arguments[i++];
debugger();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this and the import were committed by mistake.

return _jsonStringify(argToStringify);
} catch (_) {
return x;
}
break;
default:
return x;
}
});

if (i < len) {
str += ' ${arguments.skip(i).join(' ')}';
}
}

return str;
}
61 changes: 52 additions & 9 deletions lib/src/util/console_log_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,20 @@
// 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';

import 'console_log_formatter.dart';

/// Runs a provided [callback] and afterwards [print]s each log that occurs during the runtime
/// of that function.
///
Expand Down Expand Up @@ -72,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.
///
Expand All @@ -89,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(message?.toString());
consoleRefs[config].apply([message, arg1, arg2, arg3, arg4, arg5], thisArg: self);
});
consolePropertyDescriptors[config] = _getOwnPropertyDescriptor(_console, config);
consoleRefs[config] = context['console'][config];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we restore the cast that was previously here along with the typing on consoleRefs?

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

Expand Down Expand Up @@ -152,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 {}
116 changes: 95 additions & 21 deletions test/unit/console_log_utils_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import 'dart:async';
import 'dart:html';
import 'dart:js_util';

import 'package:react/react.dart' as react;
import 'package:react/react_client/react_interop.dart';
Expand Down Expand Up @@ -130,6 +131,53 @@ void main() {
expect(printCalls, ['foo', 'bar']);
});

group('prints logs that use formatter syntax', () {
test('with exact amount of args', () {
// This also tests functionally that the print call occurs in the right zone,
// which need to happen for them to be forwarded in the terminal in a test environment.
// If it wasn't in the right zone, we wouldn't be able to record it, and neither would
// the test package.
final printCalls = recordPrintCalls(() {
printConsoleLogs(() {
consoleLog(['%s World Number %d!', 'Hello', 5]);
});
});
expect(printCalls, ['Hello World Number 5!']);
});

test('with too many args', () {
// This also tests functionally that the print call occurs in the right zone,
// which need to happen for them to be forwarded in the terminal in a test environment.
// If it wasn't in the right zone, we wouldn't be able to record it, and neither would
// the test package.
final printCalls = recordPrintCalls(() {
printConsoleLogs(() {
consoleLog([
'%s World Number %d! %j',
'Hello',
5,
jsify({'doWeComeInPeace': false}),
'additional'
]);
});
});
expect(printCalls, ['Hello World Number 5! {"doWeComeInPeace":false} additional']);
});

test('without enough args', () {
// This also tests functionally that the print call occurs in the right zone,
// which need to happen for them to be forwarded in the terminal in a test environment.
// If it wasn't in the right zone, we wouldn't be able to record it, and neither would
// the test package.
final printCalls = recordPrintCalls(() {
printConsoleLogs(() {
consoleLog(['%s World Number %d! %j', 'Hello']);
});
});
expect(printCalls, ['Hello World Number %d! %j']);
});
});

test('prints even if the function throws partway through', () {
final printCalls = recordPrintCalls(() {
expect(() {
Expand Down Expand Up @@ -184,9 +232,15 @@ void main() {

if (runtimeSupportsPropTypeWarnings()) {
expect(
logs,
unorderedEquals(
[...expectedLogs, contains('shouldNeverBeNull is necessary.'), contains('⚠️ Warning:')]));
logs,
unorderedEquals(
[
...expectedLogs,
contains('shouldNeverBeNull is necessary.'),
contains('⚠️ Warning:'),
],
),
);
} else {
expect(logs, unorderedEquals(expectedLogs));
}
Expand Down Expand Up @@ -223,8 +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,
Expand All @@ -239,8 +295,10 @@ 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));
});
Expand All @@ -258,8 +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,
Expand Down Expand Up @@ -294,8 +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,
Expand All @@ -306,8 +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(
Expand All @@ -332,8 +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(
Expand Down Expand Up @@ -375,8 +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,
Expand All @@ -388,15 +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));
});
Expand Down Expand Up @@ -531,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);