Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

esm: refactor responseURL handling #43164

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,7 @@ changes:
* `context` {Object}
* `conditions` {string\[]} Export conditions of the relevant `package.json`
* `importAssertions` {Object}
* `parentURL` {string|undefined} The module importing this one, or undefined
* `parentUrl` {string|undefined} The module importing this one, or undefined
guybedford marked this conversation as resolved.
Show resolved Hide resolved
if this is the Node.js entry point
* `nextResolve` {Function} The subsequent `resolve` hook in the chain, or the
Node.js default `resolve` hook after the last user-supplied `resolve` hook
Expand Down Expand Up @@ -790,15 +790,15 @@ Node.js module specifier resolution behavior_ when calling `defaultResolve`, the

```js
export async function resolve(specifier, context, nextResolve) {
const { parentURL = null } = context;
const { parentUrl = null } = context;

if (Math.random() > 0.5) { // Some condition.
// For some or all specifiers, do some custom logic for resolving.
// Always return an object of the form {url: <string>}.
return {
shortCircuit: true,
url: parentURL ?
new URL(specifier, parentURL).href :
url: parentUrl ?
new URL(specifier, parentUrl).href :
new URL(specifier).href,
};
}
Expand Down Expand Up @@ -1007,7 +1007,7 @@ and there is no security.
import { get } from 'node:https';

export function resolve(specifier, context, nextResolve) {
const { parentURL = null } = context;
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
Expand All @@ -1017,10 +1017,10 @@ export function resolve(specifier, context, nextResolve) {
shortCircuit: true,
url: specifier
};
} else if (parentURL && parentURL.startsWith('https://')) {
} else if (parentUrl && parentUrl.startsWith('https://')) {
return {
shortCircuit: true,
url: new URL(specifier, parentURL).href,
url: new URL(specifier, parentUrl).href,
};
}

Expand Down Expand Up @@ -1083,20 +1083,20 @@ import { cwd } from 'node:process';
import { fileURLToPath, pathToFileURL } from 'node:url';
import CoffeeScript from 'coffeescript';

const baseURL = pathToFileURL(`${cwd()}/`).href;
const baseUrl = pathToFileURL(`${cwd()}/`).href;

// CoffeeScript files end in .coffee, .litcoffee or .coffee.md.
const extensionsRegex = /\.coffee$|\.litcoffee$|\.coffee\.md$/;

export async function resolve(specifier, context, nextResolve) {
if (extensionsRegex.test(specifier)) {
const { parentURL = baseURL } = context;
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
url: new URL(specifier, parentUrl).href
};
}

Expand Down
4 changes: 2 additions & 2 deletions lib/internal/modules/esm/fetch_module.js
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ async function isLocalAddress(hostname) {
* @param {ESModuleContext} context
* @returns {ReturnType<typeof fetchWithRedirects>}
*/
function fetchModule(parsed, { parentURL }) {
function fetchModule(parsed, { parentUrl }) {
const { href } = parsed;
const existing = cacheForGET.get(href);
if (existing) {
Expand All @@ -228,7 +228,7 @@ function fetchModule(parsed, { parentURL }) {
if (is !== true) {
throw new ERR_NETWORK_IMPORT_DISALLOWED(
href,
parentURL,
parentUrl,
'http can only be used to load local resources (use https instead).'
);
}
Expand Down
8 changes: 4 additions & 4 deletions lib/internal/modules/esm/get_format.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function getDataProtocolModuleFormat(parsed) {

/**
* @param {URL} url
* @param {{parentURL: string}} context
* @param {{parentUrl: string}} context
* @param {boolean} ignoreErrors
* @returns {string}
*/
Expand Down Expand Up @@ -85,7 +85,7 @@ function getFileProtocolModuleFormat(url, context, ignoreErrors) {

/**
* @param {URL} url
* @param {{parentURL: string}} context
* @param {{parentUrl: string}} context
* @returns {Promise<string> | undefined} only works when enabled
*/
function getHttpProtocolModuleFormat(url, context) {
Expand All @@ -101,7 +101,7 @@ function getHttpProtocolModuleFormat(url, context) {

/**
* @param {URL | URL['href']} url
* @param {{parentURL: string}} context
* @param {{parentUrl: string}} context
* @returns {Promise<string> | string | undefined} only works when enabled
*/
function defaultGetFormatWithoutErrors(url, context) {
Expand All @@ -113,7 +113,7 @@ function defaultGetFormatWithoutErrors(url, context) {

/**
* @param {URL | URL['href']} url
* @param {{parentURL: string}} context
* @param {{parentUrl: string}} context
* @returns {Promise<string> | string | undefined} only works when enabled
*/
function defaultGetFormat(url, context) {
Expand Down
12 changes: 6 additions & 6 deletions lib/internal/modules/esm/load.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const DATA_URL_PATTERN = /^[^/]+\/[^,;]+(?:[^,]*?)(;base64)?,([\s\S]*)$/;

async function getSource(url, context) {
guybedford marked this conversation as resolved.
Show resolved Hide resolved
const parsed = new URL(url);
let responseURL = url;
let responseUrl = url;
let source;
if (parsed.protocol === 'file:') {
source = await readFileAsync(parsed);
Expand All @@ -48,7 +48,7 @@ async function getSource(url, context) {
)) {
const res = await fetchModule(parsed, context);
source = await res.body;
responseURL = res.resolvedHREF;
responseUrl = res.resolvedHREF;
} else {
const supportedSchemes = ['file', 'data'];
if (experimentalNetworkImports) {
Expand All @@ -59,7 +59,7 @@ async function getSource(url, context) {
if (policy?.manifest) {
policy.manifest.assertIntegrity(parsed, source);
}
return { responseURL, source };
return { responseUrl, source };
}


Expand All @@ -70,7 +70,7 @@ async function getSource(url, context) {
* @returns {object}
*/
async function defaultLoad(url, context) {
let responseURL = url;
let responseUrl = url;
const { importAssertions } = context;
let {
format,
Expand All @@ -89,12 +89,12 @@ async function defaultLoad(url, context) {
) {
source = null;
} else if (source == null) {
({ responseURL, source } = await getSource(url, context));
({ responseUrl, source } = await getSource(url, context));
}

return {
format,
responseURL,
responseUrl,
source,
};
}
Expand Down
66 changes: 33 additions & 33 deletions lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -321,14 +321,14 @@ class ESMLoader {
* @param {string} specifier The string after `from` in an `import` statement,
* or the first parameter of an `import()`
* expression
* @param {string | undefined} parentURL The URL of the module importing this
* @param {string | undefined} parentUrl The URL of the module importing this
* one, unless this is the Node.js entry
* point.
* @param {Record<string, string>} importAssertions Validations for the
* module import.
* @returns {Promise<ModuleJob>} The (possibly pending) module job
*/
async getModuleJob(specifier, parentURL, importAssertions) {
async getModuleJob(specifier, parentUrl, importAssertions) {
let importAssertionsForResolve;

// By default, `this.#loaders` contains just the Node default load hook
Expand All @@ -342,7 +342,7 @@ class ESMLoader {
}

const { format, url } =
await this.resolve(specifier, parentURL, importAssertionsForResolve);
await this.resolve(specifier, parentUrl, importAssertionsForResolve);

let job = this.moduleMap.get(url, importAssertions.type);

Expand All @@ -352,7 +352,7 @@ class ESMLoader {
}

if (job === undefined) {
job = this.#createModuleJob(url, importAssertions, parentURL, format);
job = this.#createModuleJob(url, importAssertions, parentUrl, format);
}

return job;
Expand All @@ -363,17 +363,17 @@ class ESMLoader {
* @param {string} url The absolute URL that was resolved for this module
* @param {Record<string, string>} importAssertions Validations for the
* module import.
* @param {string} [parentURL] The absolute URL of the module importing this
* @param {string} [parentUrl] The absolute URL of the module importing this
* one, unless this is the Node.js entry point
* @param {string} [format] The format hint possibly returned by the
* `resolve` hook
* @returns {Promise<ModuleJob>} The (possibly pending) module job
*/
#createModuleJob(url, importAssertions, parentURL, format) {
#createModuleJob(url, importAssertions, parentUrl, format) {
const moduleProvider = async (url, isMain) => {
const {
format: finalFormat,
responseURL,
responseUrl,
source,
} = await this.load(url, {
format,
Expand All @@ -383,14 +383,14 @@ class ESMLoader {
const translator = translators.get(finalFormat);

if (!translator) {
throw new ERR_UNKNOWN_MODULE_FORMAT(finalFormat, responseURL);
throw new ERR_UNKNOWN_MODULE_FORMAT(finalFormat, responseUrl);
}

return FunctionPrototypeCall(translator, this, responseURL, source, isMain);
return FunctionPrototypeCall(translator, this, responseUrl, source, isMain);
};

const inspectBrk = (
parentURL === undefined &&
parentUrl === undefined &&
getOptionValue('--inspect-brk')
);

Expand All @@ -399,7 +399,7 @@ class ESMLoader {
url,
importAssertions,
moduleProvider,
parentURL === undefined,
parentUrl === undefined,
inspectBrk
);

Expand All @@ -416,14 +416,14 @@ class ESMLoader {
* loader module.
*
* @param {string | string[]} specifiers Path(s) to the module.
* @param {string} parentURL Path of the parent importing the module.
* @param {string} parentUrl Path of the parent importing the module.
* @param {Record<string, string>} importAssertions Validations for the
* module import.
* @returns {Promise<ExportedHooks | KeyedExports[]>}
* A collection of module export(s) or a list of collections of module
* export(s).
*/
async import(specifiers, parentURL, importAssertions) {
async import(specifiers, parentUrl, importAssertions) {
// For loaders, `import` is passed multiple things to process, it returns a
// list pairing the url and exports collected. This is especially useful for
// error messaging, to identity from where an export came. But, in most
Expand All @@ -439,7 +439,7 @@ class ESMLoader {
const jobs = new Array(count);

for (let i = 0; i < count; i++) {
jobs[i] = this.getModuleJob(specifiers[i], parentURL, importAssertions)
jobs[i] = this.getModuleJob(specifiers[i], parentUrl, importAssertions)
.then((job) => job.run())
.then(({ module }) => module.getNamespace());
}
Expand Down Expand Up @@ -550,27 +550,27 @@ class ESMLoader {
format,
source,
} = loaded;
let responseURL = loaded.responseURL;
let responseUrl = loaded.responseUrl;

if (responseURL === undefined) {
responseURL = url;
if (responseUrl === undefined) {
responseUrl = url;
}

let responseURLObj;
if (typeof responseURL === 'string') {
let responseURL;
if (typeof responseUrl === 'string') {
try {
responseURLObj = new URL(responseURL);
responseURL = new URL(responseUrl);
} catch {
// responseURLObj not defined will throw in next branch.
// responseURL not defined will throw in next branch.
}
}

if (responseURLObj?.href !== responseURL) {
if (responseURL?.href !== responseUrl) {
throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
'undefined or a fully resolved URL string',
hookErrIdentifier,
'responseURL',
responseURL,
'responseUrl',
responseUrl,
);
}

Expand Down Expand Up @@ -610,7 +610,7 @@ class ESMLoader {

return {
format,
responseURL,
responseUrl,
source,
};
}
Expand Down Expand Up @@ -713,27 +713,27 @@ class ESMLoader {
*
* @param {string} originalSpecifier The specified URL path of the module to
* be resolved.
* @param {string} [parentURL] The URL path of the module's parent.
* @param {string} [parentUrl] The URL path of the module's parent.
* @param {ImportAssertions} [importAssertions] Assertions from the import
* statement or expression.
* @returns {{ format: string, url: URL['href'] }}
*/
async resolve(
originalSpecifier,
parentURL,
parentUrl,
importAssertions = ObjectCreate(null)
) {
const isMain = parentURL === undefined;
const isMain = parentUrl === undefined;

if (
!isMain &&
typeof parentURL !== 'string' &&
!isURLInstance(parentURL)
typeof parentUrl !== 'string' &&
!isURLInstance(parentUrl)
) {
throw new ERR_INVALID_ARG_TYPE(
'parentURL',
'parentUrl',
['string', 'URL'],
parentURL,
parentUrl,
);
}
const resolvers = this.#resolvers;
Expand All @@ -749,7 +749,7 @@ class ESMLoader {
const context = {
conditions: DEFAULT_CONDITIONS,
importAssertions,
parentURL,
parentUrl,
};

const nextResolve = async (suppliedSpecifier, ctx = context) => {
Expand Down
Loading