Skip to content

Commit

Permalink
Log package configuration warnings from package exports resolution
Browse files Browse the repository at this point in the history
Summary:
- Make "exports" package validation load-bearing by implementing [*Invalid Package Configuration* error](https://nodejs.org/docs/latest-v19.x/api/esm.html#esm_resolver_algorithm_specification).
- Always catch error and output event to terminal/externally configured reporter using `context.unstable_logWarning`.

Changelog: **[Internal]**

Reviewed By: motiz88

Differential Revision: D43192757

fbshipit-source-id: 17e314d3a59589a49110a930ee612a76ffe7a2ed
  • Loading branch information
huntie authored and facebook-github-bot committed Feb 15, 2023
1 parent 0f7268f commit aa20356
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 30 deletions.
68 changes: 43 additions & 25 deletions packages/metro-resolver/src/PackageExportsResolve.js
Expand Up @@ -12,7 +12,7 @@
import type {ExportMap, ResolutionContext, SourceFileResolution} from './types';

import path from 'path';
import invariant from 'invariant';
import InvalidPackageConfigurationError from './errors/InvalidPackageConfigurationError';
import toPosixPath from './utils/toPosixPath';

/**
Expand All @@ -23,13 +23,16 @@ import toPosixPath from './utils/toPosixPath';
*
* Implements modern package resolution behaviour based on the [Package Entry
* Points spec](https://nodejs.org/docs/latest-v19.x/api/packages.html#package-entry-points).
*
* @throws {InvalidPackageConfigurationError} Raised if configuration specified
* by `exportsField` is invalid.
*/
export function resolvePackageTargetFromExports(
context: ResolutionContext,
/**
* The path to the containing npm package directory.
*/
packageRoot: string,
packagePath: string,
/**
* The unresolved absolute path to the target module. This will be converted
* to a package-relative subpath for comparison.
Expand All @@ -38,7 +41,15 @@ export function resolvePackageTargetFromExports(
exportsField: ExportMap | string,
platform: string | null,
): SourceFileResolution | null {
const packageSubpath = path.relative(packageRoot, modulePath);
const raiseConfigError = (reason: string) => {
throw new InvalidPackageConfigurationError({
reason,
modulePath,
packagePath,
});
};

const packageSubpath = path.relative(packagePath, modulePath);
const subpath =
// Convert to prefixed POSIX path for "exports" lookup
packageSubpath === '' ? '.' : './' + toPosixPath(packageSubpath);
Expand All @@ -47,16 +58,20 @@ export function resolvePackageTargetFromExports(
subpath,
exportsField,
platform,
raiseConfigError,
);

if (match != null) {
const filePath = path.join(packageRoot, match);
const filePath = path.join(packagePath, match);

if (context.doesFileExist(filePath)) {
return {type: 'sourceFile', filePath};
}
// TODO(T143882479): Throw InvalidPackageConfigurationError (entry point
// missing) and log as warning in calling context.

raiseConfigError(
`The resolution for "${modulePath}" defined in "exports" is ${filePath}, ` +
'however this file does not exist.',
);
}

return null;
Expand All @@ -77,6 +92,7 @@ function matchSubpathFromExports(
subpath: string,
exportsField: ExportMap | string,
platform: string | null,
raiseConfigError: (reason: string) => void,
): ?string {
const conditionNames = new Set([
'default',
Expand All @@ -86,7 +102,11 @@ function matchSubpathFromExports(
: []),
]);

const exportMap = reduceExportsField(exportsField, conditionNames);
const exportMap = reduceExportsField(
exportsField,
conditionNames,
raiseConfigError,
);

let match = exportMap[subpath];

Expand Down Expand Up @@ -123,13 +143,11 @@ type FlattenedExportMap = $ReadOnly<{[subpath: string]: string | null}>;
/**
* Reduce an "exports"-like field to a flat subpath mapping after resolving
* shorthand syntax and conditional exports.
*
* @throws Will throw invariant violation if structure or configuration
* specified by `exportsField` is invalid.
*/
function reduceExportsField(
exportsField: ExportMap | string,
conditionNames: $ReadOnlySet<string>,
raiseConfigError: (reason: string) => void,
): FlattenedExportMap {
let result: {[subpath: string]: string | null} = {};

Expand All @@ -141,13 +159,15 @@ function reduceExportsField(
subpathOrCondition.startsWith('.'),
);

// TODO(T143882479): Throw InvalidPackageConfigurationError and log as
// warning in calling context.
invariant(
subpathKeys.length === 0 || subpathKeys.length === firstLevelKeys.length,
'"exports" object cannot have keys mapping both subpaths and conditions ' +
'at the same level',
);
if (
subpathKeys.length !== 0 &&
subpathKeys.length !== firstLevelKeys.length
) {
raiseConfigError(
'The "exports" field cannot have keys which are both subpaths and ' +
'condition names at the same level',
);
}

let exportMap = exportsField;

Expand Down Expand Up @@ -175,14 +195,12 @@ function reduceExportsField(
value => value != null && !value.startsWith('./'),
);

// TODO(T143882479): Throw InvalidPackageConfigurationError and log as
// warning in calling context.
invariant(
invalidValues.length === 0,
'One or more mappings for subpaths in "exports" is invalid. All values ' +
'must begin with "./": ' +
JSON.stringify(invalidValues),
);
if (invalidValues.length) {
raiseConfigError(
'One or more mappings for subpaths defined in "exports" are invalid. ' +
'All values must begin with "./".',
);
}

return result;
}
Expand Down
18 changes: 14 additions & 4 deletions packages/metro-resolver/src/__tests__/package-exports-test.js
Expand Up @@ -123,6 +123,7 @@ describe('with package exports resolution enabled', () => {
});

test('[nonstrict] should fall back to "main" field resolution when file does not exist', () => {
const logWarning = jest.fn();
const context = {
...baseContext,
...createPackageAccessors({
Expand All @@ -131,17 +132,22 @@ describe('with package exports resolution enabled', () => {
exports: './foo.js',
},
}),
unstable_logWarning: logWarning,
};

expect(Resolver.resolve(context, 'test-pkg', null)).toEqual({
type: 'sourceFile',
filePath: '/root/node_modules/test-pkg/index-main.js',
});
// TODO(T142200031): Assert that an invalid package warning is logged with
// file missing message
expect(logWarning).toHaveBeenCalledTimes(1);
expect(logWarning.mock.calls[0][0]).toMatchInlineSnapshot(`
"The package /root/node_modules/test-pkg contains an invalid package.json configuration. Consider raising this issue with the package maintainer(s).
Reason: The resolution for \\"/root/node_modules/test-pkg\\" defined in \\"exports\\" is /root/node_modules/test-pkg/foo.js, however this file does not exist."
`);
});

test('[nonstrict] should fall back to "main" field resolution when "exports" is an invalid subpath', () => {
const logWarning = jest.fn();
const context = {
...baseContext,
...createPackageAccessors({
Expand All @@ -150,14 +156,18 @@ describe('with package exports resolution enabled', () => {
exports: 'index.js',
},
}),
unstable_logWarning: logWarning,
};

expect(Resolver.resolve(context, 'test-pkg', null)).toEqual({
type: 'sourceFile',
filePath: '/root/node_modules/test-pkg/index-main.js',
});
// TODO(T142200031): Assert that an invalid package warning is logged with
// invalid subpath value message
expect(logWarning).toHaveBeenCalledTimes(1);
expect(logWarning.mock.calls[0][0]).toMatchInlineSnapshot(`
"The package /root/node_modules/test-pkg contains an invalid package.json configuration. Consider raising this issue with the package maintainer(s).
Reason: One or more mappings for subpaths defined in \\"exports\\" are invalid. All values must begin with \\"./\\"."
`);
});

describe('should resolve "exports" target directly', () => {
Expand Down
@@ -0,0 +1,46 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow strict
* @format
* @oncall react_native
*/

/**
* Raised when a package contains an invalid `package.json` configuration.
*/
export default class InvalidPackageConfigurationError extends Error {
/**
* The description of the error cause.
*/
reason: string;

/**
* The module path or import specifier attempted.
*/
modulePath: string;

/**
* Absolute path of the package being resolved.
*/
packagePath: string;

constructor(
opts: $ReadOnly<{
reason: string,
modulePath: string,
packagePath: string,
}>,
) {
super(
`The package ${opts.packagePath} contains an invalid package.json ` +
'configuration. Consider raising this issue with the package ' +
'maintainer(s).\nReason: ' +
opts.reason,
);
Object.assign(this, opts);
}
}
7 changes: 6 additions & 1 deletion packages/metro-resolver/src/resolve.js
Expand Up @@ -23,6 +23,7 @@ import isAbsolutePath from 'absolute-path';
import path from 'path';
import FailedToResolveNameError from './errors/FailedToResolveNameError';
import FailedToResolvePathError from './errors/FailedToResolvePathError';
import InvalidPackageConfigurationError from './errors/InvalidPackageConfigurationError';
import InvalidPackageError from './errors/InvalidPackageError';
import formatFileCandidates from './errors/formatFileCandidates';
import {getPackageEntryPoint} from './PackageResolve';
Expand Down Expand Up @@ -259,7 +260,11 @@ function resolvePackage(
return resolvedAs(packageExportsResult);
}
} catch (e) {
// TODO(T143882479): Log invalid package configuration warning
if (e instanceof InvalidPackageConfigurationError) {
context.unstable_logWarning(e.message);
} else {
throw e;
}
}
}
}
Expand Down

0 comments on commit aa20356

Please sign in to comment.