Skip to content

Commit

Permalink
esm: refactor responseURL handling
Browse files Browse the repository at this point in the history
PR-URL: #43164
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
  • Loading branch information
guybedford authored and targos committed Jul 12, 2022
1 parent 254efd9 commit 50d64ed
Show file tree
Hide file tree
Showing 9 changed files with 97 additions and 154 deletions.
6 changes: 2 additions & 4 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -1030,8 +1030,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 @@ -1047,8 +1046,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
65 changes: 63 additions & 2 deletions lib/internal/modules/esm/load.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,67 @@
'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) {
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');
}
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 +70,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 +89,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
89 changes: 28 additions & 61 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,
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 @@ -607,6 +550,29 @@ class ESMLoader {
format,
source,
} = loaded;
let responseURL = loaded.responseURL;

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

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

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

if (format == null) {
const dataUrl = RegExpPrototypeExec(
Expand Down Expand Up @@ -644,6 +610,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);
ArrayPrototypePush(dependencyJobs, jobPromise);
const job = await jobPromise;
return job.modulePromise;
Expand Down
8 changes: 2 additions & 6 deletions lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,7 @@ function errPath(url) {
}

async function importModuleDynamically(specifier, { url }, assertions) {
return asyncESM.esmLoader.import(specifier,
asyncESM.esmLoader.getBaseURL(url),
assertions);
return asyncESM.esmLoader.import(specifier, url, assertions);
}

// Strategy for loading a standard JavaScript module.
Expand All @@ -116,9 +114,7 @@ translators.set('module', async function moduleStrategy(url, source, isMain) {
debug(`Translating StandardModule ${url}`);
const module = new ModuleWrap(url, undefined, source, 0, 0);
moduleWrap.callbackMap.set(module, {
initializeImportMeta: (meta, wrap) => this.importMetaInitialize(meta, {
url: wrap.url
}),
initializeImportMeta: (meta, wrap) => this.importMetaInitialize(meta, { url }),
importModuleDynamically,
});
return module;
Expand Down
Loading

0 comments on commit 50d64ed

Please sign in to comment.