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

Jump to bottom

ESM: remove code for Node v12 #4874

Merged
merged 2 commits into from
Apr 24, 2022
Merged

ESM: remove code for Node v12 #4874

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
d081633
chore(esm): remove code for Node v12
juergba Apr 20, 2022
15353fe
various
juergba Apr 20, 2022
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
110 changes: 40 additions & 70 deletions lib/nodejs/esm-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const formattedImport = async file => {
// the location of the syntax error in the error thrown.
// This is problematic because the user can't see what file has the problem,
// so we add the file location to the error.
// This `if` should be removed once Node.js fixes the problem.
// TODO: remove once Node.js fixes the problem.
if (
err instanceof SyntaxError &&
err.message &&
Expand All @@ -30,64 +30,52 @@ const formattedImport = async file => {
return import(file);
};

const hasStableEsmImplementation = (() => {
const [major, minor] = process.version.split('.');
// ESM is stable from v12.22.0 onward
// https://nodejs.org/api/esm.html#esm_modules_ecmascript_modules
const majorNumber = parseInt(major.slice(1), 10);
const minorNumber = parseInt(minor, 10);
return majorNumber > 12 || (majorNumber === 12 && minorNumber >= 22);
})();

exports.requireOrImport = hasStableEsmImplementation
? async file => {
if (path.extname(file) === '.mjs') {
return formattedImport(file);
}
exports.requireOrImport = async file => {
if (path.extname(file) === '.mjs') {
return formattedImport(file);
}
try {
return dealWithExports(await formattedImport(file));
} catch (err) {
if (
err.code === 'ERR_MODULE_NOT_FOUND' ||
err.code === 'ERR_UNKNOWN_FILE_EXTENSION' ||
err.code === 'ERR_UNSUPPORTED_DIR_IMPORT'
) {
try {
return dealWithExports(await formattedImport(file));
} catch (err) {
// Importing a file usually works, but the resolution of `import` is the ESM
// resolution algorithm, and not the CJS resolution algorithm. We may have
// failed because we tried the ESM resolution, so we try to `require` it.
return require(file);
} catch (requireErr) {
if (
err.code === 'ERR_MODULE_NOT_FOUND' ||
err.code === 'ERR_UNKNOWN_FILE_EXTENSION' ||
err.code === 'ERR_UNSUPPORTED_DIR_IMPORT'
requireErr.code === 'ERR_REQUIRE_ESM' ||
(requireErr instanceof SyntaxError &&
requireErr
.toString()
.includes('Cannot use import statement outside a module'))
) {
try {
// Importing a file usually works, but the resolution of `import` is the ESM
// resolution algorithm, and not the CJS resolution algorithm. So in this case
// if we fail, we may have failed because we tried the ESM resolution and failed
// So we try to `require` it
return require(file);
} catch (requireErr) {
if (
requireErr.code === 'ERR_REQUIRE_ESM' ||
(requireErr instanceof SyntaxError &&
requireErr
.toString()
.includes('Cannot use import statement outside a module'))
) {
// ERR_REQUIRE_ESM happens when the test file is a JS file, but via type:module is actually ESM,
// AND has an import to a file that doesn't exist.
// This throws an `ERR_MODULE_NOT_FOUND` error above,
// and when we try to `require` it here, it throws an `ERR_REQUIRE_ESM`.
// What we want to do is throw the original error (the `ERR_MODULE_NOT_FOUND`),
// and not the `ERR_REQUIRE_ESM` error, which is a red herring.
//
// SyntaxError happens when in an edge case: when we're using an ESM loader that loads
// a `test.ts` file (i.e. unrecognized extension), and that file includes an unknown
// import (which thows an ERR_MODULE_NOT_FOUND). require-ing it will throw the
// syntax error, because we cannot require a file that has import-s.
throw err;
} else {
throw requireErr;
}
}
} else {
// ERR_REQUIRE_ESM happens when the test file is a JS file, but via type:module is actually ESM,
// AND has an import to a file that doesn't exist.
// This throws an `ERR_MODULE_NOT_FOUND` error above,
// and when we try to `require` it here, it throws an `ERR_REQUIRE_ESM`.
// What we want to do is throw the original error (the `ERR_MODULE_NOT_FOUND`),
// and not the `ERR_REQUIRE_ESM` error, which is a red herring.
//
// SyntaxError happens when in an edge case: when we're using an ESM loader that loads
// a `test.ts` file (i.e. unrecognized extension), and that file includes an unknown
// import (which throws an ERR_MODULE_NOT_FOUND). `require`-ing it will throw the
// syntax error, because we cannot require a file that has `import`-s.
throw err;
} else {
throw requireErr;
}
}
} else {
throw err;
}
: implementationOfRequireOrImportForUnstableEsm;
}
};

function dealWithExports(module) {
if (module.default) {
Expand All @@ -104,21 +92,3 @@ exports.loadFilesAsync = async (files, preLoadFunc, postLoadFunc) => {
postLoadFunc(file, result);
}
};

/* istanbul ignore next */
async function implementationOfRequireOrImportForUnstableEsm(file) {
if (path.extname(file) === '.mjs') {
return formattedImport(file);
}
// This is currently the only known way of figuring out whether a file is CJS or ESM in
// Node.js that doesn't necessitate calling `import` first.
try {
return require(file);
} catch (err) {
if (err.code === 'ERR_REQUIRE_ESM') {
return formattedImport(file);
} else {
throw err;
}
}
}
6 changes: 0 additions & 6 deletions test/integration/diffs.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,6 @@ describe('diffs', function () {
var diffs, expected;

before(function (done) {
// @TODO: It should be removed when Node.js 10 LTS is not supported.
const nodeVersion = parseInt(process.version.match(/^v(\d+)\./)[1], 10);
if (nodeVersion === 10) {
this.skip();
}

run('diffs/diffs.fixture.js', [], function (err, res) {
if (err) {
done(err);
Expand Down
15 changes: 2 additions & 13 deletions test/integration/glob.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,13 +187,6 @@ var testGlob = {
})
};

var isFlakeyNode = (function () {
var version = process.versions.node.split('.');
return (
version[0] === '0' && version[1] === '10' && process.platform === 'win32'
);
})();

function execMochaWith(validate) {
return function execMocha(glob, assertOn, done) {
exec(
Expand All @@ -206,12 +199,8 @@ function execMochaWith(validate) {
function (error, stdout, stderr) {
try {
validate(error, stderr);
if (isFlakeyNode && error && stderr === '') {
execMocha(glob, assertOn, done);
} else {
assertOn({stdout: stdout, stderr: stderr});
done();
}
assertOn({stdout: stdout, stderr: stderr});
done();
} catch (assertion) {
done(assertion);
}
Expand Down
6 changes: 0 additions & 6 deletions test/integration/no-diff.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,6 @@ var run = helpers.runMocha;
describe('no-diff', function () {
describe('when enabled', function () {
it('should not display a diff', function (done) {
// @TODO: It should be removed when Node.js 10 LTS is not supported.
const nodeVersion = parseInt(process.version.match(/^v(\d+)\./)[1], 10);
if (nodeVersion === 10) {
this.skip();
}

run('no-diff.fixture.js', ['--no-diff'], function (err, res) {
if (err) {
done(err);
Expand Down