module: fix for #17130 shared loader cjs dep

PR-URL: #17131 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Bradley Farias <[email protected]>
nodejs · Dec 12, 2017 · 5736dc4 · 5736dc4
1 parent 6624ac3
commit 5736dc4
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 3 deletions.
diff --git a/lib/internal/loader/ModuleRequest.js b/lib/internal/loader/ModuleRequest.js
@@ -2,6 +2,7 @@
 
 const fs = require('fs');
 const internalCJSModule = require('internal/module');
+const CJSModule = require('module');
 const internalURLModule = require('internal/url');
 const internalFS = require('internal/fs');
 const NativeModule = require('native_module');
@@ -35,11 +36,19 @@ loaders.set('esm', async (url) => {
 });
 
 // Strategy for loading a node-style CommonJS module
+const isWindows = process.platform === 'win32';
+const winSepRegEx = /\//g;
 loaders.set('cjs', async (url) => {
+  const pathname = internalURLModule.getPathFromURL(new URL(url));
+  const module = CJSModule._cache[
+      isWindows ? pathname.replace(winSepRegEx, '\\') : pathname];
+  if (module && module.loaded) {
+    const ctx = createDynamicModule(['default'], url, undefined);
+    ctx.reflect.exports.default.set(module.exports);
+    return ctx;
+  }
   return createDynamicModule(['default'], url, (reflect) => {
     debug(`Loading CJSModule ${url}`);
-    const CJSModule = require('module');
-    const pathname = internalURLModule.getPathFromURL(new URL(url));
     CJSModule._load(pathname);
   });
 });

diff --git a/lib/module.js b/lib/module.js
@@ -39,6 +39,9 @@ const experimentalModules = !!process.binding('config').experimentalModules;
 
 const errors = require('internal/errors');
 
+module.exports = Module;
+
+// these are below module.exports for the circular reference
 const Loader = require('internal/loader/Loader');
 const ModuleJob = require('internal/loader/ModuleJob');
 const { createDynamicModule } = require('internal/loader/ModuleWrap');
@@ -72,7 +75,6 @@ function Module(id, parent) {
   this.loaded = false;
   this.children = [];
 }
-module.exports = Module;
 
 const builtinModules = Object.keys(NativeModule._source)
   .filter(NativeModule.nonInternalExists);

diff --git a/test/es-module/test-esm-shared-loader-dep.mjs b/test/es-module/test-esm-shared-loader-dep.mjs
@@ -0,0 +1,7 @@
+// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-shared-dep.mjs
+/* eslint-disable required-modules */
+import assert from 'assert';
+import './test-esm-ok.mjs';
+import dep from '../fixtures/es-module-loaders/loader-dep.js';
+
+assert.strictEqual(dep.format, 'esm');
diff --git a/test/fixtures/es-module-loaders/loader-shared-dep.mjs b/test/fixtures/es-module-loaders/loader-shared-dep.mjs
@@ -0,0 +1,7 @@
+import dep from './loader-dep.js';
+import assert from 'assert';
+
+export function resolve(specifier, base, defaultResolve) {
+  assert.equal(dep.format, 'esm');
+  return defaultResolve(specifier, base);
+}