From dec768394f750ca96c932d23a2e7cfe164eec866 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Thu, 17 Feb 2022 15:27:41 +0100 Subject: [PATCH] fix: correctly resolve remapped directories (#12373) --- CHANGELOG.md | 2 +- .../runtimeInternalModuleRegistry.test.js | 6 +- .../node_modules/exports/package.json | 3 +- .../exports/some-other-directory/file.js | 0 .../src/__tests__/resolve.test.ts | 14 +++ packages/jest-resolve/src/defaultResolver.ts | 114 +++++++++--------- .../src/__tests__/leak-integration.test.ts | 6 +- 7 files changed, 83 insertions(+), 62 deletions(-) create mode 100644 packages/jest-resolve/src/__mocks__/conditions/node_modules/exports/some-other-directory/file.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 67d8a7cd9fc5..8b0ea9c4eec4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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)) diff --git a/e2e/runtime-internal-module-registry/__tests__/runtimeInternalModuleRegistry.test.js b/e2e/runtime-internal-module-registry/__tests__/runtimeInternalModuleRegistry.test.js index 23d7ba831933..53334daccdb5 100644 --- a/e2e/runtime-internal-module-registry/__tests__/runtimeInternalModuleRegistry.test.js +++ b/e2e/runtime-internal-module-registry/__tests__/runtimeInternalModuleRegistry.test.js @@ -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 diff --git a/packages/jest-resolve/src/__mocks__/conditions/node_modules/exports/package.json b/packages/jest-resolve/src/__mocks__/conditions/node_modules/exports/package.json index 7fd4bab2a661..fc9039934b2c 100644 --- a/packages/jest-resolve/src/__mocks__/conditions/node_modules/exports/package.json +++ b/packages/jest-resolve/src/__mocks__/conditions/node_modules/exports/package.json @@ -11,6 +11,7 @@ "./deeplyNested" : { "require": "./nestedRequire.js", "default": "./nestedDefault.js" - } + }, + "./directory/*": "./some-other-directory/*" } } diff --git a/packages/jest-resolve/src/__mocks__/conditions/node_modules/exports/some-other-directory/file.js b/packages/jest-resolve/src/__mocks__/conditions/node_modules/exports/some-other-directory/file.js new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/packages/jest-resolve/src/__tests__/resolve.test.ts b/packages/jest-resolve/src/__tests__/resolve.test.ts index 6c6dfaf0705e..4d0b1af072a6 100644 --- a/packages/jest-resolve/src/__tests__/resolve.test.ts +++ b/packages/jest-resolve/src/__tests__/resolve.test.ts @@ -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', + ), + ); + }); }); }); diff --git a/packages/jest-resolve/src/defaultResolver.ts b/packages/jest-resolve/src/defaultResolver.ts index 1ca6204d0b6d..893a18b1b3bf 100644 --- a/packages/jest-resolve/src/defaultResolver.ts +++ b/packages/jest-resolve/src/defaultResolver.ts @@ -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, @@ -36,6 +35,9 @@ interface ResolverOptions { pathFilter?: (pkg: PkgJson, path: string, relativePath: string) => string; } +type UpstreamResolveOptionsWithConditions = UpstreamResolveOptions & + Pick; + // https://github.com/facebook/jest/pull/10617 declare global { namespace NodeJS { @@ -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. @@ -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, - 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 | 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 diff --git a/packages/jest-worker/src/__tests__/leak-integration.test.ts b/packages/jest-worker/src/__tests__/leak-integration.test.ts index 3acfa58be8df..f8e440e2d904 100644 --- a/packages/jest-worker/src/__tests__/leak-integration.test.ts +++ b/packages/jest-worker/src/__tests__/leak-integration.test.ts @@ -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(() => { @@ -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);