Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

core: add `NativeModule.prototype.deprecate`

Formalize and cleanup handling of deprecated core methods.
  • Loading branch information...
commit 5403a8ce4cce83a13569b09ee5b2ba3d1ad8c176 1 parent e3c0c86
@Benvie Benvie authored bnoordhuis committed
View
9 lib/http.js
@@ -1606,9 +1606,7 @@ exports._connectionListener = connectionListener;
// Legacy Interface
function Client(port, host) {
- // TODO http.Client can be removed in v0.9. Until then leave this message.
- util._deprecationWarning('http', 'http.Client is a legacy interface' +
- ' and will be removed in the near future. Do not use it.');
+ if (!(this instanceof Client)) return new Client(port, host);
host = host || 'localhost';
port = port || 80;
this.host = host;
@@ -1646,6 +1644,11 @@ Client.prototype.request = function(method, path, headers) {
};
exports.Client = Client;
+
+// TODO http.Client can be removed in v0.9. Until then leave this message.
+module.deprecate('Client', 'It will be removed in the near future. Do not use it.');
+
exports.createClient = function(port, host) {
return new Client(port, host);
};
+module.deprecate('createClient', 'Use `http.request` instead.');
View
3  lib/os.js
@@ -38,7 +38,6 @@ exports.platform = function() {
};
exports.getNetworkInterfaces = function() {
- require('util')._deprecationWarning('os',
- 'os.getNetworkInterfaces() is deprecated - use os.networkInterfaces()');
return exports.networkInterfaces();
};
+module.deprecate('getNetworkInterfaces', 'It is now called `os.networkInterfaces`.');
View
6 lib/path.js
@@ -403,18 +403,18 @@ exports.extname = function(path) {
exports.exists = function(path, callback) {
- _deprecationWarning('path', '`path.exists` is now called `fs.exists`');
require('fs').exists(path, callback);
};
+module.deprecate('exists', 'It is now called `fs.exists`.');
exports.existsSync = function(path) {
- _deprecationWarning('path', '`path.exists` is now called `fs.exists`');
return require('fs').existsSync(path);
};
+module.deprecate('existsSync', 'It is now called `fs.existsSync`.');
-exports._makeLong = isWindows ?
+exports._makeLong = isWindows ?
function(path) {
var resolvedPath = exports.resolve(path);
View
31 lib/util.js
@@ -407,18 +407,12 @@ function objectToString(o) {
}
-var pWarning;
-
exports.p = function() {
- if (!pWarning) {
- pWarning = 'util.p will be removed in future versions of Node. ' +
- 'Use util.puts(util.inspect()) instead.\n';
- exports.error(pWarning);
- }
for (var i = 0, len = arguments.length; i < len; ++i) {
error(exports.inspect(arguments[i]));
}
};
+module.deprecate('p', 'Use `util.puts(util.inspect())` instead.');
function pad(n) {
@@ -444,15 +438,10 @@ exports.log = function(msg) {
};
-var execWarning;
exports.exec = function() {
- if (!execWarning) {
- execWarning = 'util.exec has moved to the "child_process" module.' +
- ' Please update your source code.';
- error(execWarning);
- }
return require('child_process').exec.apply(this, arguments);
};
+module.deprecate('exec', 'It is now called `child_process.exec`.');
exports.pump = function(readStream, writeStream, callback) {
@@ -517,19 +506,3 @@ exports.inherits = function(ctor, superCtor) {
}
});
};
-
-var deprecationWarnings;
-
-exports._deprecationWarning = function(moduleId, message) {
- if (!deprecationWarnings)
- deprecationWarnings = {};
- else if (message in deprecationWarnings)
- return;
-
- deprecationWarnings[message] = true;
-
- if ((new RegExp('\\b' + moduleId + '\\b')).test(process.env.NODE_DEBUG))
- console.trace(message);
- else
- console.error(message);
-};
View
20 src/node.js
@@ -569,5 +569,25 @@
NativeModule._cache[this.id] = this;
};
+ NativeModule.prototype.deprecate = function(method, message) {
+ var original = this.exports[method];
+ var self = this;
+
+ Object.defineProperty(this.exports, method, {

Just a concern after taking a glance of this. If you did:

var method = require('module').deprecatedMethod
method()
method()

Wouldn't you get the warning twice?

@Benvie
Benvie added a note

Yes; a simple boolean flag will resolve the issue just fine. Considering this was previous behavior, taking the route of less surprise is probably a good idea.

@Benvie
Benvie added a note

#2641

It's actually worse now that I'm looking at it. Every time it's called it'll prepend more text to the message so it'll just get longer and longer. Regardless, a simple fix and the pull request is in.

Unfortunately that patch breaks a community style guideline regarding the 80 column limit. Quick patch: e4af3e9

(I will post this on the pull request as well)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ enumerable: false,
+ value: function() {
+ message = self.id + '.' + method + ' is deprecated. ' + (message || '');
+
+ if ((new RegExp('\\b' + self.id + '\\b')).test(process.env.NODE_DEBUG))
+ console.trace(message);
+ else
+ console.error(message);
+
+ self.exports[method] = original;
+ return original.apply(this, arguments);
+ }
+ });
+ };
+
startup();
});
@tim-smart

Just a concern after taking a glance of this. If you did:

var method = require('module').deprecatedMethod
method()
method()

Wouldn't you get the warning twice?

@Benvie
@tim-smart

Yes; a simple boolean flag will resolve the issue just fine. Considering this was previous behavior, taking the route of less surprise is probably a good idea.

@Benvie

#2641

It's actually worse now that I'm looking at it. Every time it's called it'll prepend more text to the message so it'll just get longer and longer. Regardless, a simple fix and the pull request is in.

@tim-smart

Unfortunately that patch breaks a community style guideline regarding the 80 column limit. Quick patch: e4af3e9

(I will post this on the pull request as well)

Please sign in to comment.
Something went wrong with that request. Please try again.