From 0dfc59e4fcf5be818530cc79d73ee444e3b539c5 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Fri, 10 Nov 2023 10:20:46 +0200 Subject: [PATCH] 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. PR-URL: https://github.com/nodejs/node/pull/50004 Reviewed-By: Geoffrey Booth --- doc/api/module.md | 3 +- lib/internal/modules/esm/load.js | 4 +- test/es-module/test-esm-type-flag-errors.mjs | 126 ++++++++++++++---- .../fixtures/es-module-require-cache/echo.cjs | 1 + .../echo-require-cache.js | 1 + 5 files changed, 105 insertions(+), 30 deletions(-) create mode 100644 test/fixtures/es-module-require-cache/echo.cjs create mode 100644 test/fixtures/es-modules/package-type-commonjs/echo-require-cache.js diff --git a/doc/api/module.md b/doc/api/module.md index f4338028abe31d..394af95ac00b41 100644 --- a/doc/api/module.md +++ b/doc/api/module.md @@ -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: diff --git a/lib/internal/modules/esm/load.js b/lib/internal/modules/esm/load.js index 6f9b73abd8a761..bcebc283828ad5 100644 --- a/lib/internal/modules/esm/load.js +++ b/lib/internal/modules/esm/load.js @@ -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'); @@ -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; diff --git a/test/es-module/test-esm-type-flag-errors.mjs b/test/es-module/test-esm-type-flag-errors.mjs index 6d54eff94763ef..8fd06c7d562bb0 100644 --- a/test/es-module/test-esm-type-flag-errors.mjs +++ b/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); + }); +}); diff --git a/test/fixtures/es-module-require-cache/echo.cjs b/test/fixtures/es-module-require-cache/echo.cjs new file mode 100644 index 00000000000000..c00af019304dbe --- /dev/null +++ b/test/fixtures/es-module-require-cache/echo.cjs @@ -0,0 +1 @@ +console.log(require.cache); diff --git a/test/fixtures/es-modules/package-type-commonjs/echo-require-cache.js b/test/fixtures/es-modules/package-type-commonjs/echo-require-cache.js new file mode 100644 index 00000000000000..c00af019304dbe --- /dev/null +++ b/test/fixtures/es-modules/package-type-commonjs/echo-require-cache.js @@ -0,0 +1 @@ +console.log(require.cache);