Skip to content

Commit b171fa2

Browse files
BridgeARrvagg
BridgeAR
authored and
rvagg
committed
util: improve display of iterators and weak entries
This adds the number of not visible elements when inspecting iterators while exceeding `maxArrayLength`. It also fixes a edge case with `maxArrayLength` and the map.entries() iterator. Now the whole entry will be visible instead of only the key but not the value of the first entry. Besides that it uses a slighly better algorithm that improves the performance by skipping unnecessary steps. PR-URL: #20961 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent d95a22c commit b171fa2

File tree

4 files changed

+89
-70
lines changed

4 files changed

+89
-70
lines changed

‎lib/console.js‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -360,8 +360,11 @@ Console.prototype.table = function(tabularData, properties) {
360360
const mapIter = isMapIterator(tabularData);
361361
let isKeyValue = false;
362362
let i = 0;
363-
if (mapIter)
364-
[ tabularData, isKeyValue ] = previewEntries(tabularData);
363+
if (mapIter) {
364+
const res = previewEntries(tabularData, true);
365+
tabularData = res[0];
366+
isKeyValue = res[1];
367+
}
365368

366369
if (isKeyValue || isMap(tabularData)) {
367370
const keys = [];
@@ -391,7 +394,7 @@ Console.prototype.table = function(tabularData, properties) {
391394

392395
const setIter = isSetIterator(tabularData);
393396
if (setIter)
394-
[ tabularData ] = previewEntries(tabularData);
397+
tabularData = previewEntries(tabularData);
395398

396399
const setlike = setIter || (mapIter && !isKeyValue) || isSet(tabularData);
397400
if (setlike) {

‎lib/util.js‎

Lines changed: 54 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,10 @@ const meta = [
115115
'', '', '', '', '', '', '', '', '', '',
116116
'', '', '', '', '', '', '', '\\\\'
117117
];
118+
// Constants to map the iterator state.
119+
const kWeak = 0;
120+
const kIterator = 1;
121+
const kMapEntries = 2;
118122

119123
const escapeFn = (str) => meta[str.charCodeAt(0)];
120124

@@ -982,77 +986,83 @@ function formatMap(ctx, value, recurseTimes, keys) {
982986
return output;
983987
}
984988

985-
function formatWeakSet(ctx, value, recurseTimes, keys) {
989+
function formatSetIterInner(ctx, value, recurseTimes, keys, entries, state) {
986990
const maxArrayLength = Math.max(ctx.maxArrayLength, 0);
987-
const [ entries ] = previewEntries(value).slice(0, maxArrayLength + 1);
988991
const maxLength = Math.min(maxArrayLength, entries.length);
989992
let output = new Array(maxLength);
990993
for (var i = 0; i < maxLength; ++i)
991994
output[i] = formatValue(ctx, entries[i], recurseTimes);
992-
// Sort all entries to have a halfway reliable output (if more entries than
993-
// retrieved ones exist, we can not reliably return the same output).
994-
output = output.sort();
995-
if (entries.length > maxArrayLength)
996-
output.push('... more items');
995+
if (state === kWeak) {
996+
// Sort all entries to have a halfway reliable output (if more entries than
997+
// retrieved ones exist, we can not reliably return the same output).
998+
output = output.sort();
999+
}
1000+
const remaining = entries.length - maxLength;
1001+
if (remaining > 0) {
1002+
output.push(`... ${remaining} more item${remaining > 1 ? 's' : ''}`);
1003+
}
9971004
for (i = 0; i < keys.length; i++)
9981005
output.push(formatProperty(ctx, value, recurseTimes, keys[i], 0));
9991006
return output;
10001007
}
10011008

1002-
function formatWeakMap(ctx, value, recurseTimes, keys) {
1009+
function formatMapIterInner(ctx, value, recurseTimes, keys, entries, state) {
10031010
const maxArrayLength = Math.max(ctx.maxArrayLength, 0);
1004-
const [ entries ] = previewEntries(value).slice(0, (maxArrayLength + 1) * 2);
10051011
// Entries exist as [key1, val1, key2, val2, ...]
1006-
const remainder = entries.length / 2 > maxArrayLength;
1007-
const len = entries.length / 2 - (remainder ? 1 : 0);
1012+
const len = entries.length / 2;
1013+
const remaining = len - maxArrayLength;
10081014
const maxLength = Math.min(maxArrayLength, len);
10091015
let output = new Array(maxLength);
1010-
for (var i = 0; i < maxLength; i++) {
1016+
let start = '';
1017+
let end = '';
1018+
let middle = ' => ';
1019+
let i = 0;
1020+
if (state === kMapEntries) {
1021+
start = '[ ';
1022+
end = ' ]';
1023+
middle = ', ';
1024+
}
1025+
for (; i < maxLength; i++) {
10111026
const pos = i * 2;
1012-
output[i] = `${formatValue(ctx, entries[pos], recurseTimes)} => ` +
1013-
formatValue(ctx, entries[pos + 1], recurseTimes);
1027+
output[i] = `${start}${formatValue(ctx, entries[pos], recurseTimes)}` +
1028+
`${middle}${formatValue(ctx, entries[pos + 1], recurseTimes)}${end}`;
1029+
}
1030+
if (state === kWeak) {
1031+
// Sort all entries to have a halfway reliable output (if more entries
1032+
// than retrieved ones exist, we can not reliably return the same output).
1033+
output = output.sort();
1034+
}
1035+
if (remaining > 0) {
1036+
output.push(`... ${remaining} more item${remaining > 1 ? 's' : ''}`);
10141037
}
1015-
// Sort all entries to have a halfway reliable output (if more entries than
1016-
// retrieved ones exist, we can not reliably return the same output).
1017-
output = output.sort();
1018-
if (remainder > 0)
1019-
output.push('... more items');
10201038
for (i = 0; i < keys.length; i++)
10211039
output.push(formatProperty(ctx, value, recurseTimes, keys[i], 0));
10221040
return output;
10231041
}
10241042

1025-
function zip2(list) {
1026-
const ret = Array(list.length / 2);
1027-
for (var i = 0; i < ret.length; ++i)
1028-
ret[i] = [list[2 * i], list[2 * i + 1]];
1029-
return ret;
1043+
function formatWeakSet(ctx, value, recurseTimes, keys) {
1044+
const entries = previewEntries(value);
1045+
return formatSetIterInner(ctx, value, recurseTimes, keys, entries, kWeak);
10301046
}
10311047

1032-
function formatCollectionIterator(ctx, value, recurseTimes, keys) {
1033-
const output = [];
1034-
var [ entries, isKeyValue ] = previewEntries(value);
1035-
if (isKeyValue)
1036-
entries = zip2(entries);
1037-
for (const entry of entries) {
1038-
if (ctx.maxArrayLength === output.length) {
1039-
output.push('... more items');
1040-
break;
1041-
}
1042-
output.push(formatValue(ctx, entry, recurseTimes));
1043-
}
1044-
for (var n = 0; n < keys.length; n++) {
1045-
output.push(formatProperty(ctx, value, recurseTimes, keys[n], 0));
1046-
}
1047-
return output;
1048+
function formatWeakMap(ctx, value, recurseTimes, keys) {
1049+
const entries = previewEntries(value);
1050+
return formatMapIterInner(ctx, value, recurseTimes, keys, entries, kWeak);
10481051
}
10491052

1050-
function formatMapIterator(ctx, value, recurseTimes, keys) {
1051-
return formatCollectionIterator(ctx, value, recurseTimes, keys);
1053+
function formatSetIterator(ctx, value, recurseTimes, keys) {
1054+
const entries = previewEntries(value);
1055+
return formatSetIterInner(ctx, value, recurseTimes, keys, entries, kIterator);
10521056
}
10531057

1054-
function formatSetIterator(ctx, value, recurseTimes, keys) {
1055-
return formatCollectionIterator(ctx, value, recurseTimes, keys);
1058+
function formatMapIterator(ctx, value, recurseTimes, keys) {
1059+
const [entries, isKeyValue] = previewEntries(value, true);
1060+
if (isKeyValue) {
1061+
return formatMapIterInner(
1062+
ctx, value, recurseTimes, keys, entries, kMapEntries);
1063+
}
1064+
1065+
return formatSetIterInner(ctx, value, recurseTimes, keys, entries, kIterator);
10561066
}
10571067

10581068
function formatPromise(ctx, value, recurseTimes, keys) {

‎src/node_util.cc‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ static void PreviewEntries(const FunctionCallbackInfo<Value>& args) {
5757
Local<Array> entries;
5858
if (!args[0].As<Object>()->PreviewEntries(&is_key_value).ToLocal(&entries))
5959
return;
60+
// Fast path for WeakMap, WeakSet and Set iterators.
61+
if (args.Length() == 1)
62+
return args.GetReturnValue().Set(entries);
6063
Local<Array> ret = Array::New(env->isolate(), 2);
6164
ret->Set(env->context(), 0, entries).FromJust();
6265
ret->Set(env->context(), 1, v8::Boolean::New(env->isolate(), is_key_value))

‎test/parallel/test-util-inspect.js‎

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -447,13 +447,15 @@ assert.strictEqual(util.inspect(-5e-324), '-5e-324');
447447
{
448448
const map = new Map();
449449
map.set(1, 2);
450-
const [ vals ] = previewEntries(map.entries());
451-
const valsOutput = [];
452-
for (const o of vals) {
453-
valsOutput.push(o);
454-
}
455-
456-
assert.strictEqual(util.inspect(valsOutput), '[ 1, 2 ]');
450+
// Passing only a single argument to indicate a set iterator.
451+
const valsSetIterator = previewEntries(map.entries());
452+
// Passing through true to indicate a map iterator.
453+
const valsMapIterEntries = previewEntries(map.entries(), true);
454+
const valsMapIterKeys = previewEntries(map.keys(), true);
455+
456+
assert.strictEqual(util.inspect(valsSetIterator), '[ 1, 2 ]');
457+
assert.strictEqual(util.inspect(valsMapIterEntries), '[ [ 1, 2 ], true ]');
458+
assert.strictEqual(util.inspect(valsMapIterKeys), '[ [ 1 ], false ]');
457459
}
458460

459461
// Test for other constructors in different context.
@@ -965,18 +967,19 @@ if (typeof Symbol !== 'undefined') {
965967
// Test Map iterators.
966968
{
967969
const map = new Map([['foo', 'bar']]);
968-
assert.strictEqual(util.inspect(map.keys()), "[Map Iterator] { 'foo' }");
969-
assert.strictEqual(util.inspect(map.values()), "[Map Iterator] { 'bar' }");
970-
assert.strictEqual(util.inspect(map.entries()),
971-
"[Map Iterator] { [ 'foo', 'bar' ] }");
970+
assert.strictEqual(util.inspect(map.keys()), '[Map Iterator] { \'foo\' }');
971+
assert.strictEqual(util.inspect(map.values()), '[Map Iterator] { \'bar\' }');
972+
map.set('A', 'B!');
973+
assert.strictEqual(util.inspect(map.entries(), { maxArrayLength: 1 }),
974+
"[Map Iterator] { [ 'foo', 'bar' ], ... 1 more item }");
972975
// Make sure the iterator doesn't get consumed.
973976
const keys = map.keys();
974-
assert.strictEqual(util.inspect(keys), "[Map Iterator] { 'foo' }");
975-
assert.strictEqual(util.inspect(keys), "[Map Iterator] { 'foo' }");
977+
assert.strictEqual(util.inspect(keys), "[Map Iterator] { 'foo', 'A' }");
978+
assert.strictEqual(util.inspect(keys), "[Map Iterator] { 'foo', 'A' }");
976979
keys.extra = true;
977980
assert.strictEqual(
978981
util.inspect(keys, { maxArrayLength: 0 }),
979-
'[Map Iterator] { ... more items, extra: true }');
982+
'[Map Iterator] { ... 2 more items, extra: true }');
980983
}
981984

982985
// Test Set iterators.
@@ -992,7 +995,7 @@ if (typeof Symbol !== 'undefined') {
992995
keys.extra = true;
993996
assert.strictEqual(
994997
util.inspect(keys, { maxArrayLength: 1 }),
995-
'[Set Iterator] { 1, ... more items, extra: true }');
998+
'[Set Iterator] { 1, ... 1 more item, extra: true }');
996999
}
9971000

9981001
// Test alignment of items in container.
@@ -1413,17 +1416,17 @@ util.inspect(process);
14131416
assert.strictEqual(out, expect);
14141417

14151418
out = util.inspect(weakMap, { maxArrayLength: 0, showHidden: true });
1416-
expect = 'WeakMap { ... more items }';
1419+
expect = 'WeakMap { ... 2 more items }';
14171420
assert.strictEqual(out, expect);
14181421

14191422
weakMap.extra = true;
14201423
out = util.inspect(weakMap, { maxArrayLength: 1, showHidden: true });
14211424
// It is not possible to determine the output reliable.
1422-
expect = 'WeakMap { [ [length]: 0 ] => {}, ... more items, extra: true }';
1423-
const expectAlt = 'WeakMap { {} => [ [length]: 0 ], ... more items, ' +
1425+
expect = 'WeakMap { [ [length]: 0 ] => {}, ... 1 more item, extra: true }';
1426+
const expectAlt = 'WeakMap { {} => [ [length]: 0 ], ... 1 more item, ' +
14241427
'extra: true }';
14251428
assert(out === expect || out === expectAlt,
1426-
`Found "${out}" rather than "${expect}" or "${expectAlt}"`);
1429+
`Found: "${out}"\nrather than: "${expect}"\nor: "${expectAlt}"`);
14271430
}
14281431

14291432
{ // Test WeakSet
@@ -1437,17 +1440,17 @@ util.inspect(process);
14371440
assert.strictEqual(out, expect);
14381441

14391442
out = util.inspect(weakSet, { maxArrayLength: -2, showHidden: true });
1440-
expect = 'WeakSet { ... more items }';
1443+
expect = 'WeakSet { ... 2 more items }';
14411444
assert.strictEqual(out, expect);
14421445

14431446
weakSet.extra = true;
14441447
out = util.inspect(weakSet, { maxArrayLength: 1, showHidden: true });
14451448
// It is not possible to determine the output reliable.
1446-
expect = 'WeakSet { {}, ... more items, extra: true }';
1447-
const expectAlt = 'WeakSet { [ 1, [length]: 1 ], ... more items, ' +
1449+
expect = 'WeakSet { {}, ... 1 more item, extra: true }';
1450+
const expectAlt = 'WeakSet { [ 1, [length]: 1 ], ... 1 more item, ' +
14481451
'extra: true }';
14491452
assert(out === expect || out === expectAlt,
1450-
`Found "${out}" rather than "${expect}" or "${expectAlt}"`);
1453+
`Found: "${out}"\nrather than: "${expect}"\nor: "${expectAlt}"`);
14511454
}
14521455

14531456
{ // Test argument objects.

0 commit comments

Comments
 (0)