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 Jan 29, 2022
1 parent 5fe75b0 commit 5fedf30
Show file tree
Hide file tree
Showing 4 changed files with 241 additions and 171 deletions.
66 changes: 42 additions & 24 deletions lib/internal/modules/esm/get_format.js
View file
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,
};
5 changes: 3 additions & 2 deletions lib/internal/modules/esm/load.js
View file
Expand Up @@ -2,15 +2,16 @@

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

async function defaultLoad(url, context) {
let {
format,
source,
} = context;

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

if (
format === 'builtin' ||
Expand Down
111 changes: 48 additions & 63 deletions lib/internal/modules/esm/resolve.js
View file
Expand Up @@ -130,7 +130,7 @@ function emitTrailingSlashPatternDeprecation(match, pjsonUrl, isExports, base) {
* @returns
*/
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 @@ -474,22 +474,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 @@ -522,18 +506,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 @@ -659,15 +647,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 @@ -703,7 +691,7 @@ function packageExportsResolve(
if (bestMatch) {
const target = exports[bestMatch];
const pattern = StringPrototypeIncludes(bestMatch, '*');
const resolveResult = resolvePackageTarget(
const resolved = resolvePackageTarget(
packageJSONUrl,
target,
bestMatchSubpath,
Expand All @@ -713,15 +701,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 @@ -761,11 +749,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 @@ -798,15 +786,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 @@ -883,7 +871,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 @@ -907,24 +896,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 @@ -967,7 +951,6 @@ function moduleResolve(specifier, base, conditions) {
// 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 @@ -976,13 +959,10 @@ function moduleResolve(specifier, base, conditions) {
try {
resolved = new URL(specifier);
} catch {
({ resolved, format } = packageResolve(specifier, base, conditions));
resolved = packageResolve(specifier, base, conditions);
}
}
return {
url: finalizeResolution(resolved, base),
...(format != null) && { format }
};
return finalizeResolution(resolved, base);
}

/**
Expand Down Expand Up @@ -1031,6 +1011,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 @@ -1093,14 +1080,8 @@ function defaultResolve(specifier, context = {}, defaultResolveUnused) {

conditions = getConditionsSet(conditions);
let url;
let format;
try {
({ url, format } =
moduleResolve(
specifier,
parentURL,
conditions
));
url = moduleResolve(specifier, parentURL, conditions);
} catch (error) {
// Try to give the user a hint of what would have been the
// resolved CommonJS module
Expand Down Expand Up @@ -1136,9 +1117,11 @@ function defaultResolve(specifier, context = {}, defaultResolveUnused) {
url.hash = old.hash;
}

throwIfUnsupportedURLProtocol(url);

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

Expand All @@ -1153,4 +1136,6 @@ module.exports = {
};

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

0 comments on commit 5fedf30

Please sign in to comment.