Skip to content

Commit

Permalink
esm: leverage loaders when resolving subsequent loaders
Browse files Browse the repository at this point in the history
PR-URL: #43772
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
  • Loading branch information
arcanis authored and ruyadorno committed Jan 31, 2023
1 parent 8b473af commit 7aac21e
Show file tree
Hide file tree
Showing 15 changed files with 140 additions and 23 deletions.
5 changes: 3 additions & 2 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -701,8 +701,9 @@ changes:
To customize the default module resolution, loader hooks can optionally be
provided via a `--experimental-loader ./loader-name.mjs` argument to Node.js.
When hooks are used they apply to the entry point and all `import` calls. They
won't apply to `require` calls; those still follow [CommonJS][] rules.
When hooks are used they apply to each subsequent loader, the entry point, and
all `import` calls. They won't apply to `require` calls; those still follow
[CommonJS][] rules.
Loaders follow the pattern of `--require`:
Expand Down
4 changes: 1 addition & 3 deletions lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ class ESMLoader {

/**
* Collect custom/user-defined hook(s). After all hooks have been collected,
* calls global preload hook(s).
* the global preload hook(s) must be called.
* @param {KeyedExports} customLoaders
* A list of exports from user-defined loaders (as returned by
* ESMLoader.import()).
Expand Down Expand Up @@ -356,8 +356,6 @@ class ESMLoader {
);
}
}

this.preload();
}

async eval(
Expand Down
48 changes: 35 additions & 13 deletions lib/internal/process/esm_loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

const {
ArrayIsArray,
ArrayPrototypePushApply,
} = primordials;

const { ESMLoader } = require('internal/modules/esm/loader');
const {
hasUncaughtExceptionCaptureCallback,
} = require('internal/process/execution');
const { pathToFileURL } = require('internal/url');
const { kEmptyObject } = require('internal/util');

const esmLoader = new ESMLoader();
exports.esmLoader = esmLoader;
Expand All @@ -28,41 +30,61 @@ async function initializeLoader() {
const { getOptionValue } = require('internal/options');
const customLoaders = getOptionValue('--experimental-loader');
const preloadModules = getOptionValue('--import');
const loaders = await loadModulesInIsolation(customLoaders);

let cwd;
try {
cwd = process.cwd() + '/';
} catch {
cwd = '/';
}

const internalEsmLoader = new ESMLoader();
const allLoaders = [];

const parentURL = pathToFileURL(cwd).href;

for (let i = 0; i < customLoaders.length; i++) {
const customLoader = customLoaders[i];

// Importation must be handled by internal loader to avoid polluting user-land
const keyedExportsSublist = await internalEsmLoader.import(
[customLoader],
parentURL,
kEmptyObject,
);

internalEsmLoader.addCustomLoaders(keyedExportsSublist);
ArrayPrototypePushApply(allLoaders, keyedExportsSublist);
}

// Hooks must then be added to external/public loader
// (so they're triggered in userland)
esmLoader.addCustomLoaders(loaders);
esmLoader.addCustomLoaders(allLoaders);
esmLoader.preload();

// Preload after loaders are added so they can be used
if (preloadModules?.length) {
await loadModulesInIsolation(preloadModules, loaders);
await loadModulesInIsolation(parentURL, preloadModules, allLoaders);
}

isESMInitialized = true;
}

function loadModulesInIsolation(specifiers, loaders = []) {
function loadModulesInIsolation(parentURL, specifiers, loaders = []) {
if (!ArrayIsArray(specifiers) || specifiers.length === 0) { return; }

let cwd;
try {
cwd = process.cwd() + '/';
} catch {
cwd = 'file:///';
}

// A separate loader instance is necessary to avoid cross-contamination
// between internal Node.js and userland. For example, a module with internal
// state (such as a counter) should be independent.
const internalEsmLoader = new ESMLoader();
internalEsmLoader.addCustomLoaders(loaders);
internalEsmLoader.preload();

// Importation must be handled by internal loader to avoid poluting userland
return internalEsmLoader.import(
specifiers,
pathToFileURL(cwd).href,
{ __proto__: null },
parentURL,
kEmptyObject,
);
}

Expand Down
18 changes: 17 additions & 1 deletion test/es-module/test-esm-loader-chaining.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import assert from 'node:assert';
import { execPath } from 'node:process';
import { describe, it } from 'node:test';


const setupArgs = [
'--no-warnings',
'--input-type=module',
Expand Down Expand Up @@ -253,6 +252,23 @@ describe('ESM: loader chaining', { concurrency: true }, () => {
assert.strictEqual(code, 0);
});

it('should allow loaders to influence subsequent loader resolutions', async () => {
const { code, stderr } = await spawnPromisified(
execPath,
[
'--loader',
fixtures.fileURL('es-module-loaders', 'loader-resolve-strip-xxx.mjs'),
'--loader',
'xxx/loader-resolve-strip-yyy.mjs',
...commonArgs,
],
{ encoding: 'utf8', cwd: fixtures.path('es-module-loaders') },
);

assert.strictEqual(stderr, '');
assert.strictEqual(code, 0);
});

it('should throw when the resolve chain is broken', async () => {
const { code, stderr, stdout } = await spawnPromisified(
execPath,
Expand Down
10 changes: 9 additions & 1 deletion test/fixtures/es-module-loaders/loader-load-foo-or-42.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
export async function load(url) {
export async function load(url, context, next) {
// This check is needed to make sure that we don't prevent the
// resolution from follow-up loaders. It wouldn't be a problem
// in real life because loaders aren't supposed to break the
// resolution, but the ones used in our tests do, for convenience.
if (url.includes('loader')) {
return next(url);
}

const val = url.includes('42')
? '42'
: '"foo"';
Expand Down
10 changes: 9 additions & 1 deletion test/fixtures/es-module-loaders/loader-load-incomplete.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
export async function load() {
export async function load(url, context, next) {
// This check is needed to make sure that we don't prevent the
// resolution from follow-up loaders. It wouldn't be a problem
// in real life because loaders aren't supposed to break the
// resolution, but the ones used in our tests do, for convenience.
if (url.includes('loader')) {
return next(url);
}

return {
format: 'module',
source: 'export default 42',
Expand Down
8 changes: 8 additions & 0 deletions test/fixtures/es-module-loaders/loader-load-passthru.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
export async function load(url, context, next) {
// This check is needed to make sure that we don't prevent the
// resolution from follow-up loaders. It wouldn't be a problem
// in real life because loaders aren't supposed to break the
// resolution, but the ones used in our tests do, for convenience.
if (url.includes('loader')) {
return next(url);
}

console.log('load passthru'); // This log is deliberate
return next(url);
}
8 changes: 8 additions & 0 deletions test/fixtures/es-module-loaders/loader-resolve-42.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
export async function resolve(specifier, context, next) {
// This check is needed to make sure that we don't prevent the
// resolution from follow-up loaders. It wouldn't be a problem
// in real life because loaders aren't supposed to break the
// resolution, but the ones used in our tests do, for convenience.
if (specifier.includes('loader')) {
return next(specifier);
}

console.log('resolve 42'); // This log is deliberate
console.log('next<HookName>:', next.name); // This log is deliberate

Expand Down
8 changes: 8 additions & 0 deletions test/fixtures/es-module-loaders/loader-resolve-foo.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
export async function resolve(specifier, context, next) {
// This check is needed to make sure that we don't prevent the
// resolution from follow-up loaders. It wouldn't be a problem
// in real life because loaders aren't supposed to break the
// resolution, but the ones used in our tests do, for convenience.
if (specifier.includes('loader')) {
return next(specifier);
}

console.log('resolve foo'); // This log is deliberate
return next('file:///foo.mjs');
}
10 changes: 9 additions & 1 deletion test/fixtures/es-module-loaders/loader-resolve-incomplete.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
export async function resolve() {
export async function resolve(specifier, context, next) {
// This check is needed to make sure that we don't prevent the
// resolution from follow-up loaders. It wouldn't be a problem
// in real life because loaders aren't supposed to break the
// resolution, but the ones used in our tests do, for convenience.
if (specifier.includes('loader')) {
return next(specifier);
}

return {
url: 'file:///incomplete-resolve-chain.js',
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
export async function resolve(url, context, next) {
// This check is needed to make sure that we don't prevent the
// resolution from follow-up loaders. It wouldn't be a problem
// in real life because loaders aren't supposed to break the
// resolution, but the ones used in our tests do, for convenience.
if (url.includes('loader')) {
return next(url);
}

const {
format,
url: nextUrl,
Expand Down
8 changes: 8 additions & 0 deletions test/fixtures/es-module-loaders/loader-resolve-passthru.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
export async function resolve(specifier, context, next) {
// This check is needed to make sure that we don't prevent the
// resolution from follow-up loaders. It wouldn't be a problem
// in real life because loaders aren't supposed to break the
// resolution, but the ones used in our tests do, for convenience.
if (specifier.includes('loader')) {
return next(specifier);
}

console.log('resolve passthru'); // This log is deliberate
return next(specifier);
}
10 changes: 9 additions & 1 deletion test/fixtures/es-module-loaders/loader-resolve-shortcircuit.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
export async function resolve(specifier) {
export async function resolve(specifier, context, next) {
// This check is needed to make sure that we don't prevent the
// resolution from follow-up loaders. It wouldn't be a problem
// in real life because loaders aren't supposed to break the
// resolution, but the ones used in our tests do, for convenience.
if (specifier.includes('loader')) {
return next(specifier);
}

return {
shortCircuit: true,
url: specifier,
Expand Down
4 changes: 4 additions & 0 deletions test/fixtures/es-module-loaders/loader-resolve-strip-xxx.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export async function resolve(specifier, context, nextResolve) {
console.log(`loader-a`, {specifier});
return nextResolve(specifier.replace(/^xxx\//, `./`));
}
4 changes: 4 additions & 0 deletions test/fixtures/es-module-loaders/loader-resolve-strip-yyy.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export async function resolve(specifier, context, nextResolve) {
console.log(`loader-b`, {specifier});
return nextResolve(specifier.replace(/^yyy\//, `./`));
}

0 comments on commit 7aac21e

Please sign in to comment.