From 8c18e91c8afc0629a097ecb193e41eab058d02bb Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Tue, 4 Feb 2020 17:01:01 -1000
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/31643
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
---
 lib/internal/modules/cjs/loader.js | 20 +++++++-------
 lib/internal/process/warning.js    | 43 +++++++++++++++++++-----------
 2 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
index ed0f201bdb2de7..a5ea81b55f1df9 100644
--- a/lib/internal/modules/cjs/loader.js
+++ b/lib/internal/modules/cjs/loader.js
@@ -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
@@ -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.'
+    );
   }
 }
 
@@ -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
diff --git a/lib/internal/process/warning.js b/lib/internal/process/warning.js
index 0068451020c8e3..4a2ce2d6b203c8 100644
--- a/lib/internal/process/warning.js
+++ b/lib/internal/process/warning.js
@@ -5,6 +5,7 @@ const {
   Error,
 } = primordials;
 
+const assert = require('internal/assert');
 const { ERR_INVALID_ARG_TYPE } = require('internal/errors').codes;
 
 // Lazily loaded
@@ -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;
@@ -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);
   }
@@ -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
 };