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

require.resolve with options.paths + require.resolve.paths #6471

Merged
merged 5 commits into from
Jul 10, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
## master

### Features

- `[jest-runtime]` Support `require.resolve.paths` ([#6471](https://github.com/facebook/jest/pull/6471))
- `[jest-runtime]` Support `paths` option for `require.resolve` ([#6471](https://github.com/facebook/jest/pull/6471))

### Fixes

- `[jest-runner]` Force parallel runs for watch mode, to avoid TTY freeze ([#6647](https://github.com/facebook/jest/pull/6647))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ exports[`moduleNameMapper wrong configuration 1`] = `
12 | module.exports = () => 'test';
13 |

at packages/jest-resolve/build/index.js:400:17
at packages/jest-resolve/build/index.js:411:17
at index.js:10:1

"
Expand Down
16 changes: 16 additions & 0 deletions e2e/__tests__/resolve-get-paths.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/
'use strict';

const runJest = require('../runJest');

test('require.resolve.paths', () => {
const {status} = runJest('resolve-get-paths');
expect(status).toBe(0);
});
36 changes: 36 additions & 0 deletions e2e/__tests__/resolve-with-paths.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/**
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/
'use strict';

const {resolve} = require('path');

const runJest = require('../runJest');
const {writeFiles, cleanup} = require('../Utils');

const workdirNodeModules = resolve(
__dirname,
'..',
'resolve-with-paths',
'node_modules',
);

beforeAll(() => {
writeFiles(resolve(workdirNodeModules, 'mod'), {
'index.js': 'module.exports = 42;',
});
});

afterAll(() => {
cleanup(workdirNodeModules);
});

test('require.resolve with paths', () => {
const {status} = runJest('resolve-with-paths');
expect(status).toBe(0);
});
24 changes: 24 additions & 0 deletions e2e/resolve-get-paths/__tests__/resolve-get-paths.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
'use strict';

import {resolve} from 'path';

test('returns the resolve path for a relative path', () => {
expect(require.resolve.paths('./mod.js')).toEqual([resolve(__dirname)]);
});

test('returns the resolve paths for a node_module', () => {
expect(require.resolve.paths('mod').slice(0, 2)).toEqual([
resolve(__dirname, 'node_modules'),
resolve(__dirname, '..', 'node_modules'),
]);
});

test('returns null for a native node module', () => {
expect(require.resolve.paths('fs')).toBeNull();
});
5 changes: 5 additions & 0 deletions e2e/resolve-get-paths/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"jest": {
"testEnvironment": "node"
}
}
37 changes: 37 additions & 0 deletions e2e/resolve-with-paths/__tests__/resolve-with-paths.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/**
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
'use strict';

import {resolve} from 'path';

test('finds a module relative to one of the given paths', () => {
expect(require.resolve('./mod.js', {paths: ['../dir']})).toEqual(
resolve(__dirname, '..', 'dir', 'mod.js')
);
});

test('finds a module without a leading "./" relative to one of the given paths', () => {
expect(require.resolve('mod.js', {paths: ['../dir']})).toEqual(
resolve(__dirname, '..', 'dir', 'mod.js')
);
});

test('finds a node_module above one of the given paths', () => {
expect(require.resolve('mod', {paths: ['../dir']})).toEqual(
resolve(__dirname, '..', 'node_modules', 'mod', 'index.js')
);
});

test('finds a native node module when paths are given', () => {
expect(require.resolve('fs', {paths: ['../dir']})).toEqual('fs');
});

test('throws an error if the module cannot be found from given paths', () => {
expect(() => require.resolve('./mod.js', {paths: ['..']})).toThrowError(
"Cannot resolve module './mod.js' from paths ['..'] from "
);
});
8 changes: 8 additions & 0 deletions e2e/resolve-with-paths/dir/mod.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/**
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

module.exports = 'mod';
5 changes: 5 additions & 0 deletions e2e/resolve-with-paths/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"jest": {
"testEnvironment": "node"
}
}
14 changes: 14 additions & 0 deletions packages/jest-resolve/src/__tests__/resolve.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,20 @@ describe('resolveModule', () => {
require.resolve('../../src/__mocks__/foo/node_modules/dep/index.js'),
);
});

it('is possible to specify custom resolve paths', () => {
const resolver = new Resolver(moduleMap, {
extensions: ['.js'],
});
const src = require.resolve('../');
const resolved = resolver.resolveModule(src, 'mockJsDependency', {
paths: [
path.resolve(__dirname, '../../src/__tests__'),
path.resolve(__dirname, '../../src/__mocks__'),
],
});
expect(resolved).toBe(require.resolve('../__mocks__/mockJsDependency.js'));
});
});

describe('getMockModule', () => {
Expand Down
31 changes: 23 additions & 8 deletions packages/jest-resolve/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,12 @@ class Resolver {
return null;
}

resolveModule(
from: Path,
resolveModuleFromDirIfExists(
dirname: Path,
moduleName: string,
options?: ResolveModuleConfig,
): Path {
const dirname = path.dirname(from);
const paths = this._options.modulePaths;
): ?Path {
const paths = (options && options.paths) || this._options.modulePaths;
const moduleDirectory = this._options.moduleDirectories;
const key = dirname + path.delimiter + moduleName;
const defaultPlatform = this._options.defaultPlatform;
Expand Down Expand Up @@ -187,9 +186,25 @@ class Resolver {
} catch (ignoredError) {}
}

// 4. Throw an error if the module could not be found. `resolve.sync`
// only produces an error based on the dirname but we have the actual
// current module name available.
return null;
}

resolveModule(
from: Path,
moduleName: string,
options?: ResolveModuleConfig,
): Path {
const dirname = path.dirname(from);
const module = this.resolveModuleFromDirIfExists(
dirname,
moduleName,
options,
);
if (module) return module;

// (4.) Throw an error if the module could not be found. `resolve.sync`
// only produces an error based on the dirname but we have the actual
// current module name available.
const relativePath = path.relative(dirname, from);
const err = new Error(
`Cannot find module '${moduleName}' from '${relativePath || '.'}'`,
Expand Down
61 changes: 59 additions & 2 deletions packages/jest-runtime/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,61 @@ class Runtime {
return to ? this._resolver.resolveModule(from, to) : from;
}

_requireResolve(
from: Path,
moduleName?: string,
{paths}: {paths?: Path[]} = {},
) {
if (moduleName == null) {
throw new Error(
'The first argument to require.resolve must be a string. Received null or undefined.',
);
}

if (paths) {
for (const p of paths) {
const absolutePath = path.resolve(from, '..', p);
const module = this._resolver.resolveModuleFromDirIfExists(
absolutePath,
moduleName,
// required to also resolve files without leading './' directly in the path
{paths: [absolutePath]},
);
if (module) {
return module;
}
}
throw new Error(
`Cannot resolve module '${moduleName}' from paths ['${paths.join(
"', '",
)}'] from ${from}`,
);
}

return this._resolveModule(from, moduleName);
}

_requireResolvePaths(from: Path, moduleName?: string) {
if (moduleName == null) {
throw new Error(
'The first argument to require.resolve.paths must be a string. Received null or undefined.',
);
}
if (!moduleName.length) {
throw new Error(
'The first argument to require.resolve.paths must not be the empty string.',
);
}

if (moduleName[0] === '.') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

.startsWith(0)? Whatever is faster on latest V8

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally forgot that function exists, would be cleaner for sure.
Not sure if performance is that critical here (how often would a user call require.resolve.paths?) but I can try to find/run a benchmark later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay so if you run it a lot, [0] === '.' is many times faster than .startsWith('.').
I think because of this and because the startsWith only really simplifies the code a lot when called with a multi character string, we can leave it this way.

Copy link
Contributor

Choose a reason for hiding this comment

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

When did V8 define string access via bracket notation? If it works on Node 6, then we're fine with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CI definitely passed on Node 6 last time.
I also think I've seen this being done somewhere else in the codebase already, let me check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here from a few years ago, so this should not cause any Node version issues.

return [path.resolve(from, '..')];
}
if (this._resolver.isCoreModule(moduleName)) {
return null;
}
return this._resolver.getModulePaths(path.resolve(from, '..'));
}

_execModule(
localModule: Module,
options: ?InternalModuleOptions,
Expand Down Expand Up @@ -722,8 +777,10 @@ class Runtime {
moduleRequire.extensions = Object.create(null);
moduleRequire.requireActual = this.requireModule.bind(this, from.filename);
moduleRequire.requireMock = this.requireMock.bind(this, from.filename);
moduleRequire.resolve = moduleName =>
this._resolveModule(from.filename, moduleName);
moduleRequire.resolve = (moduleName, options) =>
this._requireResolve(from.filename, moduleName, options);
moduleRequire.resolve.paths = moduleName =>
this._requireResolvePaths(from.filename, moduleName);
Object.defineProperty(
moduleRequire,
'main',
Expand Down
3 changes: 3 additions & 0 deletions types/Resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@

import type _Resolver from 'jest-resolve';

import type {Path} from './Config';

export type ResolveModuleConfig = {|
skipNodeResolution?: boolean,
paths?: Path[],
|};

export type Resolver = _Resolver;