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.

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

lib,fs: use process.emitWarning instead of internal print dep warning #8166

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/_linklist.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

const msg = require('internal/util').printDeprecationMessage;

module.exports = require('internal/linkedlist');
msg('_linklist module is deprecated. Please use a userland alternative.');
process.emitWarning(
'_linklist module is deprecated. Please use a userland alternative.',
'DeprecationWarning');
24 changes: 15 additions & 9 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ const isWindows = process.platform === 'win32';

const DEBUG = process.env.NODE_DEBUG && /fs/.test(process.env.NODE_DEBUG);
const errnoException = util._errnoException;
const printDeprecation = require('internal/util').printDeprecationMessage;

Copy link
Member

@ChALkeR ChALkeR Aug 30, 2016

Choose a reason for hiding this comment

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

Could you leave require('internal/util'); here so this won't become suddenly re-evaluatable in v7? Or we might be forced to do another round of deprecation…

That line could be removed when something else will land that requires internal, hopefully in time for v7.

Upd: not needed anymore, as #6749 landed.

Copy link
Member Author

Choose a reason for hiding this comment

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

There are several other open PRs that move bits to internal/fs.js that would cover this requirement.

Copy link
Member

Choose a reason for hiding this comment

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

Only if those land before this one =).

Copy link
Member Author

Choose a reason for hiding this comment

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

@ChALkeR ... #6749 landed just now, which adds a ref to internal/fs

Copy link
Member

Choose a reason for hiding this comment

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

Yay!
That discards my previous comments in this subthread =).

function throwOptionsError(options) {
throw new TypeError('Expected options to be either an object or a string, ' +
Expand Down Expand Up @@ -613,10 +612,14 @@ var readWarned = false;
fs.read = function(fd, buffer, offset, length, position, callback) {
if (!(buffer instanceof Buffer)) {
// legacy string interface (fd, length, position, encoding, callback)
readWarned = printDeprecation('fs.read\'s legacy String interface ' +
'is deprecated. Use the Buffer API as ' +
'mentioned in the documentation instead.',
readWarned);
if (!readWarned) {
readWarned = true;
process.emitWarning(
'fs.read\'s legacy String interface is deprecated. Use the Buffer ' +
'API as mentioned in the documentation instead.',
'DeprecationWarning');
}

const cb = arguments[4];
const encoding = arguments[3];

Expand Down Expand Up @@ -673,10 +676,13 @@ fs.readSync = function(fd, buffer, offset, length, position) {

if (!(buffer instanceof Buffer)) {
// legacy string interface (fd, length, position, encoding, callback)
readSyncWarned = printDeprecation('fs.readSync\'s legacy String interface' +
'is deprecated. Use the Buffer API as ' +
'mentioned in the documentation instead.',
readSyncWarned);
if (!readSyncWarned) {
readSyncWarned = true;
process.emitWarning(
'fs.readSync\'s legacy String interface is deprecated. Use the ' +
'Buffer API as mentioned in the documentation instead.',
'DeprecationWarning');
}
legacy = true;
encoding = arguments[3];

Expand Down
20 changes: 10 additions & 10 deletions lib/internal/process/warning.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
'use strict';

const traceWarnings = process.traceProcessWarnings;
const noDeprecation = process.noDeprecation;
const traceDeprecation = process.traceDeprecation;
const throwDeprecation = process.throwDeprecation;
const prefix = `(${process.release.name}:${process.pid}) `;

exports.setup = setupProcessWarnings;
Expand All @@ -13,8 +9,9 @@ function setupProcessWarnings() {
process.on('warning', (warning) => {
if (!(warning instanceof Error)) return;
const isDeprecation = warning.name === 'DeprecationWarning';
if (isDeprecation && noDeprecation) return;
const trace = traceWarnings || (isDeprecation && traceDeprecation);
if (isDeprecation && process.noDeprecation) return;
const trace = process.traceProcessWarnings ||
(isDeprecation && process.traceDeprecation);
if (trace && warning.stack) {
console.error(`${prefix}${warning.stack}`);
} else {
Expand All @@ -41,9 +38,12 @@ function setupProcessWarnings() {
if (!(warning instanceof Error)) {
throw new TypeError('\'warning\' must be an Error object or string.');
}
if (throwDeprecation && warning.name === 'DeprecationWarning')
throw warning;
else
process.nextTick(() => process.emit('warning', warning));
if (warning.name === 'DeprecationWarning') {
if (process.noDeprecation)
return;
if (process.throwDeprecation)
throw warning;
}
process.nextTick(() => process.emit('warning', warning));
};
}
15 changes: 4 additions & 11 deletions lib/internal/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,6 @@ exports.deprecate = function(fn, msg) {
return exports._deprecate(fn, msg);
};

// All the internal deprecations have to use this function only, as this will
// prepend the prefix to the actual message.
exports.printDeprecationMessage = function(msg, warned, ctor) {
if (warned || process.noDeprecation)
return true;
process.emitWarning(msg, 'DeprecationWarning',
ctor || exports.printDeprecationMessage);
return true;
};

exports.error = function(msg) {
const fmt = `${prefix}${msg}`;
if (arguments.length > 1) {
Expand Down Expand Up @@ -63,7 +53,10 @@ exports._deprecate = function(fn, msg) {

var warned = false;
function deprecated() {
warned = exports.printDeprecationMessage(msg, warned, deprecated);
if (!warned) {
warned = true;
process.emitWarning(msg, 'DeprecationWarning', deprecated);
}
if (new.target) {
return Reflect.construct(fn, arguments, new.target);
}
Expand Down
12 changes: 8 additions & 4 deletions lib/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,14 @@ Module._findPath = function(request, paths, isMain) {
if (filename) {
// Warn once if '.' resolved outside the module dir
if (request === '.' && i > 0) {
warned = internalUtil.printDeprecationMessage(
'warning: require(\'.\') resolved outside the package ' +
'directory. This functionality is deprecated and will be removed ' +
'soon.', warned);
if (!warned) {
warned = true;
process.emitWarning(
'warning: require(\'.\') resolved outside the package ' +
'directory. This functionality is deprecated and will be removed ' +
'soon.',
'DeprecationWarning');
}
}

Module._pathCache[cacheKey] = filename;
Expand Down
5 changes: 2 additions & 3 deletions lib/sys.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
'use strict';

const util = require('internal/util');

// the sys module was renamed to 'util'.
// this shim remains to keep old programs working.
// sys is deprecated and shouldn't be used

module.exports = require('util');
util.printDeprecationMessage('sys is deprecated. Use util instead.');
process.emitWarning('sys is deprecated. Use util instead.',
'DeprecationWarning');
7 changes: 3 additions & 4 deletions test/parallel/test-process-no-deprecation.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
// Flags: --expose_internals --no-warnings
// Flags: --no-warnings

// The --no-warnings flag only supresses writing the warning to stderr, not the
// emission of the corresponding event. This test file can be run without it.
Expand All @@ -15,8 +15,7 @@ function listener() {

process.addListener('warning', listener);

const internalUtil = require('internal/util');
internalUtil.printDeprecationMessage('Something is deprecated.');
process.emitWarning('Something is deprecated.', 'DeprecationWarning');

// The warning would be emitted in the next tick, so continue after that.
process.nextTick(common.mustCall(() => {
Expand All @@ -29,5 +28,5 @@ process.nextTick(common.mustCall(() => {
assert.strictEqual(warning.message, 'Something else is deprecated.');
}));

internalUtil.printDeprecationMessage('Something else is deprecated.');
process.emitWarning('Something else is deprecated.', 'DeprecationWarning');
}));