Skip to content

Commit

Permalink
loader: warn when the user imports a module with a thenable namespace
Browse files Browse the repository at this point in the history
A module with a thenable namespace will most likely make the import fail
so warn the user when it happens. As an example, if your entry file
imports a thenable namespace and expects to continue execution after
getting it, node will currently exit with no message.
  • Loading branch information
devsnek committed May 25, 2018
1 parent 8ce20af commit fc2ad59
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 1 deletion.
14 changes: 13 additions & 1 deletion lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,16 @@ const defaultResolve = require('internal/modules/esm/default_resolve');
const createDynamicModule = require(
'internal/modules/esm/create_dynamic_module');
const translators = require('internal/modules/esm/translators');
const { SafeWeakSet } = require('internal/safe_globals');

const FunctionBind = Function.call.bind(Function.prototype.bind);

const debug = require('util').debuglog('esm');

const thenableNamespaces = new SafeWeakSet();
const thenableWarning = (x) => `The namespace of ${x} is thenable. \
See https://mdn.io/thenable for more information.`;

/* A Loader instance is used as the main entry point for loading ES modules.
* Currently, this is a singleton -- there is only one used for loading
* the main module and everything in its dependency graph. */
Expand Down Expand Up @@ -73,7 +78,14 @@ class Loader {
async import(specifier, parent) {
const job = await this.getModuleJob(specifier, parent);
const module = await job.run();
return module.namespace();
const m = module.namespace();
if (typeof m.then === 'function' && m.then.length === 0 &&
!thenableNamespaces.has(m) &&
(!parent || !/node_modules/.test(parent))) {
process.emitWarning(thenableWarning(module.url), 'loader');
thenableNamespaces.add(m);
}
return m;
}

hook({ resolve, dynamicInstantiate }) {
Expand Down
1 change: 1 addition & 0 deletions lib/internal/safe_globals.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,5 @@ const makeSafe = (unsafe, safe) => {

exports.SafeMap = makeSafe(Map, class SafeMap extends Map {});
exports.SafeSet = makeSafe(Set, class SafeSet extends Set {});
exports.SafeWeakSet = makeSafe(WeakSet, class SafeWeakSet extends WeakSet {});
exports.SafePromise = makeSafe(Promise, class SafePromise extends Promise {});
15 changes: 15 additions & 0 deletions test/es-module/test-thenable-warning.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use strict';

// Flags: --experimental-modules

const common = require('../common');
const assert = require('assert');

common.crashOnUnhandledRejection();

const re = /^The namespace of .+? is thenable\. See https:\/\/mdn\.io\/thenable for more information\.$/;
process.on('warning', common.mustCall((warning) => {
assert(re.test(warning.message));
}));

import('../fixtures/es-modules/thenable.mjs');
1 change: 1 addition & 0 deletions test/fixtures/es-modules/thenable.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export function then() {}

0 comments on commit fc2ad59

Please sign in to comment.