Skip to content

Commit

Permalink
module: warn on require of .js inside type: module
Browse files Browse the repository at this point in the history
PR-URL: #29909
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
guybedford authored and targos committed Nov 10, 2019
1 parent 1fefd7f commit 2695f82
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 5 deletions.
32 changes: 27 additions & 5 deletions lib/internal/modules/cjs/loader.js
Expand Up @@ -261,7 +261,10 @@ function readPackageScope(checkPath) {
if (checkPath.endsWith(path.sep + 'node_modules'))
return false;
const pjson = readPackage(checkPath);
if (pjson) return pjson;
if (pjson) return {
path: checkPath,
data: pjson
};
}
return false;
}
Expand Down Expand Up @@ -960,13 +963,32 @@ Module.prototype._compile = function(content, filename) {
return result;
};


// Native extension for .js
let warnRequireESM = true;
Module._extensions['.js'] = function(module, filename) {
if (experimentalModules && filename.endsWith('.js')) {
if (filename.endsWith('.js')) {
const pkg = readPackageScope(filename);
if (pkg && pkg.type === 'module') {
throw new ERR_REQUIRE_ESM(filename);
if (pkg && pkg.data && pkg.data.type === 'module') {
if (warnRequireESM) {
const parentPath = module.parent && module.parent.filename;
const basename = parentPath &&
path.basename(filename) === path.basename(parentPath) ?
filename : path.basename(filename);
process.emitWarning(
'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')}.`
);
warnRequireESM = false;
}
if (experimentalModules) {
throw new ERR_REQUIRE_ESM(filename);
}
}
}
const content = fs.readFileSync(filename, 'utf8');
Expand Down
38 changes: 38 additions & 0 deletions test/es-module/test-cjs-esm-warn.js
@@ -0,0 +1,38 @@
'use strict';

const common = require('../common');
const fixtures = require('../common/fixtures');
const { spawn } = require('child_process');
const assert = require('assert');
const path = require('path');

const requiring = path.resolve(fixtures.path('/es-modules/cjs-esm.js'));
const required = path.resolve(
fixtures.path('/es-modules/package-type-module/cjs.js')
);
const pjson = path.resolve(
fixtures.path('/es-modules/package-type-module/package.json')
);

const basename = 'cjs.js';

const child = spawn(process.execPath, [requiring]);
let stderr = '';
child.stderr.setEncoding('utf8');
child.stderr.on('data', (data) => {
stderr += data;
});
child.on('close', common.mustCall((code, signal) => {
assert.strictEqual(code, 0);
assert.strictEqual(signal, null);

assert.strictEqual(stderr, `(node:${child.pid}) Warning: ` +
'require() 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`);
}));
1 change: 1 addition & 0 deletions test/fixtures/es-modules/cjs-esm.js
@@ -0,0 +1 @@
require('./package-type-module/cjs.js');
1 change: 1 addition & 0 deletions test/fixtures/es-modules/package-type-module/cjs.js
@@ -0,0 +1 @@
module.exports = 'asdf';

0 comments on commit 2695f82

Please sign in to comment.