change default resolver to not throw on unknown scheme

Fixes nodejs/loaders#138

Author: giltayar

doc/api/esm.md

@@ -1029,28 +1029,6 @@ and there is no security.
 // https-loader.mjs
 import { get } from 'node:https';
 
-export function resolve(specifier, context, nextResolve) {
-  const { parentURL = null } = context;
-
-  // Normally Node.js would error on specifiers starting with 'https://', so
-  // this hook intercepts them and converts them into absolute URLs to be
-  // passed along to the later hooks below.
-  if (specifier.startsWith('https://')) {
-    return {
-      shortCircuit: true,
-      url: specifier,
-    };
-  } else if (parentURL && parentURL.startsWith('https://')) {
-    return {
-      shortCircuit: true,
-      url: new URL(specifier, parentURL).href,
-    };
-  }
-
-  // Let Node.js handle all other specifiers.
-  return nextResolve(specifier);
-}
-
 export function load(url, context, nextLoad) {
   // For JavaScript to be loaded over the network, we need to fetch and
   // return it.
@@ -1091,9 +1069,7 @@ prints the current version of CoffeeScript per the module at the URL in
 #### Transpiler loader
 
 Sources that are in formats Node.js doesn't understand can be converted into
-JavaScript using the [`load` hook][load hook]. Before that hook gets called,
-however, a [`resolve` hook][resolve hook] needs to tell Node.js not to
-throw an error on unknown file types.
+JavaScript using the [`load` hook][load hook].
 
 This is less performant than transpiling source files before running
 Node.js; a transpiler loader should only be used for development and testing
@@ -1109,25 +1085,6 @@ import CoffeeScript from 'coffeescript';
 
 const baseURL = pathToFileURL(`${cwd()}/`).href;
 
-// CoffeeScript files end in .coffee, .litcoffee, or .coffee.md.
-const extensionsRegex = /\.coffee$|\.litcoffee$|\.coffee\.md$/;
-
-export function resolve(specifier, context, nextResolve) {
-  if (extensionsRegex.test(specifier)) {
-    const { parentURL = baseURL } = context;
-
-    // Node.js normally errors on unknown file extensions, so return a URL for
-    // specifiers ending in the CoffeeScript file extensions.
-    return {
-      shortCircuit: true,
-      url: new URL(specifier, parentURL).href,
-    };
-  }
-
-  // Let Node.js handle all other specifiers.
-  return nextResolve(specifier);
-}
-
 export async function load(url, context, nextLoad) {
   if (extensionsRegex.test(url)) {
     // Now that we patched resolve to let CoffeeScript URLs through, we need to
@@ -1220,6 +1177,49 @@ loaded from disk but before Node.js executes it; and so on for any `.coffee`,
 `.litcoffee` or `.coffee.md` files referenced via `import` statements of any
 loaded file.
 
+#### Overriding loader
+
+The above two loaders hooked into the "load" phase of the module loader.
+This loader hooks into the "resolution" phase. This loader reads an
+`overrides.json` file that specifies which specifiers to override to another
+url.
+
+```js
+// overriding-loader.js
+import fs from 'node:fs/promises'
+
+const overrides = JSON.parse(await fs.readFile('overrides.json'))
+
+export async function resolve(specifier, context, nextResolve) {
+  if (specifier in overrides) {
+    return nextResolve(overrides[specifier], context)
+  }
+
+  return nextResolve(specifier, context)
+}
+```
+
+Let's assume we have these files:
+
+```js
+// main.js
+import 'a-module-to-override'
+```
+```json
+// overrides.json
+{
+  "a-module-to-override": "./module-override.js"
+}
+```
+
+```js
+// module-override.js
+console.log('module overridden!')
+```
+
+If you run `node --experimental-loader ./overriding-loader.js main.js` the output will be
+`module overriden!`.
+
 ## Resolution algorithm
 
 ### Features
@@ -1506,9 +1506,9 @@ _isImports_, _conditions_)
 > 7. If _pjson?.type_ exists and is _"module"_, then
 >    1. If _url_ ends in _".js"_, then
 >       1. Return _"module"_.
->    2. Throw an _Unsupported File Extension_ error.
+>    2. return **undefined**.
 > 8. Otherwise,
->    1. Throw an _Unsupported File Extension_ error.
+>    1. return **undefined**.
 
 **LOOKUP\_PACKAGE\_SCOPE**(_url_)
 

lib/internal/modules/esm/load.js

@@ -79,6 +79,8 @@ async function defaultLoad(url, context = kEmptyObject) {
     source,
   } = context;
 
+  throwIfUnsupportedURLScheme(new URL(url), experimentalNetworkImports);
+
   if (format == null) {
     format = await defaultGetFormat(url, context);
   }
@@ -102,6 +104,36 @@ async function defaultLoad(url, context = kEmptyObject) {
   };
 }
 
+/**
+ * throws an error if the protocol is not one of the protocols
+ * that can be loaded in the default loader
+ *
+ * @param {URL} parsed
+ * @param {boolean} experimentalNetworkImports
+ */
+function throwIfUnsupportedURLScheme(parsed, experimentalNetworkImports) {
+  // Avoid accessing the `protocol` property due to the lazy getters.
+  const protocol = parsed?.protocol;
+  if (
+    protocol &&
+    protocol !== 'file:' &&
+    protocol !== 'data:' &&
+    protocol !== 'node:' &&
+    (
+      !experimentalNetworkImports ||
+      (
+        protocol !== 'https:' &&
+        protocol !== 'http:'
+      )
+    )
+  ) {
+    const schemes = ['file', 'data', 'node'];
+    if (experimentalNetworkImports) {
+      ArrayPrototypePush(schemes, 'https', 'http');
+    }
+    throw new ERR_UNSUPPORTED_ESM_URL_SCHEME(parsed, schemes);
+  }
+}
 
 /**
  * For a falsy `format` returned from `load`, throw an error.

lib/internal/modules/esm/resolve.js

@@ -941,37 +941,6 @@ function throwIfInvalidParentURL(parentURL) {
   }
 }
 
-function throwIfUnsupportedURLProtocol(url) {
-  // Avoid accessing the `protocol` property due to the lazy getters.
-  const protocol = url.protocol;
-  if (protocol !== 'file:' && protocol !== 'data:' &&
-      protocol !== 'node:') {
-    throw new ERR_UNSUPPORTED_ESM_URL_SCHEME(url);
-  }
-}
-
-function throwIfUnsupportedURLScheme(parsed, experimentalNetworkImports) {
-  // Avoid accessing the `protocol` property due to the lazy getters.
-  const protocol = parsed?.protocol;
-  if (
-    protocol &&
-    protocol !== 'file:' &&
-    protocol !== 'data:' &&
-    (
-      !experimentalNetworkImports ||
-      (
-        protocol !== 'https:' &&
-        protocol !== 'http:'
-      )
-    )
-  ) {
-    const schemes = ['file', 'data'];
-    if (experimentalNetworkImports) {
-      ArrayPrototypePush(schemes, 'https', 'http');
-    }
-    throw new ERR_UNSUPPORTED_ESM_URL_SCHEME(parsed, schemes);
-  }
-}
 
 function defaultResolve(specifier, context = {}) {
   let { parentURL, conditions } = context;
@@ -1048,7 +1017,6 @@ function defaultResolve(specifier, context = {}) {
   // This must come after checkIfDisallowedImport
   if (parsed && parsed.protocol === 'node:') return { __proto__: null, url: specifier };
 
-  throwIfUnsupportedURLScheme(parsed, experimentalNetworkImports);
 
   const isMain = parentURL === undefined;
   if (isMain) {
@@ -1095,8 +1063,6 @@ function defaultResolve(specifier, context = {}) {
     throw error;
   }
 
-  throwIfUnsupportedURLProtocol(url);
-
   return {
     __proto__: null,
     // Do NOT cast `url` to a string: that will work even when there are real

test/es-module/test-esm-import-meta-resolve.mjs

@@ -30,6 +30,8 @@ assert.strictEqual(
     code: 'ERR_INVALID_ARG_TYPE',
   })
 );
+assert.equal(import.meta.resolve('http://some-absolute/url'), 'http://some-absolute/url')
+assert.equal(import.meta.resolve('some://weird/protocol'), 'some://weird/protocol')
 assert.strictEqual(import.meta.resolve('baz/', fixtures),
                    fixtures + 'node_modules/baz/');
 

test/es-module/test-esm-loader-default-resolver.mjs

@@ -0,0 +1,30 @@
+import { spawnPromisified } from '../common/index.mjs';
+import * as fixtures from '../common/fixtures.mjs';
+import assert from 'node:assert';
+import { execPath } from 'node:process';
+import {describe, it} from 'node:test'
+
+describe('default resolver', () => {
+  it('should accept foreign schemas without exception (e.g. uyyt://something/or-other', async () => {
+    const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
+      '--no-warnings',
+      '--experimental-loader',
+      fixtures.fileURL('/es-module-loaders/uyyt-dummy-loader.mjs'),
+      fixtures.path('/es-module-loaders/uyyt-dummy-loader-main.mjs'),
+    ]);
+    assert.strictEqual(code, 0);
+    assert.strictEqual(stdout.trim(), 'index.mjs!');
+    assert.strictEqual(stderr, '');
+  })
+  it('should resolve foreign schemas by doing regular url absolutization', async () => {
+    const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
+      '--no-warnings',
+      '--experimental-loader',
+      fixtures.fileURL('/es-module-loaders/uyyt-dummy-loader.mjs'),
+      fixtures.path('/es-module-loaders/uyyt-dummy-loader-main2.mjs'),
+    ]);
+    assert.strictEqual(code, 0);
+    assert.strictEqual(stdout.trim(), '42');
+    assert.strictEqual(stderr, '');
+  })
+})

test/fixtures/es-module-loaders/http-loader.mjs

@@ -1,23 +1,5 @@
 import { get } from 'http';
 
-export function resolve(specifier, context, nextResolve) {
-  const { parentURL = null } = context;
-
-  if (specifier.startsWith('http://')) {
-    return {
-      shortCircuit: true,
-      url: specifier,
-    };
-  } else if (parentURL?.startsWith('http://')) {
-    return {
-      shortCircuit: true,
-      url: new URL(specifier, parentURL).href,
-    };
-  }
-
-  return nextResolve(specifier);
-}
-
 export function load(url, context, nextLoad) {
   if (url.startsWith('http://')) {
     return new Promise((resolve, reject) => {

test/fixtures/es-module-loaders/uyyt-dummy-loader-main.mjs

@@ -0,0 +1 @@
+import 'uyyt://1/index.mjs';

test/fixtures/es-module-loaders/uyyt-dummy-loader-main2.mjs

@@ -0,0 +1 @@
+import 'uyyt://1/index2.mjs';

test/fixtures/es-module-loaders/uyyt-dummy-loader.mjs

@@ -0,0 +1,24 @@
+export function load(url, context, nextLoad) {
+  switch (url) {
+    case 'uyyt://1/index.mjs':
+      return {
+        source: 'console.log("index.mjs!")',
+        format: 'module',
+        shortCircuit: true,
+      };
+    case 'uyyt://1/index2.mjs':
+      return {
+        source: 'import c from "./sub.mjs"; console.log(c);',
+        format: 'module',
+        shortCircuit: true,
+      };
+    case 'uyyt://1/sub.mjs':
+      return {
+        source: 'export default 42',
+        format: 'module',
+        shortCircuit: true,
+      };
+    default:
+      return nextLoad(url, context);
+  }
+}