Skip to content

Commit

Permalink
module: add warnings for experimental flags
Browse files Browse the repository at this point in the history
PR-URL: #30617
Fixes: #30600
Reviewed-By: Guy Bedford <guybedford@gmail.com>
  • Loading branch information
pd4d10 authored and targos committed Dec 9, 2019
1 parent 55a270b commit 851f313
Show file tree
Hide file tree
Showing 13 changed files with 133 additions and 8 deletions.
15 changes: 10 additions & 5 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
4 changes: 3 additions & 1 deletion lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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;
}

Expand Down
2 changes: 2 additions & 0 deletions src/node_process.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
16 changes: 16 additions & 0 deletions src/node_process_events.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include <cstdarg>
#include <set>

#include "env-inl.h"
#include "node_process.h"
Expand Down Expand Up @@ -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) {
Expand Down
16 changes: 16 additions & 0 deletions test/common/fixtures.mjs
Original file line number Diff line number Diff line change
@@ -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,
};
32 changes: 32 additions & 0 deletions test/es-module/test-esm-exports.mjs
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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'
));
});
});
24 changes: 23 additions & 1 deletion test/es-module/test-esm-json.mjs
Original file line number Diff line number Diff line change
@@ -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'
));
});
24 changes: 23 additions & 1 deletion test/es-module/test-esm-wasm.mjs
Original file line number Diff line number Diff line change
@@ -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');

Expand All @@ -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'
));
});
1 change: 1 addition & 0 deletions test/fixtures/es-modules/conditional-exports.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
require('pkgexports/condition')
1 change: 1 addition & 0 deletions test/fixtures/es-modules/json-modules.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import secret from '../experimental.json';
2 changes: 2 additions & 0 deletions test/fixtures/es-modules/wasm-modules.mjs
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';
1 change: 1 addition & 0 deletions test/fixtures/node_modules/pkgexports/resolve-self.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 851f313

Please sign in to comment.