Skip to content

Commit

Permalink
esm: do not interpret "main" as a URL
Browse files Browse the repository at this point in the history
As documented, its value is a path.

PR-URL: #55003
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
  • Loading branch information
aduh95 authored and targos committed Oct 4, 2024
1 parent 2df6e94 commit fe866e3
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 46 deletions.
27 changes: 13 additions & 14 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const {
StringPrototypeStartsWith,
encodeURIComponent,
} = primordials;
const assert = require('internal/assert');
const internalFS = require('internal/fs/utils');
const { BuiltinModule } = require('internal/bootstrap/realm');
const { realpathSync } = require('fs');
Expand Down Expand Up @@ -117,18 +118,17 @@ function emitInvalidSegmentDeprecation(target, request, match, pjsonUrl, interna
* Emits a deprecation warning if the given URL is a module and
* the package.json file does not define a "main" or "exports" field.
* @param {URL} url - The URL of the module being resolved.
* @param {URL} packageJSONUrl - The URL of the package.json file for the module.
* @param {string} path - The path of the module being resolved.
* @param {string} pkgPath - The path of the parent dir of the package.json file for the module.
* @param {string | URL} [base] - The base URL for the module being resolved.
* @param {string} [main] - The "main" field from the package.json file.
*/
function emitLegacyIndexDeprecation(url, packageJSONUrl, base, main) {
function emitLegacyIndexDeprecation(url, path, pkgPath, base, main) {
if (process.noDeprecation) {
return;
}
const format = defaultGetFormatWithoutErrors(url);
if (format !== 'module') { return; }
const path = fileURLToPath(url);
const pkgPath = fileURLToPath(new URL('.', packageJSONUrl));
const basePath = fileURLToPath(base);
if (!main) {
process.emitWarning(
Expand Down Expand Up @@ -196,20 +196,19 @@ const legacyMainResolveExtensionsIndexes = {
* @returns {URL}
*/
function legacyMainResolve(packageJSONUrl, packageConfig, base) {
const packageJsonUrlString = packageJSONUrl.href;

if (typeof packageJsonUrlString !== 'string') {
throw new ERR_INVALID_ARG_TYPE('packageJSONUrl', ['URL'], packageJSONUrl);
}
assert(isURL(packageJSONUrl));
const pkgPath = fileURLToPath(new URL('.', packageJSONUrl));

const baseStringified = isURL(base) ? base.href : base;

const resolvedOption = FSLegacyMainResolve(packageJsonUrlString, packageConfig.main, baseStringified);
const resolvedOption = FSLegacyMainResolve(pkgPath, packageConfig.main, baseStringified);

const baseUrl = resolvedOption <= legacyMainResolveExtensionsIndexes.kResolvedByMainIndexNode ? `./${packageConfig.main}` : '';
const resolvedUrl = new URL(baseUrl + legacyMainResolveExtensions[resolvedOption], packageJSONUrl);
const maybeMain = resolvedOption <= legacyMainResolveExtensionsIndexes.kResolvedByMainIndexNode ?
packageConfig.main || './' : '';
const resolvedPath = resolve(pkgPath, maybeMain + legacyMainResolveExtensions[resolvedOption]);
const resolvedUrl = pathToFileURL(resolvedPath);

emitLegacyIndexDeprecation(resolvedUrl, packageJSONUrl, base, packageConfig.main);
emitLegacyIndexDeprecation(resolvedUrl, resolvedPath, pkgPath, base, packageConfig.main);

return resolvedUrl;
}
Expand Down Expand Up @@ -790,8 +789,8 @@ function packageResolve(specifier, base, conditions) {
// ResolveSelf
const packageConfig = packageJsonReader.getPackageScopeConfig(base);
if (packageConfig.exists) {
const packageJSONUrl = pathToFileURL(packageConfig.pjsonPath);
if (packageConfig.exports != null && packageConfig.name === packageName) {
const packageJSONUrl = pathToFileURL(packageConfig.pjsonPath);
return packageExportsResolve(
packageJSONUrl, packageSubpath, packageConfig, base, conditions);
}
Expand Down
35 changes: 4 additions & 31 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3236,37 +3236,18 @@ void BindingData::LegacyMainResolve(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
auto isolate = env->isolate();

Utf8Value utf8_package_json_url(isolate, args[0]);
auto package_json_url =
ada::parse<ada::url_aggregator>(utf8_package_json_url.ToStringView());

if (!package_json_url) {
THROW_ERR_INVALID_URL(isolate, "Invalid URL");
return;
}
auto utf8_package_path = Utf8Value(isolate, args[0]).ToString();

std::string package_initial_file = "";

ada::result<ada::url_aggregator> file_path_url;
std::optional<std::string> initial_file_path;
std::string file_path;

if (args.Length() >= 2 && args[1]->IsString()) {
auto package_config_main = Utf8Value(isolate, args[1]).ToString();

file_path_url = ada::parse<ada::url_aggregator>(
std::string("./") + package_config_main, &package_json_url.value());

if (!file_path_url) {
THROW_ERR_INVALID_URL(isolate, "Invalid URL");
return;
}

initial_file_path = node::url::FileURLToPath(env, *file_path_url);
if (!initial_file_path.has_value()) {
return;
}

initial_file_path =
PathResolve(env, {utf8_package_path, package_config_main});
FromNamespacedPath(&initial_file_path.value());

package_initial_file = *initial_file_path;
Expand Down Expand Up @@ -3297,15 +3278,7 @@ void BindingData::LegacyMainResolve(const FunctionCallbackInfo<Value>& args) {
}
}

file_path_url =
ada::parse<ada::url_aggregator>("./index", &package_json_url.value());

if (!file_path_url) {
THROW_ERR_INVALID_URL(isolate, "Invalid URL");
return;
}

initial_file_path = node::url::FileURLToPath(env, *file_path_url);
initial_file_path = PathResolve(env, {utf8_package_path, "./index"});
if (!initial_file_path.has_value()) {
return;
}
Expand Down
10 changes: 9 additions & 1 deletion test/es-module/test-cjs-legacyMainResolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ describe('legacyMainResolve', () => {
{},
''
),
{ message: /instance of URL/, code: 'ERR_INVALID_ARG_TYPE' },
{ code: 'ERR_INTERNAL_ASSERTION' },
);
});

Expand Down Expand Up @@ -166,4 +166,12 @@ describe('legacyMainResolve', () => {
{ message: /"base" argument must be/, code: 'ERR_INVALID_ARG_TYPE' },
);
});

it('should interpret main as a path, not a URL', () => {
const packageJsonUrl = fixtures.fileURL('/es-modules/legacy-main-resolver/package.json');
assert.deepStrictEqual(
legacyMainResolve(packageJsonUrl, { main: '../folder%25with percentage#/' }, packageJsonUrl),
fixtures.fileURL('/es-modules/folder%25with percentage#/index.js'),
);
});
});

0 comments on commit fe866e3

Please sign in to comment.