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 2 commits
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
6 changes: 2 additions & 4 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -1023,8 +1023,7 @@ function wrapSafe(filename, content, cjsModuleInstance) {
displayErrors: true,
importModuleDynamically: async (specifier, _, importAssertions) => {
const loader = asyncESM.esmLoader;
return loader.import(specifier,
loader.getBaseURL(normalizeReferrerURL(filename)),
return loader.import(specifier, normalizeReferrerURL(filename),
importAssertions);
},
});
Expand All @@ -1040,8 +1039,7 @@ function wrapSafe(filename, content, cjsModuleInstance) {
filename,
importModuleDynamically(specifier, _, importAssertions) {
const loader = asyncESM.esmLoader;
return loader.import(specifier,
loader.getBaseURL(normalizeReferrerURL(filename)),
return loader.import(specifier, normalizeReferrerURL(filename),
importAssertions);
},
});
Expand Down
11 changes: 0 additions & 11 deletions lib/internal/modules/esm/fetch_module.js
Original file line number Diff line number Diff line change
Expand Up @@ -238,17 +238,6 @@ function fetchModule(parsed, { parentURL }) {
return fetchWithRedirects(parsed);
}

/**
* Checks if the given canonical URL exists in the fetch cache
*
* @param {string} key
* @returns {boolean}
*/
function inFetchCache(key) {
return cacheForGET.has(key);
}

module.exports = {
fetchModule,
inFetchCache,
};
60 changes: 0 additions & 60 deletions lib/internal/modules/esm/get_source.js

This file was deleted.

4 changes: 1 addition & 3 deletions lib/internal/modules/esm/initialize_import_meta.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,13 @@ function createImportMetaResolve(defaultParentUrl) {
* @param {{url: string}} context
*/
function initializeImportMeta(meta, context) {
let url = context.url;
const { url } = context;

// Alphabetical
if (experimentalImportMetaResolve) {
meta.resolve = createImportMetaResolve(url);
}

url = asyncESM.esmLoader.getBaseURL(url);

meta.url = url;
}

Expand Down
64 changes: 62 additions & 2 deletions lib/internal/modules/esm/load.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,66 @@
'use strict';

const {
ArrayPrototypePush,
RegExpPrototypeExec,
decodeURIComponent,
} = primordials;

const { defaultGetFormat } = require('internal/modules/esm/get_format');
const { defaultGetSource } = require('internal/modules/esm/get_source');
const { validateAssertions } = require('internal/modules/esm/assert');
const { getOptionValue } = require('internal/options');
const { fetchModule } = require('internal/modules/esm/fetch_module');

// Do not eagerly grab .manifest, it may be in TDZ
const policy = getOptionValue('--experimental-policy') ?
require('internal/process/policy') :
null;
const experimentalNetworkImports =
getOptionValue('--experimental-network-imports');

const { Buffer: { from: BufferFrom } } = require('buffer');

const { readFile: readFileAsync } = require('internal/fs/promises').exports;
const { URL } = require('internal/url');
const {
ERR_INVALID_URL,
ERR_UNSUPPORTED_ESM_URL_SCHEME,
} = require('internal/errors').codes;

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 source;
if (parsed.protocol === 'file:') {
source = await readFileAsync(parsed);
} else if (parsed.protocol === 'data:') {
const match = RegExpPrototypeExec(DATA_URL_PATTERN, parsed.pathname);
if (!match) {
throw new ERR_INVALID_URL(url);
}
const { 1: base64, 2: body } = match;
source = BufferFrom(decodeURIComponent(body), base64 ? 'base64' : 'utf8');
} else if (experimentalNetworkImports && (
parsed.protocol === 'https:' ||
parsed.protocol === 'http:'
)) {
const res = await fetchModule(parsed, context);
source = await res.body;
responseURL = res.resolvedHREF;
} else {
const supportedSchemes = ['file', 'data'];
if (experimentalNetworkImports)
ArrayPrototypePush(supportedSchemes, 'http', 'https');
guybedford marked this conversation as resolved.
Show resolved Hide resolved
throw new ERR_UNSUPPORTED_ESM_URL_SCHEME(parsed, supportedSchemes);
}
if (policy?.manifest) {
policy.manifest.assertIntegrity(parsed, source);
}
return { responseURL, source };
}


/**
* Node.js default load hook.
Expand All @@ -11,6 +69,7 @@ const { validateAssertions } = require('internal/modules/esm/assert');
* @returns {object}
*/
async function defaultLoad(url, context) {
let responseURL = url;
const { importAssertions } = context;
let {
format,
Expand All @@ -29,11 +88,12 @@ async function defaultLoad(url, context) {
) {
source = null;
} else if (source == null) {
source = await defaultGetSource(url, context);
({ responseURL, source } = await getSource(url, context));
}

return {
format,
responseURL,
source,
};
}
Expand Down
98 changes: 36 additions & 62 deletions lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,12 @@ const {
RegExpPrototypeExec,
SafeArrayIterator,
SafeWeakMap,
StringPrototypeStartsWith,
globalThis,
} = primordials;
const { MessageChannel } = require('internal/worker/io');

const {
ERR_LOADER_CHAIN_INCOMPLETE,
ERR_INTERNAL_ASSERTION,
ERR_INVALID_ARG_TYPE,
ERR_INVALID_ARG_VALUE,
ERR_INVALID_RETURN_PROPERTY_VALUE,
Expand Down Expand Up @@ -55,11 +53,6 @@ const { defaultLoad } = require('internal/modules/esm/load');
const { translators } = require(
'internal/modules/esm/translators');
const { getOptionValue } = require('internal/options');
const {
fetchModule,
inFetchCache,
} = require('internal/modules/esm/fetch_module');


/**
* @typedef {object} ExportedHooks
Expand Down Expand Up @@ -306,9 +299,7 @@ class ESMLoader {
const module = new ModuleWrap(url, undefined, source, 0, 0);
callbackMap.set(module, {
importModuleDynamically: (specifier, { url }, importAssertions) => {
return this.import(specifier,
this.getBaseURL(url),
importAssertions);
return this.import(specifier, url, importAssertions);
}
});

Expand All @@ -324,55 +315,6 @@ class ESMLoader {
};
}

/**
* Returns the url to use for the resolution of a given cache key url
* These are not guaranteed to be the same.
*
* In WHATWG HTTP spec for ESM the cache key is the non-I/O bound
* synchronous resolution using only string operations
* ~= resolveImportMap(new URL(specifier, importerHREF))
*
* The url used for subsequent resolution is the response URL after
* all redirects have been resolved.
*
* https://example.com/foo redirecting to https://example.com/bar
* would have a cache key of https://example.com/foo and baseURL
* of https://example.com/bar
*
* ! MUST BE SYNCHRONOUS for import.meta initialization
* ! MUST BE CALLED AFTER receiving the url body due to I/O
* @param {URL['href']} url
* @returns {string|Promise<URL['href']>}
*/
getBaseURL(url) {
if (getOptionValue('--experimental-network-imports') && (
StringPrototypeStartsWith(url, 'http:') ||
StringPrototypeStartsWith(url, 'https:')
)) {
// When using network-imports, the request & response have already settled
// so they are in fetchModule's cache, in which case, fetchModule returns
// immediately and synchronously
// Unless a custom loader bypassed the fetch cache, in which case we just
// use the original url
if (inFetchCache(url)) {
const module = fetchModule(new URL(url), { parentURL: url });
if (typeof module?.resolvedHREF === 'string') {
return module.resolvedHREF;
}
// Internal error
throw new ERR_INTERNAL_ASSERTION(
`Base url for module ${url} not loaded.`
);
} else {
// A custom loader was used instead of network-imports.
// Adding support for a response URL resolve return in custom loaders is
// pending.
return url;
}
}
return url;
}

/**
* Get a (possibly still pending) module job from the cache,
* or create one and return its Promise.
Expand Down Expand Up @@ -431,6 +373,7 @@ class ESMLoader {
const moduleProvider = async (url, isMain) => {
const {
format: finalFormat,
responseURL,
guybedford marked this conversation as resolved.
Show resolved Hide resolved
source,
} = await this.load(url, {
format,
Expand All @@ -440,10 +383,10 @@ class ESMLoader {
const translator = translators.get(finalFormat);

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

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

const inspectBrk = (
Expand Down Expand Up @@ -523,7 +466,7 @@ class ESMLoader {
* hooks starts at the top and each call to `nextLoad()` moves down 1 step
* until it reaches the bottom or short-circuits.
*
* @param {URL['href']} url The URL/path of the module to be loaded
* @param {string} url The URL/path of the module to be loaded
guybedford marked this conversation as resolved.
Show resolved Hide resolved
* @param {object} context Metadata about the module
* @returns {{ format: ModuleFormat, source: ModuleSource }}
*/
Expand Down Expand Up @@ -607,6 +550,36 @@ class ESMLoader {
format,
source,
} = loaded;
let responseURL = loaded.responseURL;

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

if (typeof responseURL !== 'string') {
throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
'undefined or a string',
guybedford marked this conversation as resolved.
Show resolved Hide resolved
hookErrIdentifier,
'responseURL',
responseURL,
);
}

let responseURLObj;
try {
responseURLObj = new URL(responseURL);
} catch {
// Continue regardless of error.
}
guybedford marked this conversation as resolved.
Show resolved Hide resolved

if (responseURLObj?.href !== responseURL) {
throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
'a fully resolved URL string',
hookErrIdentifier,
'responseURL',
responseURL,
);
}
guybedford marked this conversation as resolved.
Show resolved Hide resolved

if (format == null) {
const dataUrl = RegExpPrototypeExec(
Expand Down Expand Up @@ -644,6 +617,7 @@ class ESMLoader {

return {
format,
responseURL,
source,
};
}
Expand Down
7 changes: 1 addition & 6 deletions lib/internal/modules/esm/module_job.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,7 @@ class ModuleJob {
// these `link` callbacks depending on each other.
const dependencyJobs = [];
const promises = this.module.link(async (specifier, assertions) => {
const base = await this.loader.getBaseURL(url);
const baseURL = typeof base === 'string' ?
base :
base.resolvedHREF;

const jobPromise = this.loader.getModuleJob(specifier, baseURL, assertions);
const jobPromise = this.loader.getModuleJob(specifier, url, assertions);
guybedford marked this conversation as resolved.
Show resolved Hide resolved
ArrayPrototypePush(dependencyJobs, jobPromise);
const job = await jobPromise;
return job.modulePromise;
Expand Down
Loading