Skip to content

Commit

Permalink
loader: return package format from defaultResolve if known
Browse files Browse the repository at this point in the history 
This is a proposed modification of defaultResolve to return the package
format in case it has been found during package resolution.
The format will be returned as described in the documentation:
https://nodejs.org/api/esm.html#resolvespecifier-context-defaultresolve
There is one new unit test as well:
test/es-module/test-esm-resolve-type.js

PR-URL: #40980
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
@dygabo @targos
dygabo authored and targos committed Jan 14, 2022
1 parent 76abcf3 commit 6d8eb6a
Show file tree
Hide file tree
Showing 8 changed files with 372 additions and 50 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), parentPath, pkgPath);
pathToFileURL(parentPath), cjsConditions).resolved, 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), null, pkgPath);
cjsConditions).resolved, null, pkgPath);
} catch (e) {
if (e.code === 'ERR_MODULE_NOT_FOUND')
throw createEsmNotFoundErr(request, pkgPath + '/package.json');
Expand Down
158 changes: 110 additions & 48 deletions lib/internal/modules/esm/resolve.js
View file
Expand Up @@ -463,6 +463,23 @@ 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 All @@ -478,7 +495,8 @@ function resolvePackageTargetString(
const exportTarget = pattern ?
RegExpPrototypeSymbolReplace(patternRegEx, target, () => subpath) :
target + subpath;
return packageResolve(exportTarget, packageJSONUrl, conditions);
return packageResolve(
exportTarget, packageJSONUrl, conditions);
}
}
throwInvalidPackageTarget(match, target, packageJSONUrl, internal, base);
Expand All @@ -494,18 +512,21 @@ function resolvePackageTargetString(
if (!StringPrototypeStartsWith(resolvedPath, packagePath))
throwInvalidPackageTarget(match, target, packageJSONUrl, internal, base);

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

if (RegExpPrototypeTest(invalidSegmentRegEx, subpath)) {
const request = pattern ?
StringPrototypeReplace(match, '*', () => subpath) : match + subpath;
throwInvalidSubpath(request, packageJSONUrl, internal, base);
}

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

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

/**
Expand All @@ -531,9 +552,9 @@ function resolvePackageTarget(packageJSONUrl, target, subpath, packageSubpath,
let lastException;
for (let i = 0; i < target.length; i++) {
const targetItem = target[i];
let resolved;
let resolveResult;
try {
resolved = resolvePackageTarget(
resolveResult = resolvePackageTarget(
packageJSONUrl, targetItem, subpath, packageSubpath, base, pattern,
internal, conditions);
} catch (e) {
Expand All @@ -542,13 +563,13 @@ function resolvePackageTarget(packageJSONUrl, target, subpath, packageSubpath,
continue;
throw e;
}
if (resolved === undefined)
if (resolveResult === undefined)
continue;
if (resolved === null) {
if (resolveResult === null) {
lastException = null;
continue;
}
return resolved;
return resolveResult;
}
if (lastException === undefined || lastException === null)
return lastException;
Expand All @@ -567,12 +588,12 @@ function resolvePackageTarget(packageJSONUrl, target, subpath, packageSubpath,
const key = keys[i];
if (key === 'default' || conditions.has(key)) {
const conditionalTarget = target[key];
const resolved = resolvePackageTarget(
const resolveResult = resolvePackageTarget(
packageJSONUrl, conditionalTarget, subpath, packageSubpath, base,
pattern, internal, conditions);
if (resolved === undefined)
if (resolveResult === undefined)
continue;
return resolved;
return resolveResult;
}
}
return undefined;
Expand Down Expand Up @@ -631,12 +652,15 @@ function packageExportsResolve(
!StringPrototypeIncludes(packageSubpath, '*') &&
!StringPrototypeEndsWith(packageSubpath, '/')) {
const target = exports[packageSubpath];
const resolved = resolvePackageTarget(
const resolveResult = resolvePackageTarget(
packageJSONUrl, target, '', packageSubpath, base, false, false, conditions
);
if (resolved === null || resolved === undefined)

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

return resolveResult;
}

let bestMatch = '';
Expand Down Expand Up @@ -672,12 +696,20 @@ function packageExportsResolve(

if (bestMatch) {
const target = exports[bestMatch];
const resolved = resolvePackageTarget(packageJSONUrl, target,
bestMatchSubpath, bestMatch, base,
true, false, conditions);
if (resolved === null || resolved === undefined)
const resolveResult = resolvePackageTarget(
packageJSONUrl,
target,
bestMatchSubpath,
bestMatch,
base,
true,
false,
conditions);

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

throwExportsNotFound(packageSubpath, packageJSONUrl, base);
Expand Down Expand Up @@ -717,11 +749,12 @@ function packageImportsResolve(name, base, conditions) {
if (imports) {
if (ObjectPrototypeHasOwnProperty(imports, name) &&
!StringPrototypeIncludes(name, '*')) {
const resolved = resolvePackageTarget(
const resolveResult = resolvePackageTarget(
packageJSONUrl, imports[name], '', name, base, false, true, conditions
);
if (resolved !== null && resolved !== undefined)
return resolved;
if (resolveResult != null) {
return resolveResult.resolved;
}
} else {
let bestMatch = '';
let bestMatchSubpath;
Expand All @@ -747,11 +780,13 @@ function packageImportsResolve(name, base, conditions) {

if (bestMatch) {
const target = imports[bestMatch];
const resolved = resolvePackageTarget(packageJSONUrl, target,
bestMatchSubpath, bestMatch,
base, true, true, conditions);
if (resolved !== null && resolved !== undefined)
return resolved;
const resolveResult = resolvePackageTarget(packageJSONUrl, target,
bestMatchSubpath,
bestMatch, base, true,
true, conditions);
if (resolveResult != null) {
return resolveResult.resolved;
}
}
}
}
Expand Down Expand Up @@ -810,11 +845,11 @@ function parsePackageName(specifier, base) {
* @param {string} specifier
* @param {string | URL | undefined} base
* @param {Set<string>} conditions
* @returns {URL}
* @returns {resolved: URL, format? : string}
*/
function packageResolve(specifier, base, conditions) {
if (NativeModule.canBeRequiredByUsers(specifier))
return new URL('node:' + specifier);
return { resolved: new URL('node:' + specifier) };

const { packageName, packageSubpath, isScoped } =
parsePackageName(specifier, base);
Expand All @@ -826,8 +861,7 @@ 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);
}
}

Expand All @@ -849,13 +883,24 @@ function packageResolve(specifier, base, conditions) {

// Package match.
const packageConfig = getPackageConfig(packageJSONPath, specifier, base);
if (packageConfig.exports !== undefined && packageConfig.exports !== null)
if (packageConfig.exports !== undefined && packageConfig.exports !== null) {
return packageExportsResolve(
packageJSONUrl, packageSubpath, packageConfig, base, conditions
);
if (packageSubpath === '.')
return legacyMainResolve(packageJSONUrl, packageConfig, base);
return new URL(packageSubpath, packageJSONUrl);
packageJSONUrl, packageSubpath, packageConfig, base, conditions);
}
if (packageSubpath === '.') {
return {
resolved: legacyMainResolve(
packageJSONUrl,
packageConfig,
base),
...(packageConfig.type !== 'none') && { format: packageConfig.type }
};
}

return {
resolved: new URL(packageSubpath, packageJSONUrl),
...(packageConfig.type !== 'none') && { format: packageConfig.type }
};
// Cross-platform root check.
} while (packageJSONPath.length !== lastPath.length);

Expand Down Expand Up @@ -893,12 +938,13 @@ function shouldBeTreatedAsRelativeOrAbsolutePath(specifier) {
* @param {string | URL | undefined} base
* @param {Set<string>} conditions
* @param {boolean} preserveSymlinks
* @returns {URL}
* @returns {url: URL, format?: string}
*/
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 @@ -907,12 +953,19 @@ function moduleResolve(specifier, base, conditions, preserveSymlinks) {
try {
resolved = new URL(specifier);
} catch {
resolved = packageResolve(specifier, base, conditions);
({ resolved, format } = packageResolve(specifier, base, conditions));
}
}
if (resolved.protocol !== 'file:')
return resolved;
return finalizeResolution(resolved, base, preserveSymlinks);
if (resolved.protocol !== 'file:') {
return {
url: resolved
};
}

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

/**
Expand Down Expand Up @@ -1001,9 +1054,15 @@ function defaultResolve(specifier, context = {}, defaultResolveUnused) {

conditions = getConditionsSet(conditions);
let url;
let format;
try {
url = moduleResolve(specifier, parentURL, conditions,
isMain ? preserveSymlinksMain : preserveSymlinks);
({ url, format } =
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 Down Expand Up @@ -1031,7 +1090,10 @@ function defaultResolve(specifier, context = {}, defaultResolveUnused) {
url.protocol !== 'node:')
throw new ERR_UNSUPPORTED_ESM_URL_SCHEME(url);

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

module.exports = {
Expand Down
41 changes: 41 additions & 0 deletions test/es-module/test-esm-loader-resolve-type.mjs
View file
@@ -0,0 +1,41 @@
// Flags: --loader ./test/fixtures/es-module-loaders/hook-resolve-type.mjs
import { allowGlobals } from '../common/index.mjs';
import * as fixtures from '../common/fixtures.mjs';
import { strict as assert } from 'assert';
import * as fs from 'fs';

allowGlobals(global.getModuleTypeStats);

const basePath =
new URL('./node_modules/', import.meta.url);

const rel = (file) => new URL(file, basePath);
const createDir = (path) => {
if (!fs.existsSync(path)) {
fs.mkdirSync(path);
}
};

const moduleName = 'module-counter-by-type';

const moduleDir = rel(`${moduleName}`);
createDir(basePath);
createDir(moduleDir);
fs.cpSync(
fixtures.path('es-modules', moduleName),
moduleDir,
{ recursive: true }
);

const { importedESM: importedESMBefore,
importedCJS: importedCJSBefore } = global.getModuleTypeStats();

import(`${moduleName}`).finally(() => {
fs.rmSync(basePath, { recursive: true, force: true });
});

const { importedESM: importedESMAfter,
importedCJS: importedCJSAfter } = global.getModuleTypeStats();

assert.strictEqual(importedESMBefore + 1, importedESMAfter);
assert.strictEqual(importedCJSBefore, importedCJSAfter);

0 comments on commit 6d8eb6a

Please sign in to comment.