Skip to content

Commit

Permalink
loader: fix package resolution for edge case
Browse files Browse the repository at this point in the history 
this commit solves a regression introduced with PR-40980.
if a resolve call results in a script with .mjs extension the
is automatically set to . This avoids the case where an additional
 in the same directory as the .mjs file would declare the
 to commonjs

PR-URL: nodejs#41218
Refs: nodejs#40980
Refs: yargs/yargs#2068
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Backport-PR-URL: nodejs#41752
  • Loading branch information
@dygabo
dygabo committed Feb 1, 2022
1 parent 3e60925 commit 29a7f7b
Show file tree
Hide file tree
Showing 4 changed files with 240 additions and 176 deletions.
66 changes: 42 additions & 24 deletions lib/internal/modules/esm/get_format.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ const legacyExtensionFormatMap = {
'.node': 'commonjs'
};

let experimentalSpecifierResolutionWarned = false;

if (experimentalWasmModules)
extensionFormatMap['.wasm'] = legacyExtensionFormatMap['.wasm'] = 'wasm';

Expand All @@ -53,41 +55,57 @@ const protocolHandlers = ObjectAssign(ObjectCreate(null), {

return format;
},
'file:'(parsed, url) {
const ext = extname(parsed.pathname);
let format;

if (ext === '.js') {
format = getPackageType(parsed.href) === 'module' ? 'module' : 'commonjs';
} else {
format = extensionFormatMap[ext];
}
if (!format) {
if (experimentalSpecifierResolution === 'node') {
process.emitWarning(
'The Node.js specifier resolution in ESM is experimental.',
'ExperimentalWarning');
format = legacyExtensionFormatMap[ext];
} else {
throw new ERR_UNKNOWN_FILE_EXTENSION(ext, fileURLToPath(url));
}
}

return format || null;
},
'file:': getFileProtocolModuleFormat,
'node:'() { return 'builtin'; },
});

function defaultGetFormat(url, context) {
function getLegacyExtensionFormat(ext) {
if (
experimentalSpecifierResolution === 'node' &&
!experimentalSpecifierResolutionWarned
) {
process.emitWarning(
'The Node.js specifier resolution in ESM is experimental.',
'ExperimentalWarning');
experimentalSpecifierResolutionWarned = true;
}
return legacyExtensionFormatMap[ext];
}

function getFileProtocolModuleFormat(url, ignoreErrors) {
const ext = extname(url.pathname);
if (ext === '.js') {
return getPackageType(url) === 'module' ? 'module' : 'commonjs';
}

const format = extensionFormatMap[ext];
if (format) return format;
if (experimentalSpecifierResolution !== 'node') {
// Explicit undefined return indicates load hook should rerun format check
if (ignoreErrors)
return undefined;
throw new ERR_UNKNOWN_FILE_EXTENSION(ext, fileURLToPath(url));
}
return getLegacyExtensionFormat(ext) ?? null;
}

function defaultGetFormatWithoutErrors(url, context) {
const parsed = new URL(url);
if (!ObjectPrototypeHasOwnProperty(protocolHandlers, parsed.protocol))
return null;
return protocolHandlers[parsed.protocol](parsed, true);
}

function defaultGetFormat(url, context) {
const parsed = new URL(url);
return ObjectPrototypeHasOwnProperty(protocolHandlers, parsed.protocol) ?
protocolHandlers[parsed.protocol](parsed, url) :
protocolHandlers[parsed.protocol](parsed, false) :
null;
}

module.exports = {
defaultGetFormat,
defaultGetFormatWithoutErrors,
extensionFormatMap,
legacyExtensionFormatMap,
};
3 changes: 1 addition & 2 deletions lib/internal/modules/esm/load.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

const { defaultGetFormat } = require('internal/modules/esm/get_format');
const { defaultGetSource } = require('internal/modules/esm/get_source');
const { translators } = require('internal/modules/esm/translators');
const { validateAssertions } = require('internal/modules/esm/assert');

/**
Expand All @@ -18,7 +17,7 @@ async function defaultLoad(url, context) {
} = context;
const { importAssertions } = context;

if (!format || !translators.has(format)) {
if (format == null) {
format = defaultGetFormat(url);
}

Expand Down
117 changes: 49 additions & 68 deletions lib/internal/modules/esm/resolve.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ function emitTrailingSlashPatternDeprecation(match, pjsonUrl, isExports, base) {
* @returns {void}
*/
function emitLegacyIndexDeprecation(url, packageJSONUrl, base, main) {
const format = defaultGetFormat(url);
const format = defaultGetFormatWithoutErrors(url);
if (format !== 'module')
return;
const path = fileURLToPath(url);
Expand Down Expand Up @@ -488,22 +488,6 @@ const patternRegEx = /\*/g;
function resolvePackageTargetString(
target, subpath, match, packageJSONUrl, base, pattern, internal, conditions) {

const composeResult = (resolved) => {
let format;
try {
format = getPackageType(resolved);
} catch (err) {
if (err.code === 'ERR_INVALID_FILE_URL_PATH') {
const invalidModuleErr = new ERR_INVALID_MODULE_SPECIFIER(
resolved, 'must not include encoded "/" or "\\" characters', base);
invalidModuleErr.cause = err;
throw invalidModuleErr;
}
throw err;
}
return { resolved, ...(format !== 'none') && { format } };
};

if (subpath !== '' && !pattern && target[target.length - 1] !== '/')
throwInvalidPackageTarget(match, target, packageJSONUrl, internal, base);

Expand Down Expand Up @@ -536,18 +520,22 @@ function resolvePackageTargetString(
if (!StringPrototypeStartsWith(resolvedPath, packagePath))
throwInvalidPackageTarget(match, target, packageJSONUrl, internal, base);

if (subpath === '') return composeResult(resolved);
if (subpath === '') return resolved;

if (RegExpPrototypeTest(invalidSegmentRegEx, subpath))
throwInvalidSubpath(match + subpath, packageJSONUrl, internal, base);

if (pattern) {
return composeResult(new URL(RegExpPrototypeSymbolReplace(patternRegEx,
resolved.href,
() => subpath)));
return new URL(
RegExpPrototypeSymbolReplace(
patternRegEx,
resolved.href,
() => subpath
)
);
}

return composeResult(new URL(subpath, resolved));
return new URL(subpath, resolved);
}

/**
Expand Down Expand Up @@ -673,15 +661,15 @@ function packageExportsResolve(
!StringPrototypeIncludes(packageSubpath, '*') &&
!StringPrototypeEndsWith(packageSubpath, '/')) {
const target = exports[packageSubpath];
const resolveResult = resolvePackageTarget(
const resolved = resolvePackageTarget(
packageJSONUrl, target, '', packageSubpath, base, false, false, conditions
);

if (resolveResult == null) {
if (resolved == null) {
throwExportsNotFound(packageSubpath, packageJSONUrl, base);
}

return { ...resolveResult, exact: true };
return { resolved, exact: true };
}

let bestMatch = '';
Expand Down Expand Up @@ -717,7 +705,7 @@ function packageExportsResolve(
if (bestMatch) {
const target = exports[bestMatch];
const pattern = StringPrototypeIncludes(bestMatch, '*');
const resolveResult = resolvePackageTarget(
const resolved = resolvePackageTarget(
packageJSONUrl,
target,
bestMatchSubpath,
Expand All @@ -727,15 +715,15 @@ function packageExportsResolve(
false,
conditions);

if (resolveResult == null) {
if (resolved == null) {
throwExportsNotFound(packageSubpath, packageJSONUrl, base);
}

if (!pattern) {
emitFolderMapDeprecation(bestMatch, packageJSONUrl, true, base);
}

return { ...resolveResult, exact: pattern };
return { resolved, exact: pattern };
}

throwExportsNotFound(packageSubpath, packageJSONUrl, base);
Expand Down Expand Up @@ -775,11 +763,11 @@ function packageImportsResolve(name, base, conditions) {
if (ObjectPrototypeHasOwnProperty(imports, name) &&
!StringPrototypeIncludes(name, '*') &&
!StringPrototypeEndsWith(name, '/')) {
const resolveResult = resolvePackageTarget(
const resolved = resolvePackageTarget(
packageJSONUrl, imports[name], '', name, base, false, true, conditions
);
if (resolveResult != null) {
return { resolved: resolveResult.resolved, exact: true };
if (resolved != null) {
return { resolved, exact: true };
}
} else {
let bestMatch = '';
Expand Down Expand Up @@ -812,15 +800,15 @@ function packageImportsResolve(name, base, conditions) {
if (bestMatch) {
const target = imports[bestMatch];
const pattern = StringPrototypeIncludes(bestMatch, '*');
const resolveResult = resolvePackageTarget(
const resolved = resolvePackageTarget(
packageJSONUrl, target,
bestMatchSubpath, bestMatch,
base, pattern, true,
conditions);
if (resolveResult !== null) {
if (resolved !== null) {
if (!pattern)
emitFolderMapDeprecation(bestMatch, packageJSONUrl, false, base);
return { resolved: resolveResult.resolved, exact: pattern };
return { resolved, exact: pattern };
}
}
}
Expand Down Expand Up @@ -880,7 +868,7 @@ function parsePackageName(specifier, base) {
* @param {string} specifier
* @param {string | URL | undefined} base
* @param {Set<string>} conditions
* @returns {resolved: URL, format? : string}
* @returns {URL}
*/
function packageResolve(specifier, base, conditions) {
if (NativeModule.canBeRequiredByUsers(specifier))
Expand All @@ -896,7 +884,8 @@ function packageResolve(specifier, base, conditions) {
if (packageConfig.name === packageName &&
packageConfig.exports !== undefined && packageConfig.exports !== null) {
return packageExportsResolve(
packageJSONUrl, packageSubpath, packageConfig, base, conditions);
packageJSONUrl, packageSubpath, packageConfig, base, conditions
).resolved;
}
}

Expand All @@ -920,24 +909,19 @@ function packageResolve(specifier, base, conditions) {
const packageConfig = getPackageConfig(packageJSONPath, specifier, base);
if (packageConfig.exports !== undefined && packageConfig.exports !== null) {
return packageExportsResolve(
packageJSONUrl, packageSubpath, packageConfig, base, conditions);
packageJSONUrl, packageSubpath, packageConfig, base, conditions
).resolved;
}

if (packageSubpath === '.') {
return {
resolved: legacyMainResolve(
packageJSONUrl,
packageConfig,
base),
...(packageConfig.type !== 'none') && { format: packageConfig.type }
};
return legacyMainResolve(
packageJSONUrl,
packageConfig,
base
);
}

return {
resolved: new URL(packageSubpath, packageJSONUrl),
...(packageConfig.type !== 'none') && { format: packageConfig.type }
};

return new URL(packageSubpath, packageJSONUrl);
// Cross-platform root check.
} while (packageJSONPath.length !== lastPath.length);

Expand Down Expand Up @@ -981,7 +965,6 @@ function moduleResolve(specifier, base, conditions, preserveSymlinks) {
// Order swapped from spec for minor perf gain.
// Ok since relative URLs cannot parse as URLs.
let resolved;
let format;
if (shouldBeTreatedAsRelativeOrAbsolutePath(specifier)) {
resolved = new URL(specifier, base);
} else if (specifier[0] === '#') {
Expand All @@ -990,15 +973,12 @@ function moduleResolve(specifier, base, conditions, preserveSymlinks) {
try {
resolved = new URL(specifier);
} catch {
({ resolved, format } = packageResolve(specifier, base, conditions));
resolved = packageResolve(specifier, base, conditions);
}
}
if (resolved.protocol !== 'file:')
return resolved;
return {
url: finalizeResolution(resolved, base, preserveSymlinks),
...(format != null) && { format }
};
return finalizeResolution(resolved, base, preserveSymlinks);
}

/**
Expand Down Expand Up @@ -1047,6 +1027,13 @@ function resolveAsCommonJS(specifier, parentURL) {
}
}

function throwIfUnsupportedURLProtocol(url) {
if (url.protocol !== 'file:' && url.protocol !== 'data:' &&
url.protocol !== 'node:') {
throw new ERR_UNSUPPORTED_ESM_URL_SCHEME(url);
}
}

function defaultResolve(specifier, context = {}, defaultResolveUnused) {
let { parentURL, conditions } = context;
if (parentURL && policy?.manifest) {
Expand Down Expand Up @@ -1087,15 +1074,9 @@ function defaultResolve(specifier, context = {}, defaultResolveUnused) {

conditions = getConditionsSet(conditions);
let url;
let format;
try {
({ url, format } =
moduleResolve(
specifier,
parentURL,
conditions,
isMain ? preserveSymlinksMain : preserveSymlinks
));
url = moduleResolve(specifier, parentURL, conditions,
isMain ? preserveSymlinksMain : preserveSymlinks);
} catch (error) {
// Try to give the user a hint of what would have been the
// resolved CommonJS module
Expand All @@ -1119,13 +1100,11 @@ function defaultResolve(specifier, context = {}, defaultResolveUnused) {
throw error;
}

if (url.protocol !== 'file:' && url.protocol !== 'data:' &&
url.protocol !== 'node:')
throw new ERR_UNSUPPORTED_ESM_URL_SCHEME(url);
throwIfUnsupportedURLProtocol(url);

return {
url: `${url}`,
...(format != null) && { format }
format: defaultGetFormatWithoutErrors(url),
};
}

Expand All @@ -1140,4 +1119,6 @@ module.exports = {
};

// cycle
const { defaultGetFormat } = require('internal/modules/esm/get_format');
const {
defaultGetFormatWithoutErrors,
} = require('internal/modules/esm/get_format');
Loading

0 comments on commit 29a7f7b

Please sign in to comment.