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
72 changes: 72 additions & 0 deletions lib/src/util/console_log_formatter.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// 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 '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 {
return _jsonStringify([arguments[i++]]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this isn't callMethod, it shouldn't be wrapped in a list.

Suggested change
return _jsonStringify([arguments[i++]]);
return _jsonStringify(arguments[i++]);

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

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

return str;
}
52 changes: 2 additions & 50 deletions lib/src/util/console_log_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@
// limitations under the License.

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

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 @@ -154,51 +154,3 @@ class ConsoleConfig {
/// Captures calls to `console.log`, `console.warn`, and `console.error`.
static const ConsoleConfig all = ConsoleConfig._('all');
}

final formatRegExp = RegExp('%[sdj%]');
String format(dynamic f, List<dynamic> arguments) {
if (f is! String) {
final objects = [];
for (var i = 0; i < arguments.length; i++) {
objects.add(arguments[i]);
}
return objects.join(' ');
}

var str = '';
if (f is String) {
if (!f.contains('%')) return f;

var i = 0;
final args = arguments;
final len = args.length;
str += f.replaceAllMapped(formatRegExp, (m) {
final x = m[0].toString();
if (x == '%%') return '%';
if (i >= len) return x;
switch (x) {
case '%s':
return args[i++].toString();
case '%d':
return num.tryParse(args[i++].toString()).toString();
case '%j':
try {
return callMethod(getProperty(window,'JSON'), 'stringify', [args[i++]]).toString();
} catch (_) {
return '[Circular]';
}
break;
default:
return x;
}
});

if (i < len) {
for (var x = args[i]; i < len; x = args[i++]) {
str += ' $x';
}
}
}

return str;
}
87 changes: 54 additions & 33 deletions test/unit/console_log_utils_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -131,17 +131,50 @@ void main() {
expect(printCalls, ['foo', 'bar']);
});

test('prints logs that use formatter syntax', () {
// 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(() {
callMethod(getProperty(window, 'console'), 'log', ['%s World Number %d! %j', 'Hello', 5, jsify({'doWeComeInPeace':false}), 'additional']);
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(() {
callMethod(getProperty(window, 'console'), 'log', ['%s World Number %d! %j', 'Hello']);
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 is missing the jsified argument, the number, and the additional arg

});
});
expect(printCalls, ['Hello World Number 5! {"doWeComeInPeace":false} additional']);
});

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(() {
callMethod(getProperty(window, 'console'), 'log', [
'%s World Number %d! %j',
'Hello',
5,
jsify({'doWeComeInPeace': false})
]);
});
});
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(() {
callMethod(getProperty(window, 'console'), 'log', ['%s World Number %d! %j', 'Hello']);
});
});
expect(printCalls, ['Hello World Number 5! {"doWeComeInPeace":false} additional']);
});
expect(printCalls, ['Hello World Number 5! {"doWeComeInPeace":false} additional']);
});

test('prints even if the function throws partway through', () {
Expand Down Expand Up @@ -197,10 +230,7 @@ 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));
}
Expand Down Expand Up @@ -237,8 +267,7 @@ 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);
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 these got formatted differently by mistake

Copy link
Contributor Author

Choose a reason for hiding this comment

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

they bug me... so i re-formatted them in the next commit.


expect(
logs,
Expand All @@ -253,15 +282,14 @@ 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,
Expand All @@ -272,8 +300,7 @@ 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 @@ -308,8 +335,7 @@ 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 @@ -320,8 +346,7 @@ 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 @@ -346,8 +371,7 @@ 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 @@ -389,8 +413,7 @@ 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 @@ -402,15 +425,13 @@ 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