module: allow unrestricted cjs requires

nodejs · Oct 6, 2019 · 1e71621 · 1e71621
1 parent ffd22e8
commit 1e71621
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 22 deletions.
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
@@ -816,20 +816,32 @@ Module.prototype.load = function(filename) {
     const url = `${pathToFileURL(filename)}`;
     const module = ESMLoader.moduleMap.get(url);
     // Create module entry at load time to snapshot exports correctly
-    const exports = this.exports;
-    // Called from cjs translator
-    if (module !== undefined && module.module !== undefined) {
-      if (module.module.getStatus() >= kInstantiated)
-        module.module.setExport('default', exports);
-    } else { // preemptively cache
-      ESMLoader.moduleMap.set(
-        url,
-        new ModuleJob(ESMLoader, url, () =>
-          new ModuleWrap(function() {
-            this.setExport('default', exports);
-          }, ['default'], url)
-        )
-      );
+    const ext = path.extname(filename);
+    // Only inject modules that are valid in the ES module system
+    let injectIntoESM = ext === '.js' || ext === '.cjs' || ext === '.json' ||
+        ext === '.node';
+    if (ext === '.js') {
+      const pkg = readPackageScope(filename);
+      // Do not inject CJS modules that break module type correctness
+      if (pkg && pkg.type === 'module')
+        injectIntoESM = false;
+    }
+    if (injectIntoESM) {
+      const exports = this.exports;
+      // Called from cjs translator
+      if (module !== undefined && module.module !== undefined) {
+        if (module.module.getStatus() >= kInstantiated)
+          module.module.setExport('default', exports);
+      } else { // preemptively cache
+        ESMLoader.moduleMap.set(
+          url,
+          new ModuleJob(ESMLoader, url, () =>
+            new ModuleWrap(function() {
+              this.setExport('default', exports);
+            }, ['default'], url)
+          )
+        );
+      }
     }
   }
 };
@@ -963,12 +975,6 @@ Module.prototype._compile = function(content, filename) {
 
 // Native extension for .js
 Module._extensions['.js'] = function(module, filename) {
-  if (experimentalModules && filename.endsWith('.js')) {
-    const pkg = readPackageScope(filename);
-    if (pkg && pkg.type === 'module') {
-      throw new ERR_REQUIRE_ESM(filename);
-    }
-  }
   const content = fs.readFileSync(filename, 'utf8');
   module._compile(content, filename);
 };

diff --git a/test/es-module/test-esm-type-flag-errors.js b/test/es-module/test-esm-type-flag-errors.js
@@ -26,9 +26,9 @@ expect('--input-type=module', packageTypeModuleMain,
 
 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:/));
+  // Verify CommonJS load is attempted but fails on ES module syntax
+  assert(e instanceof SyntaxError);
 }
 
 function expect(opt = '', inputFile, want, wantsError = false) {