loader, docs, test: named exports from commonjs modules

doc/api/esm.md

@@ -83,12 +83,25 @@ All CommonJS, JSON, and C++ modules can be used with `import`.
 Modules loaded this way will only be loaded once, even if their query
 or fragment string differs between `import` statements.
 
-When loaded via `import` these modules will provide a single `default` export
-representing the value of `module.exports` at the time they finished evaluating.
+JSON and C++ addon modules will provide a single `default` export representing
+the value of `module.exports` at the time they finish evaluating.
+
+CommonJS modules, when imported, will be handled in one of two ways. By default
+they will provide a single `default` export representing the value of
+`module.exports` at the time they finish evaluating.
+CJS modules may also provide a boolean `@@esModuleInterop` or `__esModule`
+export indicating that the enumerable keys of `module.exports` should be used
+as named exports.
+In both cases, this should be thought of like a "snapshot" of the exports at
+the time of importing; asynchronously modifying `module.exports` will not
+affect the values of the exports. Builtin libraries are provided with named
+exports as if they were using `@@esModuleInterop`, as well as the
+`module.exports` of the builtin provided as the `default` export.
+
 
 ```js
-import fs from 'fs';
-fs.readFile('./foo.txt', (err, body) => {
+import { readFile } from 'fs';
+readFile('./foo.txt', (err, body) => {
  if (err) {
  console.error(err);
  } else {
@@ -97,6 +110,15 @@ fs.readFile('./foo.txt', (err, body) => {
 });
 ```
 
+```js
+// main.mjs
+import { part } from './other.js';
+
+// other.js
+exports.part = () => {};
+exports[Symbol.for('esModuleInterop')] = true;
+```
+
 ## Loader hooks
 
 <!-- type=misc -->

lib/internal/loader/ModuleRequest.js

@@ -19,6 +19,8 @@ const search = require('internal/loader/search');
 const asyncReadFile = require('util').promisify(require('fs').readFile);
 const debug = require('util').debuglog('esm');
 
+const esModuleInterop = Symbol.for('esModuleInterop');
+
 const realpathCache = new Map();
 
 const loaders = new Map();
@@ -36,24 +38,39 @@ loaders.set('esm', async (url) => {
 
 // Strategy for loading a node-style CommonJS module
 loaders.set('cjs', async (url) => {
- return createDynamicModule(['default'], url, (reflect) => {
- debug(`Loading CJSModule ${url}`);
- const CJSModule = require('module');
- const pathname = internalURLModule.getPathFromURL(new URL(url));
- CJSModule._load(pathname);
+ debug(`Loading CJSModule ${url}`);
+ const CJSModule = require('module');
+ const pathname = internalURLModule.getPathFromURL(new URL(url));
+ const exports = CJSModule._load(pathname);
+ const es = exports[esModuleInterop] !== undefined ?
+ exports[esModuleInterop] : exports.__esModule;
+ const keys = es ? Object.keys(exports) : ['default'];
+ return createDynamicModule(keys, url, (reflect) => {
+ if (es) {
+ for (var i = 0; i < keys.length; i++)
+ reflect.exports[keys[i]].set(exports[keys[i]]);
+ } else {
+ reflect.exports.default.set(exports);
+ }
  });
 });
 
 // Strategy for loading a node builtin CommonJS module that isn't
 // through normal resolution
 loaders.set('builtin', async (url) => {
- return createDynamicModule(['default'], url, (reflect) => {
- debug(`Loading BuiltinModule ${url}`);
- const exports = NativeModule.require(url.substr(5));
+ debug(`Loading BuiltinModule ${url}`);
+ const exports = NativeModule.require(url.substr(5));
+ const keys = Object.keys(exports);
+ return createDynamicModule(['default', ...keys], url, (reflect) => {
  reflect.exports.default.set(exports);
+ for (var i = 0; i < keys.length; i++)
+ reflect.exports[keys[i]].set(exports[keys[i]]);
  });
 });
 
+// Strategy for loading a native addon module
+// Named exports will not be parsed from these - see
+// https://github.com/nodejs/abi-stable-node/issues/256#issuecomment-325138872
 loaders.set('addon', async (url) => {
  const ctx = createDynamicModule(['default'], url, (reflect) => {
  debug(`Loading NativeModule ${url}`);

test/es-module/es-module.status

@@ -5,3 +5,5 @@ prefix es-module
 # sample-test : PASS,FLAKY
 
 [true] # This section applies to all platforms
+
+test-esm-esmoduleinterop-override : FAIL

test/es-module/test-esm-cjs-esmodule.mjs

@@ -0,0 +1,9 @@
+// Flags: --experimental-modules
+/* eslint-disable required-modules */
+
+import assert from 'assert';
+import eightyfour, { fourtytwo } from
+ '../fixtures/es-module-loaders/babel-to-esm.js';
+
+assert.strictEqual(eightyfour, 84);
+assert.strictEqual(fourtytwo, 42);

test/es-module/test-esm-esmoduleinterop-override.mjs

@@ -0,0 +1,6 @@
+// Flags: --experimental-modules
+/* eslint-disable required-modules */
+
+// eslint-disable-next-line no-unused-vars
+import eightyfour, { fourtytwo }
+ from '../fixtures/es-module-loaders/babel-to-esm-override.js';

test/es-module/test-esm-namespace.mjs

@@ -1,7 +1,14 @@
 // Flags: --experimental-modules
 /* eslint-disable required-modules */
 
-import * as fs from 'fs';
 import assert from 'assert';
+import fs, { readFile } from 'fs';
+import main, { named } from
+ '../fixtures/es-module-loaders/cjs-to-es-namespace.js';
 
-assert.deepStrictEqual(Object.keys(fs), ['default']);
+assert(fs);
+assert(fs.readFile);
+assert.strictEqual(fs.readFile, readFile);
+
+assert.strictEqual(main, 'default');
+assert.strictEqual(named, 'named');

test/es-module/test-reserved-keywords.mjs

@@ -0,0 +1,8 @@
+// Flags: --experimental-modules
+/* eslint-disable required-modules */
+
+import assert from 'assert';
+import { enum as e } from
+ '../fixtures/es-module-loaders/reserved-keywords.js';
+
+assert(e);

test/fixtures/es-module-loaders/babel-to-esm-override.js

@@ -0,0 +1,18 @@
+"use strict";
+
+/*
+created by babel with es2015 preset
+```
+export const fourtytwo = 42;
+export default 84;
+```
+*/
+
+Object.defineProperty(exports, "__esModule", {
+ value: true
+});
+var fourtytwo = exports.fourtytwo = 42;
+exports.default = 84;
+
+// added after babel compile
+exports[Symbol.for('esModuleInterop')] = false

test/fixtures/es-module-loaders/babel-to-esm.js

@@ -0,0 +1,15 @@
+"use strict";
+
+/*
+created by babel with es2015 preset
+```
+export const fourtytwo = 42;
+export default 84;
+```
+*/
+
+Object.defineProperty(exports, "__esModule", {
+ value: true
+});
+var fourtytwo = exports.fourtytwo = 42;
+exports.default = 84;

test/fixtures/es-module-loaders/cjs-to-es-namespace.js

@@ -0,0 +1,4 @@
+exports.named = 'named';
+exports.default = 'default';
+
+exports[Symbol.for('esModuleInterop')] = true;

test/fixtures/es-module-loaders/reserved-keywords.js

@@ -0,0 +1,6 @@
+module.exports = {
+ enum: 'enum',
+ class: 'class',
+ delete: 'delete',
+ [Symbol.for('esModuleInterop')]: true,
+};