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.

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

util: set super_ property to non-enumerable #23107

Closed
wants to merge 1 commit into from
Closed

util: set super_ property to non-enumerable #23107

wants to merge 1 commit into from

Conversation

BridgeAR
Copy link
Member

Using util.inherits() adds a super_ property to the constructor
and inspecting that constructor is going to show that property even
though it's not useful for the user.

Therefore the property is now set as non-enumerable and such entries
are not visible by default when using console.log() or
util.inspect().

This significantly reduces the amount of data shown when inspecting streams.

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

@BridgeAR
util: set super_ property to non-enumerable
b39109b 
Using `util.inherits()` adds a `super_` property to the constructor
and inspecting that constructor is going to show that property even
though it's not useful for the user.

Therefore the property is now set as non-enumerable and such entries
are not visible by default when using `console.log()` or
`util.inspect()`.
@nodejs-github-bot
Copy link
Collaborator

@BridgeAR build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1030/pipeline

@nodejs-github-bot nodejs-github-bot added the util Issues and PRs related to the built-in util module. label Sep 26, 2018
@BridgeAR BridgeAR added the semver-major PRs that contain breaking changes and should be released in the next major version. label Sep 26, 2018
@BridgeAR
Copy link
Member Author

@nodejs/tsc PTAL

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 26, 2018
@BridgeAR
Copy link
Member Author

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

@BridgeAR
Copy link
Member Author

Rebuild arm: https://ci.nodejs.org/job/node-test-commit-arm-fanned/3808/
Rebuild commit-linux https://ci.nodejs.org/job/node-test-commit-linux/21890/

mcollina
mcollina reviewed Sep 27, 2018
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

Can you please show the output before/after this PR?

@BridgeAR
Copy link
Member Author

Before:

> net
{ _createServerHandle: [Function: createServerHandle],
  _normalizeArgs: [Function: normalizeArgs],
  _setSimultaneousAccepts: [Function: _setSimultaneousAccepts],
  connect: [Function: connect],
  createConnection: [Function: connect],
  createServer: [Function: createServer],
  isIP: [Function: isIP],
  isIPv4: [Function: isIPv4],
  isIPv6: [Function: isIPv6],
  Server:
   { [Function: Server]
     super_:
      { [Function: EventEmitter]
        EventEmitter: [Circular],
        usingDomains: true,
        defaultMaxListeners: [Getter/Setter],
        init: [Function],
        listenerCount: [Function] } },
  Socket:
   { [Function: Socket] super_: { [Function: Duplex] super_: [Function] } },
  Stream:
   { [Function: Socket] super_: { [Function: Duplex] super_: [Function] } } }

After:

> net
{ _createServerHandle: [Function: createServerHandle],
  _normalizeArgs: [Function: normalizeArgs],
  _setSimultaneousAccepts: [Function: _setSimultaneousAccepts],
  connect: [Function: connect],
  createConnection: [Function: connect],
  createServer: [Function: createServer],
  isIP: [Function: isIP],
  isIPv4: [Function: isIPv4],
  isIPv6: [Function: isIPv6],
  Server: [Function: Server],
  Socket: [Function: Socket],
  Stream: [Function: Socket] }

mcollina
mcollina approved these changes Sep 27, 2018
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM.

@BridgeAR
Copy link
Member Author

BridgeAR commented Sep 27, 2018 •
edited
Loading

Rebuild arm again https://ci.nodejs.org/job/node-test-commit-arm-fanned/3811/ 🚫
Rebuild commit-linux again https://ci.nodejs.org/job/node-test-commit-linux/21893/ ✔️

@BridgeAR
Copy link
Member Author

BridgeAR commented Sep 28, 2018 •
edited
Loading

I can not start arm-fanned again, so here's a complete rebuild:

CI https://ci.nodejs.org/job/node-test-pull-request/17493/ ✔️

@BridgeAR
Copy link
Member Author

BridgeAR commented Oct 1, 2018

Landed in 5e6940d.

@BridgeAR BridgeAR closed this Oct 1, 2018
BridgeAR added a commit to BridgeAR/node that referenced this pull request Oct 1, 2018
@BridgeAR
util: set super_ property to non-enumerable
5e6940d 
Using `util.inherits()` adds a `super_` property to the constructor
and inspecting that constructor is going to show that property even
though it's not useful for the user.

Therefore the property is now set as non-enumerable and such entries
are not visible by default when using `console.log()` or
`util.inspect()`.

PR-URL: nodejs#23107
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeAR BridgeAR deleted the hide-super branch January 20, 2020 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcollina mcollina mcollina approved these changes

@jasnell jasnell jasnell approved these changes

@addaleax addaleax addaleax approved these changes

@Fishrock123 Fishrock123 Fishrock123 approved these changes

@targos targos targos approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. 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 @nodejs-github-bot @mcollina @jasnell @addaleax @Fishrock123 @targos