Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
esm: bypass CJS loader in default load under --default-type=module
This allows user to opt-out from using the monkey-patchable CJS loader,
even to load CJS modules.
  • Loading branch information
aduh95 committed Oct 23, 2023
commit f2000b134e1e7dea610f4424ba8a0f10c3291219
22 changes: 12 additions & 10 deletions doc/api/module.md
Original file line number Diff line number Diff line change
Expand Up @@ -610,19 +610,21 @@ not possible to replace the value of a Node.js builtin (core) module.

Omitting vs providing a `source` for `'commonjs'` has very different effects:

* When a `source` is provided, all `require` calls from this module will be
processed by the ESM loader with registered `resolve` and `load` hooks; all
`require.resolve` calls from this module will be processed by the ESM loader
with registered `resolve` hooks; only a subset of the CommonJS API will be
available (e.g. no `require.extensions`, no `require.cache`, no
* When a `source` is provided, or when running `node` with
`--experimental-default-type=module`, all `require` calls from this module
will be processed by the ESM loader with registered `resolve` and `load`
hooks; all `require.resolve` calls from this module will be processed by the
ESM loader with registered `resolve` hooks; only a subset of the CommonJS API
will be available (e.g. no `require.extensions`, no `require.cache`, no
`require.resolve.paths`) and monkey-patching on the CommonJS module loader
will not apply.
* If `source` is undefined or `null`, it will be handled by the CommonJS module
loader and `require`/`require.resolve` calls will not go through the
registered hooks. This behavior for nullish `source` is temporary — in the
future, nullish `source` will not be supported.
* If `source` is undefined or `null`, and `node` is run with
`--experimental-default-type=commonjs`, it will be handled by the CommonJS
module loader and `require`/`require.resolve` calls will not go through the
registered hooks.

The Node.js internal `load` implementation, which is the value of `next` for the
When `node` is run with `--experimental-default-type=commonjs`, the Node.js
internal `load` implementation, which is the value of `next` for the
last hook in the `load` chain, returns `null` for `source` when `format` is
`'commonjs'` for backward compatibility. Here is an example hook that would
opt-in to using the non-default behavior:
Expand Down
4 changes: 3 additions & 1 deletion lib/internal/modules/esm/load.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ const policy = getOptionValue('--experimental-policy') ?
null;
const experimentalNetworkImports =
getOptionValue('--experimental-network-imports');
const defaultType =
getOptionValue('--experimental-default-type');

const { Buffer: { from: BufferFrom } } = require('buffer');

Expand Down Expand Up @@ -140,7 +142,7 @@ async function defaultLoad(url, context = kEmptyObject) {
// Now that we have the source for the module, run `defaultGetFormat` again in case we detect ESM syntax.
format ??= await defaultGetFormat(urlInstance, contextToPass);

if (format === 'commonjs' && contextToPass !== context) {
if (format === 'commonjs' && contextToPass !== context && defaultType !== 'module') {
// For backward compatibility reasons, we need to discard the source in
// order for the CJS loader to re-fetch it.
source = null;
Expand Down
126 changes: 98 additions & 28 deletions test/es-module/test-esm-type-flag-errors.mjs
Original file line number Diff line number Diff line change
@@ -1,31 +1,101 @@
import { spawnPromisified } from '../common/index.mjs';
import * as fixtures from '../common/fixtures.mjs';
import { describe, it } from 'node:test';
import { match, strictEqual } from 'node:assert';

describe('--experimental-default-type=module should not affect the interpretation of files with unknown extensions',
{ concurrency: true }, () => {
it('should error on an entry point with an unknown extension', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
'--experimental-default-type=module',
fixtures.path('es-modules/package-type-module/extension.unknown'),
]);

match(stderr, /ERR_UNKNOWN_FILE_EXTENSION/);
strictEqual(stdout, '');
strictEqual(code, 1);
strictEqual(signal, null);
});

it('should error on an import with an unknown extension', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
'--experimental-default-type=module',
fixtures.path('es-modules/package-type-module/imports-unknownext.mjs'),
]);

match(stderr, /ERR_UNKNOWN_FILE_EXTENSION/);
strictEqual(stdout, '');
strictEqual(code, 1);
strictEqual(signal, null);
});
});
import { deepStrictEqual, match, strictEqual } from 'node:assert';

describe('--experimental-default-type=module', { concurrency: true }, () => {
describe('should not affect the interpretation of files with unknown extensions', { concurrency: true }, () => {
it('should error on an entry point with an unknown extension', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
'--experimental-default-type=module',
fixtures.path('es-modules/package-type-module/extension.unknown'),
]);

match(stderr, /ERR_UNKNOWN_FILE_EXTENSION/);
strictEqual(stdout, '');
strictEqual(code, 1);
strictEqual(signal, null);
});

it('should error on an import with an unknown extension', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
'--experimental-default-type=module',
fixtures.path('es-modules/package-type-module/imports-unknownext.mjs'),
]);

match(stderr, /ERR_UNKNOWN_FILE_EXTENSION/);
strictEqual(stdout, '');
strictEqual(code, 1);
strictEqual(signal, null);
});
});

it('should affect CJS .js files', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CJS .js files . . . that are required?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that are required, imported, or used as entry point

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth spelling that out, I was assuming “CJS .js files” meant .js files required by CommonJS modules, rather than what I now presume you meant, “.js file that is interpreted as CommonJS.”

const result = await spawnPromisified(process.execPath, [
'--experimental-default-type=module',
fixtures.path('es-modules/package-type-commonjs/echo-require-cache.js'),
]);

deepStrictEqual(result, {
code: 0,
stderr: '',
stdout: 'undefined\n',
signal: null,
});
});

it('should affect .cjs files that are imported', async () => {
const result = await spawnPromisified(process.execPath, [
'--experimental-default-type=module',
'-e',
`import ${JSON.stringify(fixtures.fileURL('es-module-require-cache/echo.cjs'))}`,
]);

deepStrictEqual(result, {
code: 0,
stderr: '',
stdout: 'undefined\n',
signal: null,
});
});

it('should not affect entry point .cjs files (with no hooks)', async () => {
const { stderr, stdout, code } = await spawnPromisified(process.execPath, [
'--experimental-default-type=module',
fixtures.path('es-module-require-cache/echo.cjs'),
]);

strictEqual(stderr, '');
match(stdout, /^\[Object: null prototype\] \{(\n .+)+\n\}\n$/);
strictEqual(code, 0);
});

it('should affect entry point .cjs files (when any hooks is registered)', async () => {
const result = await spawnPromisified(process.execPath, [
'--experimental-default-type=module',
'--import',
'data:text/javascript,import{register}from"node:module";register("data:text/javascript,");',
fixtures.path('es-module-require-cache/echo.cjs'),
]);

deepStrictEqual(result, {
code: 0,
stderr: '',
stdout: 'undefined\n',
signal: null,
});
});

it('should not affect CJS from input-type', async () => {
const { stderr, stdout, code } = await spawnPromisified(process.execPath, [
'--experimental-default-type=module',
'--input-type=commonjs',
'-p',
'require.cache',
]);

strictEqual(stderr, '');
match(stdout, /^\[Object: null prototype\] \{\}\n$/);
strictEqual(code, 0);
});
});
1 change: 1 addition & 0 deletions test/fixtures/es-module-require-cache/echo.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log(require.cache);
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log(require.cache);