Skip to content

process: remove undocumented now argument from emitWarning() #31643

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.

Sign up for GitHub

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

Merged
Trott merged 1 commit into from
Feb 8, 2020
Merged

process: remove undocumented now argument from emitWarning() #31643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
process: remove undocumented now argument from emitWarning() 
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: #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
Trott committed Feb 8, 2020
commit 8c18e91c8afc0629a097ecb193e41eab058d02bb
20 changes: 9 additions & 11 deletions lib/internal/modules/cjs/loader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
};