Skip to content
/ node Public
forked from nodejs/node

Commit 81fef20

Browse files
committed
loader,docs,test: set named exports based on keys from module.exports
I know a lot of discussion went into the original decision on how mjs would handle cjs modules but after using the system for a while, and talking with a lot of other people in the community, it just seems like the expected behavior and the wanted behavior is to export named based on the keys. This PR implements that in what is hopefully a performant enough solution, although that shouldn't be too much of a problem since this code only runs during initial module loading. This implementation remains safe with regard to named exports that are also reserved keywords such as `class` or `delete`. Refs: nodejs/node-eps#57
1 parent d1a9c02 commit 81fef20

File tree

3 files changed

+32
-17
lines changed

3 files changed

+32
-17
lines changed

doc/api/esm.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,9 @@ All CommonJS, JSON, and C++ modules can be used with `import`.
8383
Modules loaded this way will only be loaded once, even if their query
8484
or fragment string differs between `import` statements.
8585

86-
When loaded via `import` these modules will provide a single `default` export
87-
representing the value of `module.exports` at the time they finished evaluating.
86+
When loaded via `import` these modules will provide a `default` export
87+
representing the value of `module.exports` at the time they finished evaluating,
88+
and named exports for each key of `module.exports`.
8889

8990
```js
9091
import fs from 'fs';
@@ -97,6 +98,11 @@ fs.readFile('./foo.txt', (err, body) => {
9798
});
9899
```
99100

101+
```js
102+
import { readFileSync } from 'fs';
103+
console.log(readFileSync('./foo.txt').toString());
104+
```
105+
100106
## Loader hooks
101107

102108
<!-- type=misc -->

lib/internal/loader/ModuleRequest.js

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,31 +36,38 @@ loaders.set('esm', async (url) => {
3636

3737
// Strategy for loading a node-style CommonJS module
3838
loaders.set('cjs', async (url) => {
39-
return createDynamicModule(['default'], url, (reflect) => {
40-
debug(`Loading CJSModule ${url}`);
41-
const CJSModule = require('module');
42-
const pathname = internalURLModule.getPathFromURL(new URL(url));
43-
CJSModule._load(pathname);
39+
debug(`Loading CJSModule ${url}`);
40+
const CJSModule = require('module');
41+
const pathname = internalURLModule.getPathFromURL(new URL(url));
42+
const exports = CJSModule._load(pathname);
43+
const keys = Object.keys(exports);
44+
return createDynamicModule(['default', ...keys], url, (reflect) => {
45+
reflect.exports.default.set(exports);
46+
for (const key of keys) reflect.exports[key].set(exports[key]);
4447
});
4548
});
4649

4750
// Strategy for loading a node builtin CommonJS module that isn't
4851
// through normal resolution
4952
loaders.set('builtin', async (url) => {
50-
return createDynamicModule(['default'], url, (reflect) => {
51-
debug(`Loading BuiltinModule ${url}`);
52-
const exports = NativeModule.require(url.substr(5));
53+
debug(`Loading BuiltinModule ${url}`);
54+
const exports = NativeModule.require(url.substr(5));
55+
const keys = Object.keys(exports);
56+
return createDynamicModule(['default', ...keys], url, (reflect) => {
5357
reflect.exports.default.set(exports);
58+
for (const key of keys) reflect.exports[key].set(exports[key]);
5459
});
5560
});
5661

5762
loaders.set('addon', async (url) => {
58-
const ctx = createDynamicModule(['default'], url, (reflect) => {
59-
debug(`Loading NativeModule ${url}`);
60-
const module = { exports: {} };
61-
const pathname = internalURLModule.getPathFromURL(new URL(url));
62-
process.dlopen(module, _makeLong(pathname));
63+
debug(`Loading NativeModule ${url}`);
64+
const module = { exports: {} };
65+
const pathname = internalURLModule.getPathFromURL(new URL(url));
66+
process.dlopen(module, _makeLong(pathname));
67+
const keys = Object.keys(module.exports);
68+
const ctx = createDynamicModule(['default', ...keys], url, (reflect) => {
6369
reflect.exports.default.set(module.exports);
70+
for (const key of keys) reflect.exports[key].set(module.exports[key]);
6471
});
6572
return ctx;
6673
});

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
// Flags: --experimental-modules
22
/* eslint-disable required-modules */
33

4-
import * as fs from 'fs';
54
import assert from 'assert';
5+
import fs, { readFile } from 'fs';
66

7-
assert.deepStrictEqual(Object.keys(fs), ['default']);
7+
assert(fs);
8+
assert(fs.readFile);
9+
assert(readFile);

0 commit comments

Comments
 (0)