Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

esm: bypass CJS loader in default load under --default-type=module #50004

Merged
merged 4 commits into from
Nov 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
3 changes: 2 additions & 1 deletion doc/api/module.md
Original file line number Diff line number Diff line change
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
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 (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
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);