Skip to content

Commit

Permalink
esm: fallback to readFileSync when source is nullish
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.
  • Loading branch information
aduh95 committed Nov 20, 2023
1 parent a9a4778 commit 594830d
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 1 deletion.
1 change: 1 addition & 0 deletions lib/internal/modules/esm/load.js
Original file line number Diff line number Diff line change
Expand Up @@ -261,5 +261,6 @@ function throwUnknownModuleFormat(url, format) {
module.exports = {
defaultLoad,
defaultLoadSync,
getSourceSync,
throwUnknownModuleFormat,
};
9 changes: 8 additions & 1 deletion lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ 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;
function getSource(url) {
getSourceSync ??= require('internal/modules/esm/load').getSourceSync;
return getSourceSync(url).source;
}

/** @type {import('deps/cjs-module-lexer/lexer.js').parse} */
let cjsParse;
/**
Expand Down Expand Up @@ -276,7 +283,7 @@ function createCJSModuleWrap(url, source, isMain, loadCJS = loadCJSModule) {
debug(`Translating CJSModule ${url}`);

const filename = StringPrototypeStartsWith(url, 'file://') ? fileURLToPath(url) : url;
source = stringify(source);
source = stringify(source ?? getSource(new URL(url)));

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
Original file line number Diff line number Diff line change
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:///virtual-entry-point',
format: 'commonjs',
};
}
return n(s, c);
})
}export ${
encodeURIComponent(async function load(u, c, n) {
if (u === 'file:///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 594830d

Please sign in to comment.