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 danielleadams committed Jun 12, 2022
1 parent 7f8f61a commit 916e45e
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 126 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 @@ -1026,8 +1026,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 @@ -1043,8 +1042,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
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
76 changes: 28 additions & 48 deletions lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,11 @@ const {
RegExpPrototypeExec,
SafeArrayIterator,
SafeWeakMap,
StringPrototypeStartsWith,
globalThis,
} = primordials;
const { MessageChannel } = require('internal/worker/io');

const {
ERR_INTERNAL_ASSERTION,
ERR_INVALID_ARG_TYPE,
ERR_INVALID_ARG_VALUE,
ERR_INVALID_RETURN_PROPERTY_VALUE,
Expand All @@ -50,10 +48,6 @@ const { defaultLoad } = require('internal/modules/esm/load');
const { translators } = require(
'internal/modules/esm/translators');
const { getOptionValue } = require('internal/options');
const {
fetchModule,
} = require('internal/modules/esm/fetch_module');


/**
* Prevent the specifier resolution warning from being printed twice
Expand Down Expand Up @@ -238,9 +232,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 @@ -256,43 +248,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 {string} url
* @returns {string}
*/
getBaseURL(url) {
if (
StringPrototypeStartsWith(url, 'http:') ||
StringPrototypeStartsWith(url, 'https:')
) {
// The request & response have already settled, so they are in
// fetchModule's cache, in which case, fetchModule returns
// immediately and synchronously
url = fetchModule(new URL(url), { parentURL: url }).resolvedHREF;
// This should only occur if the module hasn't been fetched yet
if (typeof url !== 'string') {
throw new ERR_INTERNAL_ASSERTION(`Base url for module ${url} not loaded.`);
}
}
return url;
}

/**
* Get a (possibly still pending) module job from the cache,
* or create one and return its Promise.
Expand Down Expand Up @@ -346,6 +301,7 @@ class ESMLoader {
const moduleProvider = async (url, isMain) => {
const {
format: finalFormat,
responseURL,
source,
} = await this.load(url, {
format,
Expand All @@ -355,10 +311,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 @@ -442,6 +398,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 @@ -477,6 +456,7 @@ class ESMLoader {

return {
format,
responseURL,
source,
};
}
Expand Down
3 changes: 1 addition & 2 deletions lib/internal/modules/esm/module_job.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ class ModuleJob {
// these `link` callbacks depending on each other.
const dependencyJobs = [];
const promises = this.module.link(async (specifier, assertions) => {
const baseURL = this.loader.getBaseURL(url);
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
1 change: 0 additions & 1 deletion test/parallel/test-bootstrap-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ const expectedModules = new Set([
'NativeModule internal/modules/esm/fetch_module',
'NativeModule internal/modules/esm/formats',
'NativeModule internal/modules/esm/get_format',
'NativeModule internal/modules/esm/get_source',
'NativeModule internal/modules/esm/handle_process_exit',
'NativeModule internal/modules/esm/initialize_import_meta',
'NativeModule internal/modules/esm/load',
Expand Down

0 comments on commit 916e45e

Please sign in to comment.