Skip to content

Commit

Permalink
esm: fallback to getSource when load returns nullish source
Browse files Browse the repository at this point in the history
When using the Modules Customization Hooks API to load CommonJS modules,
we want to support the returned value of `defaultLoad` which must be
nullish to preserve backward compatibility. This can be achieved by
fetching the source from the translator.

PR-URL: #50825
Fixes: #50435
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
  • Loading branch information
aduh95 authored and RafaelGSS committed Nov 29, 2023
1 parent bd58870 commit 50f336c
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 12 deletions.
1 change: 1 addition & 0 deletions lib/internal/modules/esm/load.js
Expand Up @@ -262,5 +262,6 @@ function throwUnknownModuleFormat(url, format) {
module.exports = {
defaultLoad,
defaultLoadSync,
getSourceSync,
throwUnknownModuleFormat,
};
35 changes: 23 additions & 12 deletions lib/internal/modules/esm/translators.js
Expand Up @@ -31,6 +31,7 @@ function lazyTypes() {
}

const { containsModuleSyntax } = internalBinding('contextify');
const { BuiltinModule } = require('internal/bootstrap/realm');
const assert = require('internal/assert');
const { readFileSync } = require('fs');
const { dirname, extname, isAbsolute } = require('path');
Expand Down Expand Up @@ -58,6 +59,17 @@ const asyncESM = require('internal/process/esm_loader');
const { emitWarningSync } = require('internal/process/warning');
const { internalCompileFunction } = require('internal/vm');

// Lazy-loading to avoid circular dependencies.
let getSourceSync;
/**
* @param {Parameters<typeof import('./load').getSourceSync>[0]} url
* @returns {ReturnType<typeof import('./load').getSourceSync>}
*/
function getSource(url) {
getSourceSync ??= require('internal/modules/esm/load').getSourceSync;
return getSourceSync(url);
}

/** @type {import('deps/cjs-module-lexer/lexer.js').parse} */
let cjsParse;
/**
Expand Down Expand Up @@ -225,21 +237,19 @@ function loadCJSModule(module, source, url, filename) {
// eslint-disable-next-line func-name-matching,func-style
const requireFn = function require(specifier) {
let importAttributes = kEmptyObject;
if (!StringPrototypeStartsWith(specifier, 'node:')) {
if (!StringPrototypeStartsWith(specifier, 'node:') && !BuiltinModule.normalizeRequirableId(specifier)) {
// TODO: do not depend on the monkey-patchable CJS loader here.
const path = CJSModule._resolveFilename(specifier, module);
if (specifier !== path) {
switch (extname(path)) {
case '.json':
importAttributes = { __proto__: null, type: 'json' };
break;
case '.node':
return CJSModule._load(specifier, module);
default:
switch (extname(path)) {
case '.json':
importAttributes = { __proto__: null, type: 'json' };
break;
case '.node':
return CJSModule._load(specifier, module);
default:
// fall through
}
specifier = `${pathToFileURL(path)}`;
}
specifier = `${pathToFileURL(path)}`;
}
const job = asyncESM.esmLoader.getModuleJobSync(specifier, url, importAttributes);
job.runSync();
Expand Down Expand Up @@ -276,7 +286,8 @@ function createCJSModuleWrap(url, source, isMain, loadCJS = loadCJSModule) {
debug(`Translating CJSModule ${url}`);

const filename = StringPrototypeStartsWith(url, 'file://') ? fileURLToPath(url) : url;
source = stringify(source);
// In case the source was not provided by the `load` step, we need fetch it now.
source = stringify(source ?? getSource(new URL(url)).source);

const { exportNames, module } = cjsPreparseModuleExports(filename, source);
cjsCache.set(url, module);
Expand Down
35 changes: 35 additions & 0 deletions test/es-module/test-esm-loader-hooks.mjs
Expand Up @@ -746,4 +746,39 @@ describe('Loader hooks', { concurrency: true }, () => {
assert.strictEqual(code, 0);
assert.strictEqual(signal, null);
});

it('should handle mixed of opt-in modules and non-opt-in ones', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
'--experimental-loader',
`data:text/javascript,const fixtures=${JSON.stringify(fixtures.path('empty.js'))};export ${
encodeURIComponent(function resolve(s, c, n) {
if (s.endsWith('entry-point')) {
return {
shortCircuit: true,
url: 'file:///c:/virtual-entry-point',
format: 'commonjs',
};
}
return n(s, c);
})
}export ${
encodeURIComponent(async function load(u, c, n) {
if (u === 'file:///c:/virtual-entry-point') {
return {
shortCircuit: true,
source: `"use strict";require(${JSON.stringify(fixtures)});console.log("Hello");`,
format: 'commonjs',
};
}
return n(u, c);
})}`,
'entry-point',
]);

assert.strictEqual(stderr, '');
assert.strictEqual(stdout, 'Hello\n');
assert.strictEqual(code, 0);
assert.strictEqual(signal, null);
});
});

0 comments on commit 50f336c

Please sign in to comment.