lib: reduce internal usage of public require('util') #26546

joyeecheung · 2019-03-09T15:26:40Z

A few patterns (modulo destructuring):

Replace require('util').inherits() usages under lib: Remove util.inherits usage internally? #24395

// Change this
util.inherits(A, B);

// To this
Object.setPrototypeOf(A.prototype, B.prototype);
Object.setPrototypeOf(A, B);

Replace require('util').inspect with require('internal/util/inspect').inspect
Replace require('util').format with require('internal/util/inspect').format
Replace require('util').debuglog with require('internal/util/debuglog').debuglog
Replace require('util').types with require('internal/util/types')
Replace require('util').deprecate with require('internal/util').deprecate
Replace require('util').promisify with require('internal/util').promisify

List of JS files that can be refactored:

Subsystem	Files	PR
assert	lib/assert.js#L32, lib/internal/assert/assertion_error.js#L3	#26762, #26750
bootstrap	lib/internal/bootstrap/node.js#L192	#26547
child_process	lib/child_process.js#L24, lib/internal/child_process.js#L21	#26769, #26773
console	lib/internal/console/constructor.js#L18	#26777
dgram	lib/dgram.js#L48	#26770
domain	~~lib/domain.js#L29~~
encoding	lib/internal/encoding.js#L330	#26779
errors	lib/internal/error-serdes.js#L83, lib/internal/errors.js#L210	#26782, #26781
fs	lib/internal/fs/utils.js#L15	#26783
http	lib/_http_agent.js#L25, lib/_http_common.js#L39, lib/_http_outgoing.js#L26, lib/_http_server.js#L24	#26548
http2	lib/internal/http2/core.js#L21	#26789
https	lib/https.js#L28	#26772
inspector	~~lib/inspector.js#L19~~
lib	lib/internal/policy/manifest.js#L7	#26833
module	lib/internal/modules/cjs/loader.js#L26, lib/internal/modules/esm/create_dynamic_module.js#L4, lib/internal/modules/esm/loader.js#L21, lib/internal/modules/esm/module_map.js#L7, lib/internal/modules/esm/translators.js#L22	#26802, #26803, #26804, #26805, #26806
net	lib/net.js#L26	#26807
process	lib/internal/process/per_thread.js#L18	#26817
readline	lib/readline.js#L35	#26818
repl	lib/internal/repl/history.js#L7, lib/repl.js#L55	#26819 #26820
stream	lib/internal/js_stream_socket.js#L4, lib/internal/streams/buffer_list.js#L4, lib/internal/streams/lazy_transform.js#L7, lib/stream.js#L46	#26698
timers	lib/internal/timers.js#L19, lib/timers.js#L43
tls	lib/_tls_wrap.js#L30	#26747
trace_events	lib/trace_events.js#L22	#26822
tty	lib/tty.js#L24	#26797
url	lib/internal/url.js#L3	#26808
vm	lib/internal/vm/source_text_module.js#L3, lib/vm.js#L35	#26716
worker	lib/internal/main/worker_thread.js#L46, lib/internal/worker/io.js#L17	#26814, #26810
zlib	lib/zlib.js#L38	#26716

The text was updated successfully, but these errors were encountered:

micheleriva · 2019-03-09T17:40:16Z

I can work on it 🎉

joyeecheung · 2019-03-10T02:21:09Z

@micheleriva That's great! If possible please try to refactor each file in single commit and submit them as separate PRs, otherwise the PR is bound to require manual backport.

Remove public require('util') and replace util.isFunction with internal/util/isFunction Fixes: nodejs#26546

SimonSchick · 2019-03-10T12:09:52Z

@joyeecheung Just curious, in many cases util.inherits could be replaced by class ... extends ... instead, is there a reason setPrototypeOf should be used instead?

micheleriva · 2019-03-10T13:21:09Z

@joyeecheung ok no prob! So I'll close my PR #26552 in flavour of single PRs.

joyeecheung · 2019-03-10T16:33:47Z

@SimonSchick For public ES5-style classes we have to support instantiation without new so we cannot refactor them to ES6 classes.

SimonSchick · 2019-03-10T16:41:39Z

I was talking about those that do not support instantiation without new.

Might also be worth mentioning that we dealt with the same issue in sequelize using Proxys https://github.com/sequelize/sequelize/blob/master/lib/utils/classToInvokable.js#L9
I'm not sure what the exact performance penalties are tho.

PR-URL: nodejs#26548 Refs: nodejs#26546 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

PR-URL: #26547 Refs: #26546 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>

dnishar1 · 2019-03-12T15:19:03Z

I can work on it!

joyeecheung · 2019-03-13T16:41:02Z

I was talking about those that do not support instantiation without new.

@SimonSchick I guess if you are sure those do not support instantiation without new, those could be refactored if the tests are happy (probably also needs a CITGM run anyway because undoing util.inherits also removes the hacky super_ property on the classes)

PR-URL: #26547 Refs: #26546 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: nodejs#26548 Refs: nodejs#26546 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

PR-URL: #26547 Refs: #26546 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>

gengjiawen · 2019-03-15T12:21:09Z

@joyeecheung Maybe make this task a table like this jestjs/jest#7807 (comment)

Refs: nodejs#26546

Remove the usage of public require('util'), as described in issue: nodejs#26546

Use `require('internal/util/inspect').inspect` and `require('internal/util/debuglog').debuglog` instead of `require('util').debuglog` and `require('util').inspect`. PR-URL: #26807 Refs: #26546 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

Use `require('internal/util/inspect').format` instead of `require('util').format`. PR-URL: #26817 Refs: #26546 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

Replace `require('util').inspect` and `require('util').format` with `require('util/internal/inspect').inspect` and `require('util/internal/inspect').format` in `lib/internal/errors.js`. PR-URL: #26782 Refs: #26546 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>

Remove internal usage of `require('util').inspect`. PR-URL: #26781 Refs: #26546 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

PR-URL: #26583 Refs: #26546 Reviewed-By: Anna Henningsen <anna@addaleax.net>

Refs: #26546 PR-URL: #26583 Reviewed-By: Anna Henningsen <anna@addaleax.net>

This patch: - Moves the timer callback initialization into bootstrap/node.js, documents when they will be called, and make the dependency on process._tickCallback explicit. - Moves the initialization of tick callbacks and timer callbacks to the end of the bootstrap to make sure the operations done before those initializations are synchronous. - Moves more internals into internal/timers.js from timers.js. PR-URL: #26583 Refs: #26546 Reviewed-By: Anna Henningsen <anna@addaleax.net>

Backport-PR-URL: #26650 PR-URL: #26548 Refs: #26546 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

PR-URL: #26810 Refs: #26546 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: #26814 Refs: #26546 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Masashi Hirano <shisama07@gmail.com> Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>

Use `require('internal/util/inspect').inspect` and `require('internal/util/debuglog').debuglog` instead of `require('util').inspect` and `require('util').debuglog`. Refs: #26546 PR-URL: #26820 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Masashi Hirano <shisama07@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>

Use `require('internal/util/inspect').inspect`, `require('internal/util/debuglog').debuglog`, `require('internal/util').deprecate` and `Object.setPrototypeOf` instead of `require('util')`. Fix test in `test/parallel/test-net-access-byteswritten.js` to do not check the `super_` property that was set when using `require('util').inherits`. Refs: #26546 Refs: #26896 PR-URL: #26920 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Michaël Zasso <targos@protonmail.com>

Backport-PR-URL: #26650 PR-URL: #26548 Refs: #26546 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

PR-URL: #26810 Refs: #26546 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: #26814 Refs: #26546 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Masashi Hirano <shisama07@gmail.com> Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>

Use `require('internal/util/inspect').inspect` and `require('internal/util/debuglog').debuglog` instead of `require('util').inspect` and `require('util').debuglog`. Refs: #26546 PR-URL: #26820 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Masashi Hirano <shisama07@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>

Use `require('internal/util/inspect').inspect`, `require('internal/util/debuglog').debuglog`, `require('internal/util').deprecate` and `Object.setPrototypeOf` instead of `require('util')`. Fix test in `test/parallel/test-net-access-byteswritten.js` to do not check the `super_` property that was set when using `require('util').inherits`. Refs: #26546 Refs: #26896 PR-URL: #26920 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Michaël Zasso <targos@protonmail.com>

Use `require('internal/util/debuglog').debuglog` instead of `require('util').debuglog` in `lib/internal/modules/esm/create_dynamic_module.js`. Refs: nodejs#26546

Use `require('internal/util/debuglog').debuglog` instead of `require('util').debuglog` in `lib/internal/modules/esm/create_dynamic_module.js`. PR-URL: nodejs#26803 Refs: nodejs#26546 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>

Use `require('internal/util/inspect').format` instead of `require('util').format`. Refs: #26546 PR-URL: #26822 Reviewed-By: Masashi Hirano <shisama07@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>

PR-URL: #27285 Refs: #26546 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>

Use `require('internal/util/inspect').format` instead of `require('util').format`. Refs: #26546 PR-URL: #26822 Reviewed-By: Masashi Hirano <shisama07@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>

karansapolia · 2019-06-03T19:58:58Z

Was looking for issues to work on and noticed that required changes have already been made and merged for timers in lib/internal/timers.js and lib/timers.js. Corresponding PRs can be updated in the original post.

lesagi · 2019-09-28T15:17:02Z

Any reason that this is still open?
Seems like everything was refactored? (given what @karansapolia said).

BridgeAR · 2019-10-03T00:53:51Z

I can only find a single require('util') in lib at the moment that could be replaced. I'll open a PR for that and close this issue as resolved.

joyeecheung added help wanted Issues that need assistance from volunteers or PRs that need help to proceed. util Issues and PRs related to the built-in util module. good first issue Issues that are suitable for first-time contributors. labels Mar 9, 2019

joyeecheung mentioned this issue Mar 9, 2019

lib: import TextEncoder and TextDecoder from internal/encoding #26547

Closed

2 tasks

ZYSzys mentioned this issue Mar 9, 2019

http: reduce usage of public util #26548

Closed

2 tasks

anantsinha added a commit to anantsinha/node that referenced this issue Mar 10, 2019

lib: remove require('util') from inspector.js

9ad84b0

Remove public require('util') and replace util.isFunction with internal/util/isFunction Fixes: nodejs#26546

This was referenced Mar 10, 2019

Fixes: #26546 #26552

Closed

http: use internal/debuglog directly in lib/_http_agent.js #26567

Closed

joyeecheung mentioned this issue Mar 11, 2019

timers: refactor timer callback initialization #26583

Closed

2 tasks

cjihrig mentioned this issue Mar 12, 2019

Remove util.inherits usage internally? #24395

Closed

refack mentioned this issue Mar 14, 2019

[v11.x backport] http: reduce usage of public util #26650

Closed

2 tasks

BeniCheni mentioned this issue Mar 16, 2019

stream: reduce internal usage of public require of util #26698

Closed

2 tasks

joyeecheung added a commit to joyeecheung/node that referenced this issue Mar 18, 2019

timers: reduce usage of public util

4ac4f32

Refs: nodejs#26546

dnlup added a commit to dnlup/node that referenced this issue Mar 18, 2019

_tls_wrap: remove usage of public require('util')

956cbcd

Remove the usage of public require('util'), as described in issue: nodejs#26546

targos pushed a commit that referenced this issue Mar 27, 2019

timers: refactor to use module.exports

e6367c2

PR-URL: #26583 Refs: #26546 Reviewed-By: Anna Henningsen <anna@addaleax.net>

targos pushed a commit that referenced this issue Mar 27, 2019

timers: reduce usage of public util

ebb0c2a

Refs: #26546 PR-URL: #26583 Reviewed-By: Anna Henningsen <anna@addaleax.net>

dnlup added a commit to dnlup/node that referenced this issue Apr 10, 2019

module: remove usage of require('util')

07117c3

Use `require('internal/util/debuglog').debuglog` instead of `require('util').debuglog` in `lib/internal/modules/esm/create_dynamic_module.js`. Refs: nodejs#26546

dozingpip mentioned this issue Apr 17, 2019

lib: remove require('util') from lib/vm/source_text_module.js #27285

Closed

2 tasks

ZYSzys pushed a commit that referenced this issue Apr 21, 2019

vm: remove require('util') from lib/vm/source_text_module.js

e9fb92d

PR-URL: #27285 Refs: #26546 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>

trivikr added the Hacktoberfest label Oct 1, 2019

BridgeAR closed this as completed Oct 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib: reduce internal usage of public require('util') #26546

lib: reduce internal usage of public require('util') #26546

joyeecheung commented Mar 9, 2019 •

edited by ZYSzys

Loading

micheleriva commented Mar 9, 2019

joyeecheung commented Mar 10, 2019

SimonSchick commented Mar 10, 2019

micheleriva commented Mar 10, 2019

joyeecheung commented Mar 10, 2019 •

edited

Loading

SimonSchick commented Mar 10, 2019

dnishar1 commented Mar 12, 2019

joyeecheung commented Mar 13, 2019 •

edited

Loading

gengjiawen commented Mar 15, 2019

karansapolia commented Jun 3, 2019

lesagi commented Sep 28, 2019

BridgeAR commented Oct 3, 2019

lib: reduce internal usage of public require('util') #26546

lib: reduce internal usage of public require('util') #26546

Comments

joyeecheung commented Mar 9, 2019 • edited by ZYSzys Loading

micheleriva commented Mar 9, 2019

joyeecheung commented Mar 10, 2019

SimonSchick commented Mar 10, 2019

micheleriva commented Mar 10, 2019

joyeecheung commented Mar 10, 2019 • edited Loading

SimonSchick commented Mar 10, 2019

dnishar1 commented Mar 12, 2019

joyeecheung commented Mar 13, 2019 • edited Loading

gengjiawen commented Mar 15, 2019

karansapolia commented Jun 3, 2019

lesagi commented Sep 28, 2019

BridgeAR commented Oct 3, 2019

joyeecheung commented Mar 9, 2019 •

edited by ZYSzys

Loading

joyeecheung commented Mar 10, 2019 •

edited

Loading

joyeecheung commented Mar 13, 2019 •

edited

Loading