module: add warnings for experimental flags

PR-URL: #30617 Fixes: #30600 Reviewed-By: Guy Bedford <guybedford@gmail.com>
nodejs · MylesBorins · Dec 17, 2019 · May 19, 2019 · Aug 19, 2019 · Sep 3, 2019
commit 851f3135aba0fdbf0fc89f1ef346adf438f4c399
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
@@ -43,7 +43,7 @@ const {
   rekeySourceMap
 } = require('internal/source_map/source_map_cache');
 const { pathToFileURL, fileURLToPath, URL } = require('internal/url');
-const { deprecate } = require('internal/util');
+const { deprecate, emitExperimentalWarning } = require('internal/util');
 const vm = require('vm');
 const assert = require('internal/assert');
 const fs = require('fs');
@@ -584,17 +584,21 @@ function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) {
     if (experimentalConditionalExports &&
         ObjectPrototypeHasOwnProperty(target, 'require')) {
       try {
-        return resolveExportsTarget(pkgPath, target.require, subpath,
-                                    basePath, mappingKey);
+        const result = resolveExportsTarget(pkgPath, target.require, subpath,
+                                            basePath, mappingKey);
+        emitExperimentalWarning('Conditional exports');
+        return result;
       } catch (e) {
         if (e.code !== 'MODULE_NOT_FOUND') throw e;
       }
     }
     if (experimentalConditionalExports &&
         ObjectPrototypeHasOwnProperty(target, 'node')) {
       try {
-        return resolveExportsTarget(pkgPath, target.node, subpath,
-                                    basePath, mappingKey);
+        const result = resolveExportsTarget(pkgPath, target.node, subpath,
+                                            basePath, mappingKey);
+        emitExperimentalWarning('Conditional exports');
+        return result;
       } catch (e) {
         if (e.code !== 'MODULE_NOT_FOUND') throw e;
       }
@@ -697,6 +701,7 @@ Module._findPath = function(request, paths, isMain) {
 
   const selfFilename = trySelf(paths, exts, isMain, trailingSlash, request);
   if (selfFilename) {
+    emitExperimentalWarning('Package name self resolution');
     Module._pathCache[cacheKey] = selfFilename;
     return selfFilename;
   }

diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js
@@ -22,7 +22,7 @@ const createDynamicModule = require(
 const fs = require('fs');
 const { fileURLToPath, URL } = require('url');
 const { debuglog } = require('internal/util/debuglog');
-const { promisify } = require('internal/util');
+const { promisify, emitExperimentalWarning } = require('internal/util');
 const {
   ERR_INVALID_URL,
   ERR_INVALID_URL_SCHEME,
@@ -133,6 +133,7 @@ translators.set('builtin', async function builtinStrategy(url) {
 
 // Strategy for loading a JSON file
 translators.set('json', async function jsonStrategy(url) {
+  emitExperimentalWarning('Importing JSON modules');
   debug(`Translating JSONModule ${url}`);
   debug(`Loading JSONModule ${url}`);
   const pathname = url.startsWith('file:') ? fileURLToPath(url) : null;
@@ -187,6 +188,7 @@ translators.set('json', async function jsonStrategy(url) {
 
 // Strategy for loading a wasm module
 translators.set('wasm', async function(url) {
+  emitExperimentalWarning('Importing Web Assembly modules');
   const buffer = await getSource(url);
   debug(`Translating WASMModule ${url}`);
   let compiled;

diff --git a/src/module_wrap.cc b/src/module_wrap.cc
@@ -7,6 +7,7 @@
 #include "util-inl.h"
 #include "node_contextify.h"
 #include "node_watchdog.h"
+#include "node_process.h"
 
 #include <sys/stat.h>  // S_IFDIR
 
@@ -962,6 +963,7 @@ Maybe<URL> ResolveExportsTarget(Environment* env,
       Maybe<URL> resolved = ResolveExportsTarget(env, pjson_url,
             conditionalTarget, subpath, pkg_subpath, base, false);
       if (!resolved.IsNothing()) {
+        ProcessEmitExperimentalWarning(env, "Conditional exports");
         return resolved;
       }
     }
@@ -1267,6 +1269,7 @@ Maybe<URL> PackageResolve(Environment* env,
 
   Maybe<URL> self_url = ResolveSelf(env, specifier, base);
   if (self_url.IsJust()) {
+    ProcessEmitExperimentalWarning(env, "Package name self resolution");
     return self_url;
   }
 

diff --git a/src/node_process.h b/src/node_process.h
@@ -27,6 +27,8 @@ v8::Maybe<bool> ProcessEmitWarningGeneric(Environment* env,
                                           const char* code = nullptr);
 
 v8::Maybe<bool> ProcessEmitWarning(Environment* env, const char* fmt, ...);
+v8::Maybe<bool> ProcessEmitExperimentalWarning(Environment* env,
+                                              const char* warning);
 v8::Maybe<bool> ProcessEmitDeprecationWarning(Environment* env,
                                               const char* warning,
                                               const char* deprecation_code);

diff --git a/src/node_process_events.cc b/src/node_process_events.cc
@@ -1,4 +1,5 @@
 #include <cstdarg>
+#include <set>
 
 #include "env-inl.h"
 #include "node_process.h"
@@ -95,6 +96,21 @@ Maybe<bool> ProcessEmitWarning(Environment* env, const char* fmt, ...) {
   return ProcessEmitWarningGeneric(env, warning);
 }
 
+
+std::set<std::string> experimental_warnings;
+
+Maybe<bool> ProcessEmitExperimentalWarning(Environment* env,
+                                          const char* warning) {
+  if (experimental_warnings.find(warning) != experimental_warnings.end())
+    return Nothing<bool>();
+
+  experimental_warnings.insert(warning);
+  std::string message(warning);
+  message.append(
+      " is an experimental feature. This feature could change at any time");
+  return ProcessEmitWarningGeneric(env, message.c_str(), "ExperimentalWarning");
+}
+
 Maybe<bool> ProcessEmitDeprecationWarning(Environment* env,
                                           const char* warning,
                                           const char* deprecation_code) {

diff --git a/test/common/fixtures.mjs b/test/common/fixtures.mjs
@@ -0,0 +1,16 @@
+/* eslint-disable node-core/require-common-first, node-core/required-modules */
+import fixtures from './fixtures.js';
+
+const {
+  fixturesDir,
+  path,
+  readSync,
+  readKey,
+} = fixtures;
+
+export {
+  fixturesDir,
+  path,
+  readSync,
+  readKey,
+};
diff --git a/test/es-module/test-esm-exports.mjs b/test/es-module/test-esm-exports.mjs
@@ -1,6 +1,8 @@
 // Flags: --experimental-modules
 import { mustCall } from '../common/index.mjs';
+import { path } from '../common/fixtures.mjs';
 import { ok, deepStrictEqual, strictEqual } from 'assert';
+import { spawn } from 'child_process';
 
 import { requireFixture, importFixture } from '../fixtures/pkgexports.mjs';
 import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js';
@@ -149,3 +151,33 @@ function assertIncludes(actual, expected) {
   ok(actual.toString().indexOf(expected) !== -1,
      `${JSON.stringify(actual)} includes ${JSON.stringify(expected)}`);
 }
+
+// Test warning message
+[
+  [
+    '--experimental-conditional-exports',
+    '/es-modules/conditional-exports.js',
+    'Conditional exports',
+  ],
+  [
+    '--experimental-resolve-self',
+    '/node_modules/pkgexports/resolve-self.js',
+    'Package name self resolution',
+  ],
+].forEach(([flag, file, message]) => {
+  const child = spawn(process.execPath, [flag, path(file)]);
+
+  let stderr = '';
+  child.stderr.setEncoding('utf8');
+  child.stderr.on('data', (data) => {
+    stderr += data;
+  });
+  child.on('close', (code, signal) => {
+    strictEqual(code, 0);
+    strictEqual(signal, null);
+    ok(stderr.toString().includes(
+      `ExperimentalWarning: ${message} is an experimental feature. ` +
+      'This feature could change at any time'
+    ));
+  });
+});
diff --git a/test/es-module/test-esm-json.mjs b/test/es-module/test-esm-json.mjs
@@ -1,7 +1,29 @@
 // Flags: --experimental-json-modules
 import '../common/index.mjs';
-import { strictEqual } from 'assert';
+import { path } from '../common/fixtures.mjs';
+import { strictEqual, ok } from 'assert';
+import { spawn } from 'child_process';
 
 import secret from '../fixtures/experimental.json';
 
 strictEqual(secret.ofLife, 42);
+
+// Test warning message
+const child = spawn(process.execPath, [
+  '--experimental-json-modules',
+  path('/es-modules/json-modules.mjs')
+]);
+
+let stderr = '';
+child.stderr.setEncoding('utf8');
+child.stderr.on('data', (data) => {
+  stderr += data;
+});
+child.on('close', (code, signal) => {
+  strictEqual(code, 0);
+  strictEqual(signal, null);
+  ok(stderr.toString().includes(
+    'ExperimentalWarning: Importing JSON modules is an experimental feature. ' +
+    'This feature could change at any time'
+  ));
+});
diff --git a/test/es-module/test-esm-wasm.mjs b/test/es-module/test-esm-wasm.mjs
@@ -1,8 +1,10 @@
 // Flags: --experimental-wasm-modules
 import '../common/index.mjs';
+import { path } from '../common/fixtures.mjs';
 import { add, addImported } from '../fixtures/es-modules/simple.wasm';
 import { state } from '../fixtures/es-modules/wasm-dep.mjs';
-import { strictEqual } from 'assert';
+import { strictEqual, ok } from 'assert';
+import { spawn } from 'child_process';
 
 strictEqual(state, 'WASM Start Executed');
 
@@ -13,3 +15,23 @@ strictEqual(addImported(0), 42);
 strictEqual(state, 'WASM JS Function Executed');
 
 strictEqual(addImported(1), 43);
+
+// Test warning message
+const child = spawn(process.execPath, [
+  '--experimental-wasm-modules',
+  path('/es-modules/wasm-modules.mjs')
+]);
+
+let stderr = '';
+child.stderr.setEncoding('utf8');
+child.stderr.on('data', (data) => {
+  stderr += data;
+});
+child.on('close', (code, signal) => {
+  strictEqual(code, 0);
+  strictEqual(signal, null);
+  ok(stderr.toString().includes(
+    'ExperimentalWarning: Importing Web Assembly modules is ' +
+    'an experimental feature. This feature could change at any time'
+  ));
+});
diff --git a/test/fixtures/es-modules/conditional-exports.js b/test/fixtures/es-modules/conditional-exports.js
@@ -0,0 +1 @@
+require('pkgexports/condition')
diff --git a/test/fixtures/es-modules/json-modules.mjs b/test/fixtures/es-modules/json-modules.mjs
@@ -0,0 +1 @@
+import secret from '../experimental.json';
diff --git a/test/fixtures/es-modules/wasm-modules.mjs b/test/fixtures/es-modules/wasm-modules.mjs
@@ -0,0 +1,2 @@
+import { add, addImported } from './simple.wasm';
+import { state } from './wasm-dep.mjs';
diff --git a/test/fixtures/node_modules/pkgexports/resolve-self.js b/test/fixtures/node_modules/pkgexports/resolve-self.js
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		import { add, addImported } from './simple.wasm';
		import { state } from './wasm-dep.mjs';