From 1ecf9f7a4ffa736dfbc32e2ddfec76c0cf382b1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Jos=C3=A9=20Arboleda?= Date: Wed, 27 Nov 2019 20:26:36 -0500 Subject: [PATCH 1/7] lib,test: improves ERR_REQUIRE_ESM message Fixes: https://github.com/nodejs/node/issues/30599 --- lib/internal/modules/cjs/loader.js | 16 +++++++++++++--- test/es-module/test-cjs-esm-warn.js | 3 +++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 0b98fb9d4f0568..19d4d121fb4972 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -1195,13 +1195,16 @@ let warnRequireESM = true; Module._extensions['.js'] = function(module, filename) { if (filename.endsWith('.js')) { const pkg = readPackageScope(filename); + // Function require shouldn't be used in ES modules. if (pkg && pkg.data && pkg.data.type === 'module') { + const err = new ERR_REQUIRE_ESM(filename); if (warnRequireESM) { const parentPath = module.parent && module.parent.filename; const basename = parentPath && path.basename(filename) === path.basename(parentPath) ? filename : path.basename(filename); - process.emitWarning( + + const warningMsg = 'require() of ES modules is not supported.\nrequire() of ' + `${filename} ${parentPath ? `from ${module.parent.filename} ` : ''}` + 'is an ES module file as it is a .js file whose nearest parent ' + @@ -1209,7 +1212,14 @@ Module._extensions['.js'] = function(module, filename) { 'files in that package scope as ES modules.\nInstead rename ' + `${basename} to end in .cjs, change the requiring code to use ` + 'import(), or remove "type": "module" from ' + - `${path.resolve(pkg.path, 'package.json')}.`, + `${path.resolve(pkg.path, 'package.json')}.\n` + + err.message; + + // The error message should be (in this case) the as the warning. + // https://github.com/nodejs/node/issues/30599 + err.message = warningMsg; + process.emitWarning( + warningMsg, undefined, undefined, undefined, @@ -1217,7 +1227,7 @@ Module._extensions['.js'] = function(module, filename) { ); warnRequireESM = false; } - throw new ERR_REQUIRE_ESM(filename); + throw err; } } const content = fs.readFileSync(filename, 'utf8'); diff --git a/test/es-module/test-cjs-esm-warn.js b/test/es-module/test-cjs-esm-warn.js index b1b2e7f434256f..20d919c1c02889 100644 --- a/test/es-module/test-cjs-esm-warn.js +++ b/test/es-module/test-cjs-esm-warn.js @@ -37,4 +37,7 @@ child.on('close', common.mustCall((code, signal) => { `${pjson}.\n`)); assert.ok(stderr.indexOf( 'Error [ERR_REQUIRE_ESM]: Must use import to load ES Module') !== -1); + + assert.strictEqual( + stderr.match(/Must use import to load ES Module/g).length, 2); })); From f112525b56f8b86bc238cfb8621a2beca846f994 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Jos=C3=A9=20Arboleda?= Date: Wed, 27 Nov 2019 21:12:57 -0500 Subject: [PATCH 2/7] fixup --- test/es-module/test-esm-type-flag-errors.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/es-module/test-esm-type-flag-errors.js b/test/es-module/test-esm-type-flag-errors.js index 5d19cedd84a844..e0dedc16cabf8c 100644 --- a/test/es-module/test-esm-type-flag-errors.js +++ b/test/es-module/test-esm-type-flag-errors.js @@ -27,7 +27,10 @@ try { require('../fixtures/es-modules/package-type-module/index.js'); assert.fail('Expected CJS to fail loading from type: module package.'); } catch (e) { - assert(e.toString().match(/Error \[ERR_REQUIRE_ESM\]: Must use import to load ES Module:/)); + assert.strictEqual(e.name, 'Error'); + assert.strictEqual(e.code, 'ERR_REQUIRE_ESM'); + assert(e.toString().match(/Must use import to load ES Module/g)); + assert(e.message.match(/Must use import to load ES Module/g)); } function expect(opt = '', inputFile, want, wantsError = false) { From 42f7d59b8d00c7fdf1ccfd4b178c98b7dcbf24de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Jos=C3=A9=20Arboleda?= Date: Wed, 27 Nov 2019 21:49:29 -0500 Subject: [PATCH 3/7] fixup --- lib/internal/modules/cjs/loader.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 19d4d121fb4972..6d058325b634a5 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -1203,7 +1203,6 @@ Module._extensions['.js'] = function(module, filename) { const basename = parentPath && path.basename(filename) === path.basename(parentPath) ? filename : path.basename(filename); - const warningMsg = 'require() of ES modules is not supported.\nrequire() of ' + `${filename} ${parentPath ? `from ${module.parent.filename} ` : ''}` + @@ -1212,19 +1211,18 @@ Module._extensions['.js'] = function(module, filename) { 'files in that package scope as ES modules.\nInstead rename ' + `${basename} to end in .cjs, change the requiring code to use ` + 'import(), or remove "type": "module" from ' + - `${path.resolve(pkg.path, 'package.json')}.\n` + - err.message; - - // The error message should be (in this case) the as the warning. - // https://github.com/nodejs/node/issues/30599 - err.message = warningMsg; + `${path.resolve(pkg.path, 'package.json')}.\n`; process.emitWarning( - warningMsg, + warningMsg + err.message, undefined, undefined, undefined, true ); + + // The error message should be (in this case) the as the warning. + // https://github.com/nodejs/node/issues/30599 + err.message += `\n${warningMsg}`; warnRequireESM = false; } throw err; From 64c96777dabe4cb1b8bf861dacdf2c66ec223907 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Jos=C3=A9=20Arboleda?= Date: Tue, 3 Dec 2019 00:47:08 -0500 Subject: [PATCH 4/7] fixup --- lib/internal/errors.js | 28 +++++++++++++++++++++++++- lib/internal/modules/cjs/loader.js | 31 +---------------------------- test/es-module/test-cjs-esm-warn.js | 9 +++++---- 3 files changed, 33 insertions(+), 35 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 7dd976e299c079..735d4084977d5c 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -90,6 +90,13 @@ function lazyBuffer() { return buffer; } +let pathModule; +function lazyPath() { + if (pathModule === undefined) + pathModule = require('path'); + return pathModule; +} + // A specialized Error that includes an additional info property with // additional information about the error condition. // It has the properties present in a UVException but with a custom error @@ -1122,7 +1129,26 @@ E('ERR_OUT_OF_RANGE', msg += ` It must be ${range}. Received ${received}`; return msg; }, RangeError); -E('ERR_REQUIRE_ESM', 'Must use import to load ES Module: %s', Error); +E('ERR_REQUIRE_ESM', (filename, module = null, pkg = null) => { + let msg = `Must use import to load ES Module: ${filename}\n`; + if (module && pkg) { + const parentPath = module.parent && module.parent.filename; + const basename = parentPath && + lazyPath().basename(filename) === lazyPath().basename(parentPath) ? + filename : lazyPath().basename(filename); + msg += + 'require() of ES modules is not supported.\nrequire() of ' + + `${filename} ${parentPath ? `from ${module.parent.filename} ` : ''}` + + 'is an ES module file as it is a .js file whose nearest parent ' + + 'package.json contains "type": "module" which defines all .js ' + + 'files in that package scope as ES modules.\nInstead rename ' + + `${basename} to end in .cjs, change the requiring code to use ` + + 'import(), or remove "type": "module" from ' + + `${lazyPath().resolve(pkg.path, 'package.json')}.\n`; + return msg; + } + return msg; +}, Error); E('ERR_SCRIPT_EXECUTION_INTERRUPTED', 'Script execution was interrupted by `SIGINT`', Error); E('ERR_SERVER_ALREADY_LISTEN', diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 6d058325b634a5..ebf51fb9154a87 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -1191,41 +1191,12 @@ Module.prototype._compile = function(content, filename) { }; // Native extension for .js -let warnRequireESM = true; Module._extensions['.js'] = function(module, filename) { if (filename.endsWith('.js')) { const pkg = readPackageScope(filename); // Function require shouldn't be used in ES modules. if (pkg && pkg.data && pkg.data.type === 'module') { - const err = new ERR_REQUIRE_ESM(filename); - if (warnRequireESM) { - const parentPath = module.parent && module.parent.filename; - const basename = parentPath && - path.basename(filename) === path.basename(parentPath) ? - filename : path.basename(filename); - const warningMsg = - 'require() of ES modules is not supported.\nrequire() of ' + - `${filename} ${parentPath ? `from ${module.parent.filename} ` : ''}` + - 'is an ES module file as it is a .js file whose nearest parent ' + - 'package.json contains "type": "module" which defines all .js ' + - 'files in that package scope as ES modules.\nInstead rename ' + - `${basename} to end in .cjs, change the requiring code to use ` + - 'import(), or remove "type": "module" from ' + - `${path.resolve(pkg.path, 'package.json')}.\n`; - process.emitWarning( - warningMsg + err.message, - undefined, - undefined, - undefined, - true - ); - - // The error message should be (in this case) the as the warning. - // https://github.com/nodejs/node/issues/30599 - err.message += `\n${warningMsg}`; - warnRequireESM = false; - } - throw err; + throw new ERR_REQUIRE_ESM(filename, module, pkg); } } const content = fs.readFileSync(filename, 'utf8'); diff --git a/test/es-module/test-cjs-esm-warn.js b/test/es-module/test-cjs-esm-warn.js index 20d919c1c02889..b800a47d0515d5 100644 --- a/test/es-module/test-cjs-esm-warn.js +++ b/test/es-module/test-cjs-esm-warn.js @@ -26,18 +26,19 @@ child.on('close', common.mustCall((code, signal) => { assert.strictEqual(code, 1); assert.strictEqual(signal, null); - assert.ok(stderr.startsWith(`(node:${child.pid}) Warning: ` + - 'require() of ES modules is not supported.\nrequire() of ' + + assert.ok(stderr.indexOf( + `Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: ${required}` + + '\nrequire() of ES modules is not supported.\nrequire() of ' + `${required} from ${requiring} ` + 'is an ES module file as it is a .js file whose nearest parent ' + 'package.json contains "type": "module" which defines all .js ' + 'files in that package scope as ES modules.\nInstead rename ' + `${basename} to end in .cjs, change the requiring code to use ` + 'import(), or remove "type": "module" from ' + - `${pjson}.\n`)); + `${pjson}.\n`) !== -1); assert.ok(stderr.indexOf( 'Error [ERR_REQUIRE_ESM]: Must use import to load ES Module') !== -1); assert.strictEqual( - stderr.match(/Must use import to load ES Module/g).length, 2); + stderr.match(/Must use import to load ES Module/g).length, 1); })); From b22ec8d2a244d5bf8d8959c7704a157b0aa68429 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Jos=C3=A9=20Arboleda?= Date: Tue, 3 Dec 2019 19:02:03 -0500 Subject: [PATCH 5/7] fixup --- lib/internal/errors.js | 40 +++++++++++------------------- lib/internal/modules/cjs/loader.js | 6 ++++- 2 files changed, 20 insertions(+), 26 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 735d4084977d5c..900a1ee8274037 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -90,13 +90,6 @@ function lazyBuffer() { return buffer; } -let pathModule; -function lazyPath() { - if (pathModule === undefined) - pathModule = require('path'); - return pathModule; -} - // A specialized Error that includes an additional info property with // additional information about the error condition. // It has the properties present in a UVException but with a custom error @@ -1129,25 +1122,22 @@ E('ERR_OUT_OF_RANGE', msg += ` It must be ${range}. Received ${received}`; return msg; }, RangeError); -E('ERR_REQUIRE_ESM', (filename, module = null, pkg = null) => { - let msg = `Must use import to load ES Module: ${filename}\n`; - if (module && pkg) { - const parentPath = module.parent && module.parent.filename; - const basename = parentPath && - lazyPath().basename(filename) === lazyPath().basename(parentPath) ? - filename : lazyPath().basename(filename); - msg += - 'require() of ES modules is not supported.\nrequire() of ' + - `${filename} ${parentPath ? `from ${module.parent.filename} ` : ''}` + - 'is an ES module file as it is a .js file whose nearest parent ' + - 'package.json contains "type": "module" which defines all .js ' + - 'files in that package scope as ES modules.\nInstead rename ' + - `${basename} to end in .cjs, change the requiring code to use ` + - 'import(), or remove "type": "module" from ' + - `${lazyPath().resolve(pkg.path, 'package.json')}.\n`; +E('ERR_REQUIRE_ESM', + (filename, parentPath = null, packageJsonPath = null, basename = null) => { + let msg = `Must use import to load ES Module: ${filename}\n`; + if (parentPath && packageJsonPath) { + msg += + 'require() of ES modules is not supported.\nrequire() of ' + + `${filename} ${parentPath ? `from ${parentPath} ` : ''}` + + 'is an ES module file as it is a .js file whose nearest parent ' + + 'package.json contains "type": "module" which defines all .js ' + + 'files in that package scope as ES modules.\nInstead rename ' + + `${basename} to end in .cjs, change the requiring code to use ` + + 'import(), or remove "type": "module" from ' + + `${packageJsonPath}.\n`; + return msg; + } return msg; - } - return msg; }, Error); E('ERR_SCRIPT_EXECUTION_INTERRUPTED', 'Script execution was interrupted by `SIGINT`', Error); diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index ebf51fb9154a87..168b2a860978a0 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -1196,7 +1196,11 @@ Module._extensions['.js'] = function(module, filename) { const pkg = readPackageScope(filename); // Function require shouldn't be used in ES modules. if (pkg && pkg.data && pkg.data.type === 'module') { - throw new ERR_REQUIRE_ESM(filename, module, pkg); + const parentPath = module.parent && module.parent.filename; + const packageJsonPath = path.resolve(pkg.path, 'package.json'); + const basename = path.basename(filename) === path.basename(parentPath) ? + filename : path.basename(filename); + throw new ERR_REQUIRE_ESM(filename, parentPath, packageJsonPath, basename); } } const content = fs.readFileSync(filename, 'utf8'); From 46b04d53b3e31fbcdb1900f70a6f8324cd78ba75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Jos=C3=A9=20Arboleda?= Date: Tue, 3 Dec 2019 19:23:40 -0500 Subject: [PATCH 6/7] fixup --- lib/internal/errors.js | 7 +++++-- lib/internal/modules/cjs/loader.js | 4 +--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 900a1ee8274037..bdd73a93667992 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1123,7 +1123,10 @@ E('ERR_OUT_OF_RANGE', return msg; }, RangeError); E('ERR_REQUIRE_ESM', - (filename, parentPath = null, packageJsonPath = null, basename = null) => { + (filename, parentPath = null, packageJsonPath = null) => { + const path = require('path'); + const basename = path.basename(filename) === path.basename(parentPath) ? + filename : path.basename(filename); let msg = `Must use import to load ES Module: ${filename}\n`; if (parentPath && packageJsonPath) { msg += @@ -1138,7 +1141,7 @@ E('ERR_REQUIRE_ESM', return msg; } return msg; -}, Error); + }, Error); E('ERR_SCRIPT_EXECUTION_INTERRUPTED', 'Script execution was interrupted by `SIGINT`', Error); E('ERR_SERVER_ALREADY_LISTEN', diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 168b2a860978a0..1b37047ca048cd 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -1198,9 +1198,7 @@ Module._extensions['.js'] = function(module, filename) { if (pkg && pkg.data && pkg.data.type === 'module') { const parentPath = module.parent && module.parent.filename; const packageJsonPath = path.resolve(pkg.path, 'package.json'); - const basename = path.basename(filename) === path.basename(parentPath) ? - filename : path.basename(filename); - throw new ERR_REQUIRE_ESM(filename, parentPath, packageJsonPath, basename); + throw new ERR_REQUIRE_ESM(filename, parentPath, packageJsonPath); } } const content = fs.readFileSync(filename, 'utf8'); From 551c91e1479c119fc1dac0d28235d864f394a6bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Jos=C3=A9=20Arboleda?= Date: Tue, 3 Dec 2019 23:46:25 -0500 Subject: [PATCH 7/7] fixup --- lib/internal/errors.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index bdd73a93667992..91e24778733a09 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1124,13 +1124,13 @@ E('ERR_OUT_OF_RANGE', }, RangeError); E('ERR_REQUIRE_ESM', (filename, parentPath = null, packageJsonPath = null) => { - const path = require('path'); - const basename = path.basename(filename) === path.basename(parentPath) ? - filename : path.basename(filename); - let msg = `Must use import to load ES Module: ${filename}\n`; + let msg = `Must use import to load ES Module: ${filename}`; if (parentPath && packageJsonPath) { + const path = require('path'); + const basename = path.basename(filename) === path.basename(parentPath) ? + filename : path.basename(filename); msg += - 'require() of ES modules is not supported.\nrequire() of ' + + '\nrequire() of ES modules is not supported.\nrequire() of ' + `${filename} ${parentPath ? `from ${parentPath} ` : ''}` + 'is an ES module file as it is a .js file whose nearest parent ' + 'package.json contains "type": "module" which defines all .js ' +