Skip to content

Commit

Permalink
fix: correctly resolve remapped directories (#12373)
Browse files Browse the repository at this point in the history
  • Loading branch information
SimenB committed Feb 17, 2022
1 parent bb09565 commit dec7683
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 62 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
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
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');
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
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
114 changes: 60 additions & 54 deletions packages/jest-resolve/src/defaultResolver.ts
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 @@ -55,16 +57,22 @@ 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 =
// if `getPathInModule` doesn't change the path, attempt to resolve it
pathToResolve === path
? resolveSync(pathToResolve, resolveOptions)
: pathToResolve;

// Dereference symlinks to ensure we don't create a separate
// module instance depending on how it was referenced.
Expand All @@ -79,67 +87,65 @@ function readPackageSync(_: unknown, file: string): PkgJson {
return readPackageCached(file);
}

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

return function packageFilter(pkg, ...rest) {
let filteredPkg = pkg;
const segments = path.split('/');

if (userFilter) {
filteredPkg = userFilter(filteredPkg, ...rest);
}
let moduleName = segments.shift();

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

return {
...filteredPkg,
// remove `main` so `resolve` doesn't look at it and confuse the `.`
// loading in `pathFilter`
main: undefined,
};
};
}
let packageJsonPath = '';

function createPathFilter(
originalPath: string,
conditions?: Array<string>,
userFilter?: ResolverOptions['pathFilter'],
): ResolverOptions['pathFilter'] {
if (shouldIgnoreRequestForExports(originalPath)) {
return userFilter;
}
try {
packageJsonPath = resolveSync(`${moduleName}/package.json`, options);
} catch {
// ignore if package.json cannot be found
}

const options: ResolveExportsOptions = conditions
? {conditions, unsafe: true}
: // no conditions were passed - let's assume this is Jest internal and it should be `require`
{browser: false, require: true};
if (packageJsonPath && isFile(packageJsonPath)) {
const pkg = readPackageCached(packageJsonPath);

return function pathFilter(pkg, path, relativePath, ...rest) {
let pathToUse = relativePath;
if (pkg.exports) {
// we need to make sure resolve ignores `main`
delete pkg.main;

if (userFilter) {
pathToUse = userFilter(pkg, path, relativePath, ...rest);
}
const subpath = segments.join('/') || '.';

if (pkg.exports == null) {
return pathToUse;
}
const resolved = resolveExports(
pkg,
subpath,
createResolveOptions(options.conditions),
);

// 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');
// TODO: should we throw if not?
if (resolved) {
return pathResolve(dirname(packageJsonPath), resolved);
}
}
}
}

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

return resolveExports(pkg, newPath, options) || pathToUse;
};
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};
}

// if it's a relative import or an absolute path, exports are ignored
Expand Down
6 changes: 3 additions & 3 deletions packages/jest-worker/src/__tests__/leak-integration.test.ts
Expand Up @@ -9,7 +9,7 @@ import {tmpdir} from 'os';
import {join} from 'path';
import {writeFileSync} from 'graceful-fs';
import LeakDetector from 'jest-leak-detector';
import {Worker} from '../..';
import {Worker} from '../../build/index';

let workerFile!: string;
beforeAll(() => {
Expand All @@ -30,10 +30,10 @@ afterEach(async () => {

it('does not retain arguments after a task finished', async () => {
let leakDetector!: LeakDetector;
await new Promise(resolve => {
await new Promise((resolve, reject) => {
const obj = {};
leakDetector = new LeakDetector(obj);
(worker as any).fn(obj).then(resolve);
(worker as any).fn(obj).then(resolve, reject);
});

expect(await leakDetector.isLeaking()).toBe(false);
Expand Down

0 comments on commit dec7683

Please sign in to comment.