Skip to content

Commit

Permalink
process: remove undocumented now argument from emitWarning()
Browse files Browse the repository at this point in the history
process.emitWarning() "now" option is undocumented and a Boolean trap.
Remove it before people start adopting it.

We only need it in one place internally. Replace it with an
internal-only emitWarningSync() function.

PR-URL: nodejs#31643
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
  • Loading branch information
Trott committed Feb 8, 2020
1 parent f368210 commit 8c18e91
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 27 deletions.
20 changes: 9 additions & 11 deletions lib/internal/modules/cjs/loader.js
Expand Up @@ -57,6 +57,7 @@ const assert = require('internal/assert');
const fs = require('fs');
const internalFS = require('internal/fs/utils');
const path = require('path');
const { emitWarningSync } = require('internal/process/warning');
const {
internalModuleReadJSON,
internalModuleStat
Expand Down Expand Up @@ -122,13 +123,13 @@ function enrichCJSError(err) {
*/
if (err.message.startsWith('Unexpected token \'export\'') ||
(/^\s*import(?=[ {'"*])\s*(?![ (])/).test(lineWithErr)) {
process.emitWarning(
// Emit the warning synchronously because we are in the middle of handling
// a SyntaxError that will throw and likely terminate the process before an
// asynchronous warning would be emitted.
emitWarningSync(
'To load an ES module, set "type": "module" in the package.json or use ' +
'the .mjs extension.',
undefined,
undefined,
undefined,
true);
'the .mjs extension.'
);
}
}

Expand Down Expand Up @@ -839,11 +840,8 @@ Module._resolveLookupPaths = function(request, parent) {
function emitCircularRequireWarning(prop) {
process.emitWarning(
`Accessing non-existent property '${String(prop)}' of module exports ` +
'inside circular dependency',
'Warning',
undefined, // code
undefined, // ctor
true); // emit now
'inside circular dependency'
);
}

// A Proxy that can be used as the prototype of a module.exports object and
Expand Down
43 changes: 27 additions & 16 deletions lib/internal/process/warning.js
Expand Up @@ -5,6 +5,7 @@ const {
Error,
} = primordials;

const assert = require('internal/assert');
const { ERR_INVALID_ARG_TYPE } = require('internal/errors').codes;

// Lazily loaded
Expand Down Expand Up @@ -87,7 +88,7 @@ function onWarning(warning) {
// process.emitWarning(error)
// process.emitWarning(str[, type[, code]][, ctor])
// process.emitWarning(str[, options])
function emitWarning(warning, type, code, ctor, now) {
function emitWarning(warning, type, code, ctor) {
let detail;
if (type !== null && typeof type === 'object' && !ArrayIsArray(type)) {
ctor = type.ctor;
Expand All @@ -110,18 +111,7 @@ function emitWarning(warning, type, code, ctor, now) {
throw new ERR_INVALID_ARG_TYPE('code', 'string', code);
}
if (typeof warning === 'string') {
// Improve error creation performance by skipping the error frames.
// They are added in the `captureStackTrace()` function below.
const tmpStackLimit = Error.stackTraceLimit;
Error.stackTraceLimit = 0;
// eslint-disable-next-line no-restricted-syntax
warning = new Error(warning);
Error.stackTraceLimit = tmpStackLimit;
warning.name = String(type || 'Warning');
if (code !== undefined) warning.code = code;
if (detail !== undefined) warning.detail = detail;
// eslint-disable-next-line no-restricted-syntax
Error.captureStackTrace(warning, ctor || process.emitWarning);
warning = createWarningObject(warning, type, code, ctor, detail);
} else if (!(warning instanceof Error)) {
throw new ERR_INVALID_ARG_TYPE('warning', ['Error', 'string'], warning);
}
Expand All @@ -131,11 +121,32 @@ function emitWarning(warning, type, code, ctor, now) {
if (process.throwDeprecation)
throw warning;
}
if (now) process.emit('warning', warning);
else process.nextTick(doEmitWarning(warning));
process.nextTick(doEmitWarning(warning));
}

function emitWarningSync(warning) {
process.emit('warning', createWarningObject(warning));
}

function createWarningObject(warning, type, code, ctor, detail) {
assert(typeof warning === 'string');
// Improve error creation performance by skipping the error frames.
// They are added in the `captureStackTrace()` function below.
const tmpStackLimit = Error.stackTraceLimit;
Error.stackTraceLimit = 0;
// eslint-disable-next-line no-restricted-syntax
warning = new Error(warning);
Error.stackTraceLimit = tmpStackLimit;
warning.name = String(type || 'Warning');
if (code !== undefined) warning.code = code;
if (detail !== undefined) warning.detail = detail;
// eslint-disable-next-line no-restricted-syntax
Error.captureStackTrace(warning, ctor || process.emitWarning);
return warning;
}

module.exports = {
emitWarning,
emitWarningSync,
onWarning,
emitWarning
};

0 comments on commit 8c18e91

Please sign in to comment.