From f0bc97dedf1924137a2cb66716398de82f6e81a5 Mon Sep 17 00:00:00 2001 From: Ryan Graham Date: Sat, 9 Jan 2016 12:54:42 -0800 Subject: [PATCH 1/2] lib: replace util._extend usage with Object.assign If Object.assign was available when util._extend was written, util._extend would never have been written. Additionally, Object.assign provides support for multiple arguments, which is a bit of a gotcha for anyone using util._extend. --- lib/_http_agent.js | 5 ++--- lib/_http_client.js | 2 +- lib/_tls_wrap.js | 6 +++--- lib/child_process.js | 18 +++++++++--------- lib/cluster.js | 14 ++++++-------- lib/console.js | 2 +- lib/domain.js | 2 +- lib/fs.js | 6 +++--- lib/https.js | 4 ++-- lib/tty.js | 2 +- lib/util.js | 2 +- test/parallel/test-crypto.js | 3 +-- test/parallel/test-http-methods.js | 3 +-- test/parallel/test-http-server-stale-close.js | 3 +-- test/parallel/test-readline-keys.js | 3 +-- 15 files changed, 34 insertions(+), 41 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index ddb1c5bfff9b63..2a449e3cd4f729 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -28,7 +28,7 @@ function Agent(options) { self.defaultPort = 80; self.protocol = 'http:'; - self.options = util._extend({}, options); + self.options = Object.assign({}, options); // don't confuse net and make it think that we're connecting to a pipe self.options.path = null; @@ -151,8 +151,7 @@ Agent.prototype.addRequest = function(req, options) { Agent.prototype.createSocket = function(req, options) { var self = this; - options = util._extend({}, options); - options = util._extend(options, self.options); + options = Object.assign({}, options, self.options); if (!options.servername) { options.servername = options.host; diff --git a/lib/_http_client.js b/lib/_http_client.js index 34072f8f996840..9791ff4a120a1e 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -25,7 +25,7 @@ function ClientRequest(options, cb) { throw new Error('Unable to determine the domain name'); } } else { - options = util._extend({}, options); + options = Object.assign({}, options); } var agent = options.agent; diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index d3df48a4c59fc7..4e2d479bebf2f0 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -947,9 +947,9 @@ function normalizeConnectArgs(listArgs) { var cb = args[1]; if (listArgs[1] !== null && typeof listArgs[1] === 'object') { - options = util._extend(options, listArgs[1]); + options = Object.assign(options, listArgs[1]); } else if (listArgs[2] !== null && typeof listArgs[2] === 'object') { - options = util._extend(options, listArgs[2]); + options = Object.assign(options, listArgs[2]); } return (cb) ? [options, cb] : [options]; @@ -967,7 +967,7 @@ exports.connect = function(/* [port, host], options, cb */) { minDHSize: 1024 }; - options = util._extend(defaults, options || {}); + options = Object.assign(defaults, options); if (!options.keepAlive) options.singleUse = true; diff --git a/lib/child_process.js b/lib/child_process.js index ee73562d24e80f..708bd1e3a46268 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -22,12 +22,12 @@ exports.fork = function(modulePath /*, args, options*/) { var options, args, execArgv; if (Array.isArray(arguments[1])) { args = arguments[1]; - options = util._extend({}, arguments[2]); + options = Object.assign({}, arguments[2]); } else if (arguments[1] && typeof arguments[1] !== 'object') { throw new TypeError('Incorrect value of args option'); } else { args = []; - options = util._extend({}, arguments[1]); + options = Object.assign({}, arguments[1]); } // Prepare arguments for fork: @@ -86,7 +86,7 @@ function normalizeExecArgs(command /*, options, callback*/) { args = ['/s', '/c', '"' + command + '"']; // Make a shallow copy before patching so we don't clobber the user's // options object. - options = util._extend({}, options); + options = Object.assign({}, options); options.windowsVerbatimArguments = true; } else { file = '/bin/sh'; @@ -135,7 +135,7 @@ exports.execFile = function(file /*, args, options, callback*/) { } if (pos < arguments.length && typeof arguments[pos] === 'object') { - options = util._extend(options, arguments[pos++]); + options = Object.assign(options, arguments[pos++]); } else if (pos < arguments.length && arguments[pos] == null) { pos++; } @@ -331,7 +331,7 @@ function normalizeSpawnArguments(file /*, args, options*/) { else if (options === null || typeof options !== 'object') throw new TypeError('"options" argument must be an object'); - options = util._extend({}, options); + options = Object.assign({}, options); args.unshift(file); var env = options.env || process.env; @@ -405,7 +405,7 @@ function spawnSync(/*file, args, options*/) { options.stdio = _validateStdio(options.stdio || 'pipe', true).stdio; if (options.input) { - var stdin = options.stdio[0] = util._extend({}, options.stdio[0]); + var stdin = options.stdio[0] = Object.assign({}, options.stdio[0]); stdin.input = options.input; } @@ -413,7 +413,7 @@ function spawnSync(/*file, args, options*/) { for (i = 0; i < options.stdio.length; i++) { var input = options.stdio[i] && options.stdio[i].input; if (input != null) { - var pipe = options.stdio[i] = util._extend({}, options.stdio[i]); + var pipe = options.stdio[i] = Object.assign({}, options.stdio[i]); if (Buffer.isBuffer(input)) pipe.input = input; else if (typeof input === 'string') @@ -445,7 +445,7 @@ function spawnSync(/*file, args, options*/) { result.error.spawnargs = opts.args.slice(1); } - util._extend(result, opts); + Object.assign(result, opts); return result; } @@ -464,7 +464,7 @@ function checkExecSyncError(ret) { err = new Error(msg); } - util._extend(err, ret); + Object.assign(err, ret); return err; } diff --git a/lib/cluster.js b/lib/cluster.js index 8356fad88a60d0..6f148ac5426a85 100644 --- a/lib/cluster.js +++ b/lib/cluster.js @@ -231,8 +231,7 @@ function masterInit() { execArgv: process.execArgv, silent: false }; - settings = util._extend(settings, cluster.settings); - settings = util._extend(settings, options || {}); + settings = Object.assign(settings, cluster.settings, options); // Tell V8 to write profile data for each process to a separate file. // Without --logfile=v8-%p.log, everything ends up in a single, unusable // file. (Unusable because what V8 logs are memory addresses and each @@ -285,10 +284,9 @@ function masterInit() { var debugPortOffset = 1; function createWorkerProcess(id, env) { - var workerEnv = util._extend({}, process.env); + var workerEnv = Object.assign({}, process.env, env); var execArgv = cluster.settings.execArgv.slice(); - workerEnv = util._extend(workerEnv, env); workerEnv.NODE_UNIQUE_ID = '' + id; for (var i = 0; i < execArgv.length; i++) { @@ -478,7 +476,7 @@ function masterInit() { // Set custom server data handle.add(worker, function(errno, reply, handle) { - reply = util._extend({ + reply = Object.assign({ errno: errno, key: key, ack: message.seq, @@ -556,7 +554,7 @@ function workerInit() { else indexes[key]++; - const message = util._extend({ + const message = Object.assign({ act: 'queryServer', index: indexes[key], data: null @@ -623,7 +621,7 @@ function workerInit() { } function getsockname(out) { - if (key) util._extend(out, message.sockname); + if (key) Object.assign(out, message.sockname); return 0; } @@ -702,7 +700,7 @@ var seq = 0; var callbacks = {}; function sendHelper(proc, message, handle, cb) { // Mark message as internal. See INTERNAL_PREFIX in lib/child_process.js - message = util._extend({ cmd: 'NODE_CLUSTER' }, message); + message = Object.assign({ cmd: 'NODE_CLUSTER' }, message); if (cb) callbacks[seq] = cb; message.seq = seq; seq += 1; diff --git a/lib/console.js b/lib/console.js index a0eec4e34878be..c91c770fc6240f 100644 --- a/lib/console.js +++ b/lib/console.js @@ -49,7 +49,7 @@ Console.prototype.error = Console.prototype.warn; Console.prototype.dir = function(object, options) { - this._stdout.write(util.inspect(object, util._extend({ + this._stdout.write(util.inspect(object, Object.assign({ customInspect: false }, options)) + '\n'); }; diff --git a/lib/domain.js b/lib/domain.js index 630d02ec332751..6c250731dcee8a 100644 --- a/lib/domain.js +++ b/lib/domain.js @@ -239,7 +239,7 @@ function intercepted(_this, self, cb, fnargs) { if (fnargs[0] && fnargs[0] instanceof Error) { var er = fnargs[0]; - util._extend(er, { + Object.assign(er, { domainBound: cb, domainThrown: false, domain: self diff --git a/lib/fs.js b/lib/fs.js index 54e6b4ac0d2536..995162e47b1a3e 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1269,7 +1269,7 @@ fs.appendFile = function(path, data, options, callback_) { } if (!options.flag) - options = util._extend({ flag: 'a' }, options); + options = Object.assign({ flag: 'a' }, options); // force append behavior when using a supplied file descriptor if (isFd(path)) @@ -1288,7 +1288,7 @@ fs.appendFileSync = function(path, data, options) { } if (!options.flag) - options = util._extend({ flag: 'a' }, options); + options = Object.assign({ flag: 'a' }, options); // force append behavior when using a supplied file descriptor if (isFd(path)) @@ -1417,7 +1417,7 @@ fs.watchFile = function(filename, options, listener) { }; if (options !== null && typeof options === 'object') { - options = util._extend(defaults, options); + options = Object.assign(defaults, options); } else { listener = options; options = defaults; diff --git a/lib/https.js b/lib/https.js index 7008a79131c663..4c5bdc925cf8de 100644 --- a/lib/https.js +++ b/lib/https.js @@ -71,7 +71,7 @@ function createConnection(port, host, options) { const session = this._getSession(options._agentKey); if (session) { debug('reuse session for %j', options._agentKey); - options = util._extend({ + options = Object.assign({ session: session }, options); } @@ -176,7 +176,7 @@ exports.request = function(options, cb) { throw new Error('Unable to determine the domain name'); } } else { - options = util._extend({}, options); + options = Object.assign({}, options); } options._defaultAgent = globalAgent; return http.request(options, cb); diff --git a/lib/tty.js b/lib/tty.js index 72efcd078b2892..90d13f7bb36f0d 100644 --- a/lib/tty.js +++ b/lib/tty.js @@ -17,7 +17,7 @@ function ReadStream(fd, options) { if (!(this instanceof ReadStream)) return new ReadStream(fd, options); - options = util._extend({ + options = Object.assign({ highWaterMark: 0, readable: true, writable: false, diff --git a/lib/util.js b/lib/util.js index 50cc5bc5b4b25d..089bd856cfd72a 100644 --- a/lib/util.js +++ b/lib/util.js @@ -98,7 +98,7 @@ function inspect(obj, opts) { ctx.showHidden = opts; } else if (opts) { // got an "options" object - exports._extend(ctx, opts); + Object.assign(ctx, opts); } // set default options if (ctx.showHidden === undefined) ctx.showHidden = false; diff --git a/test/parallel/test-crypto.js b/test/parallel/test-crypto.js index df59fd9bded726..725ada4cfad5cd 100644 --- a/test/parallel/test-crypto.js +++ b/test/parallel/test-crypto.js @@ -1,7 +1,6 @@ 'use strict'; var common = require('../common'); var assert = require('assert'); -var util = require('util'); if (!common.hasCrypto) { console.log('1..0 # Skipped: missing crypto'); @@ -56,7 +55,7 @@ assert.throws(function() { function assertSorted(list) { // Array#sort() modifies the list in place so make a copy. - var sorted = util._extend([], list).sort(); + var sorted = Object.assign([], list).sort(); assert.deepEqual(list, sorted); } diff --git a/test/parallel/test-http-methods.js b/test/parallel/test-http-methods.js index 62a4187841bdbc..64d01421c75fdf 100644 --- a/test/parallel/test-http-methods.js +++ b/test/parallel/test-http-methods.js @@ -2,11 +2,10 @@ require('../common'); var assert = require('assert'); var http = require('http'); -var util = require('util'); assert(Array.isArray(http.METHODS)); assert(http.METHODS.length > 0); assert(http.METHODS.indexOf('GET') !== -1); assert(http.METHODS.indexOf('HEAD') !== -1); assert(http.METHODS.indexOf('POST') !== -1); -assert.deepEqual(util._extend([], http.METHODS), http.METHODS.sort()); +assert.deepEqual(Object.assign([], http.METHODS), http.METHODS.sort()); diff --git a/test/parallel/test-http-server-stale-close.js b/test/parallel/test-http-server-stale-close.js index 349e869c1a4fbc..43b196d52796b1 100644 --- a/test/parallel/test-http-server-stale-close.js +++ b/test/parallel/test-http-server-stale-close.js @@ -1,7 +1,6 @@ 'use strict'; var common = require('../common'); var http = require('http'); -var util = require('util'); var fork = require('child_process').fork; if (process.env.NODE_TEST_FORK) { @@ -25,7 +24,7 @@ else { }); server.listen(common.PORT, function() { fork(__filename, { - env: util._extend(process.env, {NODE_TEST_FORK: '1'}) + env: Object.assign(process.env, {NODE_TEST_FORK: '1'}) }); }); } diff --git a/test/parallel/test-readline-keys.js b/test/parallel/test-readline-keys.js index e026c0b583cd9e..6e720e7e833313 100644 --- a/test/parallel/test-readline-keys.js +++ b/test/parallel/test-readline-keys.js @@ -3,7 +3,6 @@ require('../common'); var PassThrough = require('stream').PassThrough; var assert = require('assert'); var inherits = require('util').inherits; -var extend = require('util')._extend; var Interface = require('readline').Interface; @@ -33,7 +32,7 @@ function addTest(sequences, expectedKeys) { } expectedKeys = expectedKeys.map(function(k) { - return k ? extend({ ctrl: false, meta: false, shift: false }, k) : k; + return k ? Object.assign({ ctrl: false, meta: false, shift: false }, k) : k; }); keys = []; From 19e0d9c0b483182a4a035424c071b16a793acee1 Mon Sep 17 00:00:00 2001 From: Ryan Graham Date: Sat, 9 Jan 2016 14:06:57 -0800 Subject: [PATCH 2/2] util: deprecate util._extend Mark util._extend as deprecated with a suggestion to use Object.assign or a 3rd party module. util._extend is a long standing member of the undocumented and unofficial public API. It has never been removed because, quite frankly, it is ridiculously useful and is used extensively throughout node core itself. It has also never been documented because it has always been considered internal or "private", as indicated by the _ prefix. Despite it never being documented or an official part of the API, it is widely used by many modules, so it is being marked as deprecated instead of simply removing it. --- lib/util.js | 18 +++++++++--------- test/parallel/test-util.js | 9 --------- 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/lib/util.js b/lib/util.js index 089bd856cfd72a..d38fdbc593746f 100644 --- a/lib/util.js +++ b/lib/util.js @@ -806,7 +806,14 @@ exports.inherits = function(ctor, superCtor) { Object.setPrototypeOf(ctor.prototype, superCtor.prototype); }; -exports._extend = function(origin, add) { +function hasOwnProperty(obj, prop) { + return Object.prototype.hasOwnProperty.call(obj, prop); +} + + +// Deprecated old stuff. + +exports._extend = internalUtil.deprecate(function(origin, add) { // Don't do anything if add isn't an object if (add === null || typeof add !== 'object') return origin; @@ -816,14 +823,7 @@ exports._extend = function(origin, add) { origin[keys[i]] = add[keys[i]]; } return origin; -}; - -function hasOwnProperty(obj, prop) { - return Object.prototype.hasOwnProperty.call(obj, prop); -} - - -// Deprecated old stuff. +}, 'util._extend is deprecated. Use Object.assign or a module instead.'); exports.print = internalUtil.deprecate(function() { for (var i = 0, len = arguments.length; i < len; ++i) { diff --git a/test/parallel/test-util.js b/test/parallel/test-util.js index 864361d80a6dfa..3f10b934e27485 100644 --- a/test/parallel/test-util.js +++ b/test/parallel/test-util.js @@ -74,12 +74,3 @@ assert.equal(true, util.isPrimitive(Symbol('symbol'))); // isBuffer assert.equal(false, util.isBuffer('foo')); assert.equal(true, util.isBuffer(new Buffer('foo'))); - -// _extend -assert.deepEqual(util._extend({a:1}), {a:1}); -assert.deepEqual(util._extend({a:1}, []), {a:1}); -assert.deepEqual(util._extend({a:1}, null), {a:1}); -assert.deepEqual(util._extend({a:1}, true), {a:1}); -assert.deepEqual(util._extend({a:1}, false), {a:1}); -assert.deepEqual(util._extend({a:1}, {b:2}), {a:1, b:2}); -assert.deepEqual(util._extend({a:1, b:2}, {b:3}), {a:1, b:3});