Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: correctly resolve remapped directories #12373

Merged
merged 15 commits into from
Feb 17, 2022
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
- `[jest-environment-node]` [**BREAKING**] Add default `node` and `node-addon` conditions to `exportConditions` for `node` environment ([#11924](https://github.com/facebook/jest/pull/11924))
- `[@jest/expect]` New module which extends `expect` with `jest-snapshot` matchers ([#12404](https://github.com/facebook/jest/pull/12404), [#12410](https://github.com/facebook/jest/pull/12410), [#12418](https://github.com/facebook/jest/pull/12418))
- `[@jest/expect-utils]` New module exporting utils for `expect` ([#12323](https://github.com/facebook/jest/pull/12323))
- `[jest-resolver]` [**BREAKING**] Add support for `package.json` `exports` ([11961](https://github.com/facebook/jest/pull/11961))
- `[jest-resolve]` [**BREAKING**] Add support for `package.json` `exports` ([#11961](https://github.com/facebook/jest/pull/11961), [#12373](https://github.com/facebook/jest/pull/12373))
- `[jest-resolve, jest-runtime]` Add support for `data:` URI import and mock ([#12392](https://github.com/facebook/jest/pull/12392))
- `[@jest/schemas]` New module for JSON schemas for Jest's config ([#12384](https://github.com/facebook/jest/pull/12384))
- `[jest-worker]` [**BREAKING**] Allow only absolute `workerPath` ([#12343](https://github.com/facebook/jest/pull/12343))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ describe('Runtime internal module registry', () => {
it('behaves correctly when requiring a module that is used by jest internals', () => {
const fs = require('fs');

// We require from this crazy path so that we can mimick Jest (and it's
// transitive deps) being installed along side a projects deps (e.g. with an
// We require from this crazy path so that we can mimick Jest (and its
// transitive deps) being installed alongside a projects deps (e.g. with an
// NPM3 flat dep tree)
const jestUtil = require('../../../packages/jest-util');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I honestly don't understand this test 🙈 Comes from #1572

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment above doesn't make much sense, you're not simulating a real environment by using a relative path to get into some module

const jestUtil = require('jest-util');

// If FS is mocked correctly, this folder won't actually be created on the
// filesystem
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions packages/jest-resolve/src/__tests__/resolve.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,20 @@ describe('findNodeModule', () => {
path.resolve(conditionsRoot, './node_modules/exports/nestedDefault.js'),
);
});

test('supports separate directory path', () => {
const result = Resolver.findNodeModule('exports/directory/file.js', {
basedir: conditionsRoot,
conditions: [],
});

expect(result).toEqual(
path.resolve(
conditionsRoot,
'./node_modules/exports/some-other-directory/file.js',
),
);
});
});
});

Expand Down
135 changes: 91 additions & 44 deletions packages/jest-resolve/src/defaultResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@
* LICENSE file in the root directory of this source tree.
*/

import {isAbsolute} from 'path';
import {dirname, isAbsolute, resolve as pathResolve} from 'path';
import pnpResolver from 'jest-pnp-resolver';
import {sync as resolveSync} from 'resolve';
import {SyncOpts as UpstreamResolveOptions, sync as resolveSync} from 'resolve';
import {
Options as ResolveExportsOptions,
resolve as resolveExports,
} from 'resolve.exports';
import slash = require('slash');
import {
PkgJson,
isDirectory,
Expand All @@ -36,6 +35,9 @@ interface ResolverOptions {
pathFilter?: (pkg: PkgJson, path: string, relativePath: string) => string;
}

type UpstreamResolveOptionsWithConditions = UpstreamResolveOptions &
Pick<ResolverOptions, 'conditions'>;

// https://github.com/facebook/jest/pull/10617
declare global {
namespace NodeJS {
Expand All @@ -45,6 +47,58 @@ declare global {
}
}

function getPathInModule(
path: string,
options: UpstreamResolveOptionsWithConditions,
): string {
if (isAbsolute(path) || path.startsWith('.')) {
return path;
}

const segments = path.split('/');

let moduleName = segments.shift();

if (moduleName) {
// TODO: handle `#` here: https://github.com/facebook/jest/issues/12270
if (moduleName.startsWith('@')) {
moduleName = `${moduleName}/${segments.shift()}`;
}

let packageJsonPath = '';

try {
packageJsonPath = resolveSync(`${moduleName}/package.json`, options);
} catch {
// ignore if package.json cannot be found
}

if (packageJsonPath && isFile(packageJsonPath)) {
const pkg = readPackageCached(packageJsonPath);

if (pkg.exports) {
// we need to make sure resolve ignores `main`
delete pkg.main;

const subpath = segments.join('/') || '.';

const resolved = resolveExports(
pkg,
subpath,
createResolveOptions(options.conditions),
);

// TODO: should we throw if not?
if (resolved) {
return pathResolve(dirname(packageJsonPath), resolved);
}
}
}
}

return path;
}

export default function defaultResolver(
path: string,
options: ResolverOptions,
Expand All @@ -55,16 +109,23 @@ export default function defaultResolver(
return pnpResolver(path, options);
}

const result = resolveSync(path, {
const resolveOptions: UpstreamResolveOptionsWithConditions = {
...options,
isDirectory,
isFile,
packageFilter: createPackageFilter(path, options.packageFilter),
pathFilter: createPathFilter(path, options.conditions, options.pathFilter),
preserveSymlinks: false,
readPackageSync,
realpathSync,
});
};

const pathToResolve = getPathInModule(path, resolveOptions);

const result = isAbsolute(pathToResolve)
? pathToResolve
: resolveSync(pathToResolve, {
...resolveOptions,
packageFilter: createPackageFilter(pathToResolve, options),
});

// Dereference symlinks to ensure we don't create a separate
// module instance depending on how it was referenced.
Expand All @@ -81,65 +142,51 @@ function readPackageSync(_: unknown, file: string): PkgJson {

function createPackageFilter(
originalPath: string,
userFilter?: ResolverOptions['packageFilter'],
options: ResolverOptions,
): ResolverOptions['packageFilter'] {
if (shouldIgnoreRequestForExports(originalPath)) {
return userFilter;
return options.packageFilter;
}

return function packageFilter(pkg, ...rest) {
let filteredPkg = pkg;

if (userFilter) {
filteredPkg = userFilter(filteredPkg, ...rest);
if (options.packageFilter) {
filteredPkg = options.packageFilter(filteredPkg, ...rest);
}

if (filteredPkg.exports == null) {
return filteredPkg;
}

let resolvedMain: string | void = undefined;

try {
resolvedMain = resolveExports(
filteredPkg,
'.',
createResolveOptions(options.conditions),
);
} catch {
// ignore
}

return {
...filteredPkg,
// remove `main` so `resolve` doesn't look at it and confuse the `.`
// loading in `pathFilter`
main: undefined,
// override `main` so `resolve` resolves it correctly while respecting
// `exports`.
main: resolvedMain,
};
};
}

function createPathFilter(
originalPath: string,
conditions?: Array<string>,
userFilter?: ResolverOptions['pathFilter'],
): ResolverOptions['pathFilter'] {
if (shouldIgnoreRequestForExports(originalPath)) {
return userFilter;
}

const options: ResolveExportsOptions = conditions
function createResolveOptions(
conditions: Array<string> | undefined,
): ResolveExportsOptions {
return conditions
? {conditions, unsafe: true}
: // no conditions were passed - let's assume this is Jest internal and it should be `require`
{browser: false, require: true};

return function pathFilter(pkg, path, relativePath, ...rest) {
let pathToUse = relativePath;

if (userFilter) {
pathToUse = userFilter(pkg, path, relativePath, ...rest);
}

if (pkg.exports == null) {
return pathToUse;
}

// this `index` thing can backfire, but `resolve` adds it: https://github.com/browserify/resolve/blob/f1b51848ecb7f56f77bfb823511d032489a13eab/lib/sync.js#L192
const isRootRequire =
pathToUse === 'index' && !originalPath.endsWith('/index');

const newPath = isRootRequire ? '.' : slash(pathToUse);

return resolveExports(pkg, newPath, options) || pathToUse;
};
}

// if it's a relative import or an absolute path, exports are ignored
Expand Down