Skip to content

Commit

Permalink
change default resolver to not throw on unknown scheme
Browse files Browse the repository at this point in the history
  • Loading branch information
giltayar committed May 1, 2023
1 parent 32778b8 commit 4e80b07
Show file tree
Hide file tree
Showing 9 changed files with 136 additions and 98 deletions.
92 changes: 46 additions & 46 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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_)
Expand Down
32 changes: 32 additions & 0 deletions lib/internal/modules/esm/load.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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.
Expand Down
34 changes: 0 additions & 34 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions test/es-module/test-esm-import-meta-resolve.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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/');

Expand Down
30 changes: 30 additions & 0 deletions test/es-module/test-esm-loader-default-resolver.mjs
Original file line number Diff line number Diff line change
@@ -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, '');
})
})
18 changes: 0 additions & 18 deletions test/fixtures/es-module-loaders/http-loader.mjs
Original file line number Diff line number Diff line change
@@ -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) => {
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/es-module-loaders/uyyt-dummy-loader-main.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import 'uyyt://1/index.mjs';
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import 'uyyt://1/index2.mjs';
24 changes: 24 additions & 0 deletions test/fixtures/es-module-loaders/uyyt-dummy-loader.mjs
Original file line number Diff line number Diff line change
@@ -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);
}
}

0 comments on commit 4e80b07

Please sign in to comment.