Skip to content

Commit

Permalink
util: emit deprecation code only once
Browse files Browse the repository at this point in the history
If another function has already emitted the deprecation warning with the
same code as the warning that is about to be emitted, do not emit the
warning.

This is a breaking change. Previously, different functions could emit
the same deprecation warning multiple times. This was a known bug rather
than a feature, but this change is being treated as a breaking change
out of caution. Identical deprecation warnings should not be emitted.

PR-URL: #16393
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
Trott committed Nov 17, 2017
1 parent ccc87eb commit 07d39a2
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 9 deletions.
29 changes: 21 additions & 8 deletions doc/api/util.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,9 @@ environment variable. For example: `NODE_DEBUG=fs,net,tls`.
## util.deprecate(fn, msg[, code])
<!-- YAML
added: v0.8.0
changes:
- version: REPLACEME
description: Deprecation warnings are only emitted once for each code.
-->

* `fn` {Function} The function that is being deprecated.
Expand All @@ -122,21 +125,31 @@ added: v0.8.0
The `util.deprecate()` method wraps `fn` (which may be a function or class) in
such a way that it is marked as deprecated.

<!-- eslint-disable prefer-rest-params -->
```js
const util = require('util');

exports.puts = util.deprecate(function() {
for (let i = 0, len = arguments.length; i < len; ++i) {
process.stdout.write(arguments[i] + '\n');
}
}, 'util.puts: Use console.log instead');
exports.obsoleteFunction = util.deprecate(function() {
// Do something here.
}, 'obsoleteFunction() is deprecated. Use newShinyFunction() instead.');
```

When called, `util.deprecate()` will return a function that will emit a
`DeprecationWarning` using the `process.on('warning')` event. The warning will
be emitted and printed to `stderr` exactly once, the first time it is called.
After the warning is emitted, the wrapped function is called.
be emitted and printed to `stderr` the first time the returned function is
called. After the warning is emitted, the wrapped function is called without
emitting a warning.

If the same optional `code` is supplied in multiple calls to `util.deprecate()`,
the warning will be emitted only once for that `code`.

```js
const util = require('util');

const fn1 = util.deprecate(someFunction, someMessage, 'DEP0001');
const fn2 = util.deprecate(someOtherFunction, someOtherMessage, 'DEP0001');
fn1(); // emits a deprecation warning with code DEP0001
fn2(); // does not emit a deprecation warning because it has the same code
```

If either the `--no-deprecation` or `--no-warnings` command line flags are
used, or if the `process.noDeprecation` property is set to `true` *prior* to
Expand Down
9 changes: 8 additions & 1 deletion lib/internal/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ function objectToString(o) {
return Object.prototype.toString.call(o);
}

// Keep a list of deprecation codes that have been warned on so we only warn on
// each one once.
const codesWarned = {};

// Mark that a method should not be used.
// Returns a modified function which warns once by default.
// If --no-deprecation is set, then it is a no-op.
Expand All @@ -46,7 +50,10 @@ function deprecate(fn, msg, code) {
if (!warned) {
warned = true;
if (code !== undefined) {
process.emitWarning(msg, 'DeprecationWarning', code, deprecated);
if (!codesWarned[code]) {
process.emitWarning(msg, 'DeprecationWarning', code, deprecated);
codesWarned[code] = true;
}
} else {
process.emitWarning(msg, 'DeprecationWarning', deprecated);
}
Expand Down
57 changes: 57 additions & 0 deletions test/parallel/test-util-deprecate.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
'use strict';

require('../common');

// Tests basic functionality of util.deprecate().

const assert = require('assert');
const util = require('util');

const expectedWarnings = new Map();

// Emits deprecation only once if same function is called.
{
const msg = 'fhqwhgads';
const fn = util.deprecate(() => {}, msg);
expectedWarnings.set(msg, { code: undefined, count: 1 });
fn();
fn();
}

// Emits deprecation twice for different functions.
{
const msg = 'sterrance';
const fn1 = util.deprecate(() => {}, msg);
const fn2 = util.deprecate(() => {}, msg);
expectedWarnings.set(msg, { code: undefined, count: 2 });
fn1();
fn2();
}

// Emits deprecation only once if optional code is the same, even for different
// functions.
{
const msg = 'cannonmouth';
const code = 'deprecatesque';
const fn1 = util.deprecate(() => {}, msg, code);
const fn2 = util.deprecate(() => {}, msg, code);
expectedWarnings.set(msg, { code, count: 1 });
fn1();
fn2();
fn1();
fn2();
}

process.on('warning', (warning) => {
assert.strictEqual(warning.name, 'DeprecationWarning');
assert.ok(expectedWarnings.has(warning.message));
const expected = expectedWarnings.get(warning.message);
assert.strictEqual(warning.code, expected.code);
expected.count = expected.count - 1;
if (expected.count === 0)
expectedWarnings.delete(warning.message);
});

process.on('exit', () => {
assert.deepStrictEqual(expectedWarnings, new Map());
});

0 comments on commit 07d39a2

Please sign in to comment.