-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
util: support inspecting Map, Set, and Promise #1471
Changes from all commits
613a63b
21e4e92
87e36a6
10d88e5
1185ff3
c4d2cd0
ba53e97
79627ae
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
'use strict'; | ||
|
||
const uv = process.binding('uv'); | ||
const Debug = require('vm').runInDebugContext('Debug'); | ||
|
||
const formatRegExp = /%[sdj%]/g; | ||
exports.format = function(f) { | ||
|
@@ -192,6 +193,14 @@ function arrayToHash(array) { | |
} | ||
|
||
|
||
function inspectPromise(p) { | ||
var mirror = Debug.MakeMirror(p, true); | ||
if (!mirror.isPromise()) | ||
return null; | ||
return {status: mirror.status(), value: mirror.promiseValue().value_}; | ||
} | ||
|
||
|
||
function formatValue(ctx, value, recurseTimes) { | ||
// Provide a hook for user-specified inspect functions. | ||
// Check that value is an object with an inspect function on it | ||
|
@@ -276,14 +285,43 @@ function formatValue(ctx, value, recurseTimes) { | |
} | ||
} | ||
|
||
var base = '', array = false, braces = ['{', '}']; | ||
var base = '', empty = false, braces, formatter; | ||
|
||
// Make Array say that they are Array | ||
if (Array.isArray(value)) { | ||
array = true; | ||
braces = ['[', ']']; | ||
empty = value.length === 0; | ||
formatter = formatArray; | ||
} else if (value instanceof Set) { | ||
braces = ['Set {', '}']; | ||
// With `showHidden`, `length` will display as a hidden property for | ||
// arrays. For consistency's sake, do the same for `size`, even though this | ||
// property isn't selected by Object.getOwnPropertyNames(). | ||
if (ctx.showHidden) | ||
keys.unshift('size'); | ||
empty = value.size === 0; | ||
formatter = formatSet; | ||
} else if (value instanceof Map) { | ||
braces = ['Map {', '}']; | ||
// Ditto. | ||
if (ctx.showHidden) | ||
keys.unshift('size'); | ||
empty = value.size === 0; | ||
formatter = formatMap; | ||
} else { | ||
// Only create a mirror if the object superficially looks like a Promise. | ||
var promiseInternals = value instanceof Promise && inspectPromise(value); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just curious, is there a reason to keep the instanceof check out of inspectPromise? |
||
if (promiseInternals) { | ||
braces = ['Promise {', '}']; | ||
formatter = formatPromise; | ||
} else { | ||
braces = ['{', '}']; | ||
empty = true; // No other data than keys. | ||
formatter = formatObject; | ||
} | ||
} | ||
|
||
empty = empty === true && keys.length === 0; | ||
|
||
// Make functions say that they are functions | ||
if (typeof value === 'function') { | ||
var n = value.name ? ': ' + value.name : ''; | ||
|
@@ -323,7 +361,7 @@ function formatValue(ctx, value, recurseTimes) { | |
base = ' ' + '[Boolean: ' + formatted + ']'; | ||
} | ||
|
||
if (keys.length === 0 && (!array || value.length === 0)) { | ||
if (empty === true) { | ||
return braces[0] + base + braces[1]; | ||
} | ||
|
||
|
@@ -337,14 +375,7 @@ function formatValue(ctx, value, recurseTimes) { | |
|
||
ctx.seen.push(value); | ||
|
||
var output; | ||
if (array) { | ||
output = formatArray(ctx, value, recurseTimes, visibleKeys, keys); | ||
} else { | ||
output = keys.map(function(key) { | ||
return formatProperty(ctx, value, recurseTimes, visibleKeys, key, array); | ||
}); | ||
} | ||
var output = formatter(ctx, value, recurseTimes, visibleKeys, keys); | ||
|
||
ctx.seen.pop(); | ||
|
||
|
@@ -397,6 +428,13 @@ function formatError(value) { | |
} | ||
|
||
|
||
function formatObject(ctx, value, recurseTimes, visibleKeys, keys) { | ||
return keys.map(function(key) { | ||
return formatProperty(ctx, value, recurseTimes, visibleKeys, key, false); | ||
}); | ||
} | ||
|
||
|
||
function formatArray(ctx, value, recurseTimes, visibleKeys, keys) { | ||
var output = []; | ||
for (var i = 0, l = value.length; i < l; ++i) { | ||
|
@@ -417,6 +455,59 @@ function formatArray(ctx, value, recurseTimes, visibleKeys, keys) { | |
} | ||
|
||
|
||
function formatSet(ctx, value, recurseTimes, visibleKeys, keys) { | ||
var output = []; | ||
value.forEach(function(v) { | ||
var nextRecurseTimes = recurseTimes === null ? null : recurseTimes - 1; | ||
var str = formatValue(ctx, v, nextRecurseTimes); | ||
output.push(str); | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: we should be able to do something like |
||
keys.forEach(function(key) { | ||
output.push(formatProperty(ctx, value, recurseTimes, visibleKeys, | ||
key, false)); | ||
}); | ||
return output; | ||
} | ||
|
||
|
||
function formatMap(ctx, value, recurseTimes, visibleKeys, keys) { | ||
var output = []; | ||
value.forEach(function(v, k) { | ||
var nextRecurseTimes = recurseTimes === null ? null : recurseTimes - 1; | ||
var str = formatValue(ctx, k, nextRecurseTimes); | ||
str += ' => '; | ||
str += formatValue(ctx, v, nextRecurseTimes); | ||
output.push(str); | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. similarly, |
||
keys.forEach(function(key) { | ||
output.push(formatProperty(ctx, value, recurseTimes, visibleKeys, | ||
key, false)); | ||
}); | ||
return output; | ||
} | ||
|
||
function formatPromise(ctx, value, recurseTimes, visibleKeys, keys) { | ||
var output = []; | ||
var internals = inspectPromise(value); | ||
if (internals.status === 'pending') { | ||
output.push('<pending>'); | ||
} else { | ||
var nextRecurseTimes = recurseTimes === null ? null : recurseTimes - 1; | ||
var str = formatValue(ctx, internals.value, nextRecurseTimes); | ||
if (internals.status === 'rejected') { | ||
output.push('<rejected> ' + str); | ||
} else { | ||
output.push(str); | ||
} | ||
} | ||
keys.forEach(function(key) { | ||
output.push(formatProperty(ctx, value, recurseTimes, visibleKeys, | ||
key, false)); | ||
}); | ||
return output; | ||
} | ||
|
||
|
||
function formatProperty(ctx, value, recurseTimes, visibleKeys, key, array) { | ||
var name, str, desc; | ||
desc = Object.getOwnPropertyDescriptor(value, key) || { value: value[key] }; | ||
|
@@ -488,7 +579,10 @@ function reduceToSingleString(output, base, braces) { | |
|
||
if (length > 60) { | ||
return braces[0] + | ||
(base === '' ? '' : base + '\n ') + | ||
// If the opening "brace" is too large, like in the case of "Set {", | ||
// we need to force the first item to be on the next line or the | ||
// items will not line up correctly. | ||
(base === '' && braces[0].length === 1 ? '' : base + '\n ') + | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like this case needs tests |
||
' ' + | ||
output.join(',\n ') + | ||
' ' + | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@domenic: Should we add support for
Weak{Set,Map}
inspection throughDebug
as a follow-on to this PR?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noooo that seems very bad, basically exposes iteration to JS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah – I thought it might be a bit fraught with peril; was surprised to see the chrome dev tools expose that info.