Skip to content

util: don't init Debug if it's not needed yet #8452

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.

Sign up for GitHub

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

Closed
bengl wants to merge 1 commit into from
Closed

util: don't init Debug if it's not needed yet #8452

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
util: don't init Debug if it's not needed yet 
Because any call to util.inspect() with an object results in
inspectPromise() being called, Debug was being initialized even when
it's not needed. Instead, the initialization is placed after the
isPromise check.
  • Loading branch information
@bengl
bengl committed Sep 8, 2016
commit 4ab5ccbd8a470182b2279b3ff05b911034ccaf65
2 changes: 1 addition & 1 deletion lib/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,10 @@ function ensureDebugIsInitialized() {


function inspectPromise(p) {
ensureDebugIsInitialized();
// Only create a mirror if the object is a Promise.
Copy link
Contributor

@mscdex mscdex Sep 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

This comment should probably be moved also

Copy link
Member Author

@bengl bengl Sep 9, 2016

Choose a reason for hiding this comment

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

Where to? Wouldn't the best place for the comment about checking whether it's a promise be right before the line that checks if it's a promise?

Copy link
Contributor

@mscdex mscdex Sep 9, 2016

Choose a reason for hiding this comment

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

I guess to me it's a little confusing since the comment is talking about creating a mirror which isn't done on the next line and the function name does not indicate that a mirror is being created. shrug

if (!binding.isPromise(p))
return null;
ensureDebugIsInitialized();
const mirror = Debug.MakeMirror(p, true);
return {status: mirror.status(), value: mirror.promiseValue().value_};
}
Expand Down