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: #41218
Refs: #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>
  • Loading branch information
@dygabo @targos
dygabo authored and targos committed Jan 14, 2022
1 parent fac0871 commit 68fd2ac
Show file tree
Hide file tree
Showing 5 changed files with 233 additions and 169 deletions.
4 changes: 2 additions & 2 deletions lib/internal/modules/cjs/loader.js
View file
Expand Up @@ -457,7 +457,7 @@ function trySelf(parentPath, request) {
try {
return finalizeEsmResolution(packageExportsResolve(
pathToFileURL(pkgPath + '/package.json'), expansion, pkg,
pathToFileURL(parentPath), cjsConditions).resolved, parentPath, pkgPath);
pathToFileURL(parentPath), cjsConditions), parentPath, pkgPath);
} catch (e) {
if (e.code === 'ERR_MODULE_NOT_FOUND')
throw createEsmNotFoundErr(request, pkgPath + '/package.json');
Expand All @@ -481,7 +481,7 @@ function resolveExports(nmPath, request) {
try {
return finalizeEsmResolution(packageExportsResolve(
pathToFileURL(pkgPath + '/package.json'), '.' + expansion, pkg, null,
cjsConditions).resolved, null, pkgPath);
cjsConditions), null, pkgPath);
} catch (e) {
if (e.code === 'ERR_MODULE_NOT_FOUND')
throw createEsmNotFoundErr(request, pkgPath + '/package.json');
Expand Down
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,
};
3 changes: 1 addition & 2 deletions lib/internal/modules/esm/load.js
View file
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
99 changes: 40 additions & 59 deletions lib/internal/modules/esm/resolve.js
View file
Expand Up @@ -107,7 +107,7 @@ function emitTrailingSlashPatternDeprecation(match, pjsonUrl, 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 @@ -464,22 +464,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 @@ -512,7 +496,7 @@ 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)) {
const request = pattern ?
Expand All @@ -521,12 +505,16 @@ function resolvePackageTargetString(
}

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 @@ -753,7 +741,7 @@ function packageImportsResolve(name, base, conditions) {
packageJSONUrl, imports[name], '', name, base, false, true, conditions
);
if (resolveResult != null) {
return resolveResult.resolved;
return resolveResult;
}
} else {
let bestMatch = '';
Expand Down Expand Up @@ -785,7 +773,7 @@ function packageImportsResolve(name, base, conditions) {
bestMatch, base, true,
true, conditions);
if (resolveResult != null) {
return resolveResult.resolved;
return resolveResult;
}
}
}
Expand Down Expand Up @@ -849,7 +837,7 @@ function parsePackageName(specifier, base) {
*/
function packageResolve(specifier, base, conditions) {
if (NativeModule.canBeRequiredByUsers(specifier))
return { resolved: new URL('node:' + specifier) };
return new URL('node:' + specifier);

const { packageName, packageSubpath, isScoped } =
parsePackageName(specifier, base);
Expand Down Expand Up @@ -888,19 +876,14 @@ function packageResolve(specifier, base, conditions) {
packageJSONUrl, packageSubpath, packageConfig, base, conditions);
}
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 @@ -944,7 +927,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 @@ -953,19 +935,13 @@ 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 {
url: resolved
};
return resolved;
}

return {
url: finalizeResolution(resolved, base, preserveSymlinks),
...(format != null) && { format }
};
return finalizeResolution(resolved, base, preserveSymlinks);
}

/**
Expand Down Expand Up @@ -1014,6 +990,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 @@ -1054,15 +1037,13 @@ 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 @@ -1086,13 +1067,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 @@ -1107,4 +1086,6 @@ module.exports = {
};

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

0 comments on commit 68fd2ac

Please sign in to comment.