Skip to content

Commit

Permalink
esm: bypass CJS loader in default load under --default-type=module
Browse files Browse the repository at this point in the history
This allows user to opt-out from using the monkey-patchable CJS loader,
even to load CJS modules.

PR-URL: #50004
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
  • Loading branch information
aduh95 authored and UlisesGascon committed Dec 11, 2023
1 parent 4cf10a1 commit b48cf31
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 30 deletions.
3 changes: 2 additions & 1 deletion doc/api/module.md
Expand Up @@ -622,7 +622,8 @@ Omitting vs providing a `source` for `'commonjs'` has very different effects:
registered hooks. This behavior for nullish `source` is temporary — in the
future, nullish `source` will not be supported.
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
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
@@ -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 (imported, required, entry points)', async () => {
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 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, /^undefined\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
@@ -0,0 +1 @@
console.log(require.cache);
@@ -0,0 +1 @@
console.log(require.cache);

0 comments on commit b48cf31

Please sign in to comment.