Skip to content

util: use constructor name #14886

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

util: use constructor name #14886

wants to merge 1 commit into from

Conversation

@BridgeAR
Copy link
Member

For circular references util.inspect always prints Array
or Object not matter if it is a subclass or not.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

util

@nodejs-github-bot nodejs-github-bot added the util Issues and PRs related to the built-in util module. label Aug 17, 2017
BridgeAR
BridgeAR commented Aug 17, 2017
View reviewed changes
lib/util.js Outdated
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: this check is redundant as soon as #14881 has landed. Right now Arrays set the constructor to null and the check has to be done again.

@BridgeAR BridgeAR added the semver-major PRs that contain breaking changes and should be released in the next major version. label Aug 17, 2017
@TimothyGu
Copy link
Member

Commit message nit: doesn't this apply to objects that exceed the depth limit rather than circular objects? Or both?

@BridgeAR BridgeAR force-pushed the add-proper-constructor branch from d43d928 to d76998e Compare August 17, 2017 17:08
@BridgeAR
Copy link
Member Author

@TimothyGu you are absolutely right! Fixed

@BridgeAR
Copy link
Member Author

PTAL I changed the commit message and this could land otherwise.

@hiroppy
Copy link
Member

hiroppy commented Aug 26, 2017

CI: https://ci.nodejs.org/job/node-test-pull-request/9845/

@BridgeAR
Copy link
Member Author

I would like to land this after #14881 as that one would otherwise not cleanly land on 8.x. Otherwise I would have already landed that. Marking this as blocked because of that.

@BridgeAR BridgeAR added the blocked PRs that are blocked by other issues or PRs. label Aug 26, 2017
@BridgeAR BridgeAR removed the blocked PRs that are blocked by other issues or PRs. label Sep 15, 2017
@BridgeAR
util: use constructor name
572f1ef 
When reaching the depth limit util.inspect always prints [Array]
or [Object] no matter if it is a subclass or not.
This fixes it by showing the actual constructor name instead.
@BridgeAR BridgeAR force-pushed the add-proper-constructor branch from d76998e to 572f1ef Compare September 15, 2017 01:42
@BridgeAR
Copy link
Member Author

Rebased. PTAL, I would like to land this very soon.

@BridgeAR
Copy link
Member Author

CI https://ci.nodejs.org/job/node-test-pull-request/10144/

@BridgeAR
Copy link
Member Author

Landed in b1c8f15

@BridgeAR BridgeAR closed this Sep 19, 2017
BridgeAR added a commit that referenced this pull request Sep 19, 2017
@BridgeAR
util: use constructor name
b1c8f15 
When reaching the depth limit util.inspect always prints [Array]
or [Object] no matter if it is a subclass or not.
This fixes it by showing the actual constructor name instead.

PR-URL: #14886
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Qard pushed a commit to Qard/ayo that referenced this pull request Sep 21, 2017
@BridgeAR
util: use constructor name
e1e9eb1 
When reaching the depth limit util.inspect always prints [Array]
or [Object] no matter if it is a subclass or not.
This fixes it by showing the actual constructor name instead.

PR-URL: nodejs/node#14886
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Qard pushed a commit to Qard/ayo that referenced this pull request Sep 21, 2017
@BridgeAR
util: use constructor name
f2844dc 
When reaching the depth limit util.inspect always prints [Array]
or [Object] no matter if it is a subclass or not.
This fixes it by showing the actual constructor name instead.

PR-URL: nodejs/node#14886
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
@jasnell jasnell mentioned this pull request Oct 30, 2017
Merged
@BridgeAR BridgeAR deleted the add-proper-constructor branch April 1, 2019 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@cjihrig cjihrig cjihrig approved these changes

+3 more reviewers

@refack refack refack approved these changes

@TimothyGu TimothyGu TimothyGu approved these changes

@hiroppy hiroppy hiroppy approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

semver-major PRs that contain breaking changes and should be released in the next major version. util Issues and PRs related to the built-in util module.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@BridgeAR @TimothyGu @hiroppy @refack @jasnell @cjihrig @nodejs-github-bot