Skip to content
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

Use import() on esm files in supported node versions #547

Merged
merged 2 commits into from
Jul 23, 2021
Merged

Use import() on esm files in supported node versions #547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
28d6e51
[New] Use import() on esm files in supported node versions
lkmill Feb 22, 2021
e6d2faf
[Tests] uncaught exceptions and unhandled rejections importing files …
lkmill Jul 6, 2021
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
11 changes: 11 additions & 0 deletions .eslintrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,18 @@
}],
"strict": "error",
},
"ignorePatterns": [ "syntax-error.*" ],
"overrides": [
{
"files": ["*.mjs", "test/import/package_type/*.js"],
"extends": "@ljharb/eslint-config/esm",
},
{
"files": ["bin/import-or-require.js"],
"parserOptions": {
"ecmaVersion": 2020,
},
ljharb marked this conversation as resolved.
Show resolved Hide resolved
},
{
"files": ["test/async-await/*"],
"parserOptions": {
Expand Down
1 change: 1 addition & 0 deletions .nycrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"check-coverage": false,
"reporter": ["text-summary", "text", "html", "json"],
"exclude": [
"bin/import-or-require.js",
"coverage",
"example",
"test"
Expand Down
13 changes: 13 additions & 0 deletions bin/import-or-require.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use strict';

const { extname: extnamePath } = require('path');
const getPackageType = require('get-package-type');

module.exports = function (file) {
const ext = extnamePath(file);

if (ext === '.mjs' || (ext === '.js' && getPackageType.sync(file) === 'module')) {
return import(file);
}
require(file);
};
29 changes: 27 additions & 2 deletions bin/tape
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ var readFileSync = require('fs').readFileSync;
var parseOpts = require('minimist');
var glob = require('glob');
var ignore = require('dotignore');
var hasImport = require('has-dynamic-import');

var tape = require('../');

var opts = parseOpts(process.argv.slice(2), {
alias: { r: 'require', i: 'ignore' },
Expand Down Expand Up @@ -47,7 +50,7 @@ var files = opts._.reduce(function (result, arg) {
var files = glob.sync(arg);

if (!Array.isArray(files)) {
throw new TypeError('unknown error: glob.sync did not return an array or throw. Please report this.');
throw new TypeError('unknown error: glob.sync("' + arg + '") did not return an array or throw. Please report this.');
}

return result.concat(files);
Expand All @@ -57,6 +60,28 @@ var files = opts._.reduce(function (result, arg) {
return resolvePath(cwd, file);
});

files.forEach(function (x) { require(x); });
hasImport().then(function (hasSupport) {
// the nextTick callback gets called outside the promise chain, avoiding
// promises and unhandled rejections when only loading commonjs files
process.nextTick(importFiles, hasSupport);
});

function importFiles(hasSupport) {
if (!hasSupport) {
return files.forEach(function (x) { require(x); });
}

var importOrRequire = require('./import-or-require');

tape.wait();

var promise = files.reduce(function (promise, file) {
return promise ? promise.then(function () {
return importOrRequire(file);
}) : importOrRequire(file);
}, null);

return promise ? promise.then(function () { tape.run(); }) : tape.run();
}

// vim: ft=javascript
39 changes: 30 additions & 9 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,22 @@ var canExit = typeof process !== 'undefined' && process
;

exports = module.exports = (function () {
var wait = false;
var harness;
var lazyLoad = function () {
return getHarness().apply(this, arguments);
};

lazyLoad.wait = function () {
wait = true;
};

lazyLoad.run = function () {
var run = getHarness().run;

if (run) run();
};

lazyLoad.only = function () {
return getHarness().only.apply(this, arguments);
};
Expand Down Expand Up @@ -48,26 +59,25 @@ exports = module.exports = (function () {
function getHarness(opts) {
if (!opts) opts = {};
opts.autoclose = !canEmitExit;
if (!harness) harness = createExitHarness(opts);
if (!harness) harness = createExitHarness(opts, wait);
return harness;
}
})();

function createExitHarness(conf) {
function createExitHarness(conf, wait) {
if (!conf) conf = {};
var harness = createHarness({
autoclose: defined(conf.autoclose, false)
});
var running = false;
var ended = false;

var stream = harness.createStream({ objectMode: conf.objectMode });
var es = stream.pipe(conf.stream || createDefaultStream());
if (canEmitExit) {
es.on('error', function (err) { harness._exitCode = 1; });
if (wait) {
harness.run = run;
} else {
run();
}

var ended = false;
stream.on('end', function () { ended = true; });

if (conf.exit === false) return harness;
if (!canEmitExit || !canExit) return harness;

Expand All @@ -92,6 +102,17 @@ function createExitHarness(conf) {
});

return harness;

lkmill marked this conversation as resolved.
Show resolved Hide resolved
function run() {
if (running) return;
running = true;
var stream = harness.createStream({ objectMode: conf.objectMode });
var es = stream.pipe(conf.stream || createDefaultStream());
if (canEmitExit) {
es.on('error', function (err) { harness._exitCode = 1; });
}
stream.on('end', function () { ended = true; });
};
}

exports.createHarness = createHarness;
Expand Down
5 changes: 4 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@
"defined": "^1.0.0",
"dotignore": "^0.1.2",
"for-each": "^0.3.3",
"get-package-type": "^0.1.0",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m a bit concerned that this package’s engines declaration will cause false errors for yarn users, and anyone using engines-strict or https://npmjs.com/ls-engines, if it ever adds an engines declaration in the future (cc @coreyfarrell)

"glob": "^7.1.7",
"has": "^1.0.3",
"has-dynamic-import": "^2.0.0",
"inherits": "^2.0.4",
"is-regex": "^1.1.3",
"minimist": "^1.2.5",
Expand All @@ -41,6 +43,7 @@
"through": "^2.3.8"
},
"devDependencies": {
"@ljharb/eslint-config": "^17.6.0",
"array.prototype.flatmap": "^1.2.4",
"aud": "^1.1.5",
"concat-stream": "^1.6.2",
Expand All @@ -57,7 +60,7 @@
"prepublishOnly": "safe-publish-latest",
"prepublish": "!(type not-in-publish) || not-in-publish || npm run prepublishOnly",
"prelint": "eclint check",
"lint": "eslint . bin/*",
"lint": "eslint --ext .js,.cjs,.mjs . bin/*",
"pretest": "npm run lint",
"test": "npm run tests-only",
"posttest": "aud --production",
Expand Down
198 changes: 198 additions & 0 deletions test/import.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
'use strict';

var tap = require('tap');
var spawn = require('child_process').spawn;
var concat = require('concat-stream');
var hasDynamicImport = require('has-dynamic-import');
var assign = require('object.assign');

tap.test('importing mjs files', function (t) {
hasDynamicImport().then(function (hasSupport) {
if (hasSupport) {
var tc = function (rows) {
t.same(rows.toString('utf8'), [
'TAP version 13',
'# mjs-a',
'ok 1 test ran',
'# mjs-b',
'ok 2 test ran after mjs-a',
'# mjs-c',
'ok 3 test ran after mjs-b',
'# mjs-d',
'ok 4 test ran after mjs-c',
'# mjs-e',
'ok 5 test ran after mjs-d',
'# mjs-f',
'ok 6 test ran after mjs-e',
'# mjs-g',
'ok 7 test ran after mjs-f',
'# mjs-h',
'ok 8 test ran after mjs-g',
'',
'1..8',
'# tests 8',
'# pass 8',
'',
'# ok'
].join('\n') + '\n\n');
};

var ps = tape('import/mjs-*.mjs');
ps.stdout.pipe(concat(tc));
ps.stderr.pipe(process.stderr);
ps.on('exit', function (code) {
t.equal(code, 0);
t.end();
});
} else {
t.pass('does not support dynamic import');
t.end();
}
});
});

tap.test('importing type: "module" files', function (t) {
hasDynamicImport().then(function (hasSupport) {
if (hasSupport) {
var tc = function (rows) {
t.same(rows.toString('utf8'), [
'TAP version 13',
'# package-type-a',
'ok 1 test ran',
'# package-type-b',
'ok 2 test ran after package-type-a',
'# package-type-c',
'ok 3 test ran after package-type-b',
'',
'1..3',
'# tests 3',
'# pass 3',
'',
'# ok'
].join('\n') + '\n\n');
};

var ps = tape('import/package_type/*.js');
ps.stdout.pipe(concat(tc));
ps.stderr.pipe(process.stderr);
ps.on('exit', function (code) {
t.equal(code, 0);
t.end();
});
} else {
t.pass('does not support dynamic import');
t.end();
}
});
});

tap.test('errors importing test files', function (t) {
hasDynamicImport().then(function (hasSupport) {
var createTest = function (options) {
var message = options.error + ' in `' + options.mode + '` mode`';
var ps = tape(options.files, { env: { NODE_OPTIONS: '--unhandled-rejections=' + options.mode } });
ps.stderr.pipe(concat(options.unhandledRejection(message)));
ps.on('exit', function (code, sig) {
t.equal(code, options.exitCode, message + ' has exit code ' + options.exitCode);
});
};

var warning = function (message) {
return function (rows) {
t.match(rows.toString('utf8'), 'UnhandledPromiseRejectionWarning', 'should have unhandled rejection warning: ' + message);
};
};

var noWarning = function (message) {
return function (rows) {
t.notMatch(rows.toString('utf8'), 'UnhandledPromiseRejectionWarning', 'should not have unhandled rejection warning: ' + message);
};
};

if (hasSupport) {
var tests = [{
files: 'import/syntax-error.mjs import/mjs-a.mjs import/mjs-b.mjs',
error: 'syntax errors in first imported esm file',
mode: 'warn',
exitCode: 0,
unhandledRejection: warning
}, {
files: 'import/throws.mjs import/mjs-a.mjs import/mjs-b.mjs',
error: 'thrown errors in first imported esm file',
mode: 'warn',
exitCode: 0,
unhandledRejection: warning
}, {
files: 'import/mjs-a.mjs import/syntax-error.mjs',
error: 'syntax error in esm file',
mode: 'warn',
exitCode: 1,
unhandledRejection: warning
}, {
files: 'import/syntax-error.mjs',
error: 'syntax error in esm file',
mode: 'strict',
exitCode: 1,
unhandledRejection: noWarning
}, {
files: 'import/throws.mjs',
error: 'thrown error in esm file',
mode: 'strict',
exitCode: 1,
unhandledRejection: noWarning
}, {
files: 'import/syntax-error.cjs',
error: 'syntax error in cjs file',
mode: 'warn',
exitCode: 1,
unhandledRejection: noWarning
}, {
files: 'import/throws.cjs',
error: 'thrown error in cjs file',
mode: 'warn',
exitCode: 1,
unhandledRejection: noWarning
}, {
files: 'import/syntax-error.cjs',
error: 'syntax error in cjs file',
mode: 'strict',
exitCode: 1,
unhandledRejection: noWarning
}, {
files: 'import/throws.cjs',
error: 'thrown error in cjs file',
mode: 'strict',
exitCode: 1,
unhandledRejection: noWarning
}, {
files: 'import/mjs-a.mjs import/syntax-error.cjs',
error: 'syntax error in cjs file in loading promise',
mode: 'warn',
exitCode: 1,
unhandledRejection: warning
}, {
files: 'import/mjs-a.mjs import/syntax-error.cjs',
error: 'syntax error in cjs file in loading promise',
mode: 'strict',
exitCode: 1,
unhandledRejection: noWarning
}];

t.plan(tests.length * 2);

tests.map(createTest);
} else {
t.pass('does not support dynamic import');
t.end();
}
});
});

function tape(args, options) {
options = assign({ cwd: __dirname }, options);

var proc = require('child_process');
var bin = __dirname + '/../bin/tape';

return proc.spawn('node', [bin].concat(args.split(' ')), options);
}
Loading