Skip to content

Commit ac09860

Browse files
gaearoncpojer
authored andcommitted
Fix NODE_PATH resolving for relative paths (jestjs#3616)
* Add failing integration test for relative NODE_PATH It is failing because babel-jest traverses directories up searching for package.json, but the path is relative. It is exposing the underlying issue that NODE_PATH can be relative, but the code assumes all paths are absolute by that point. * Resolve NODE_PATH to absolute paths This fixes the issue because the rest of the code assumes the path is already absolute. We also resolve symlink if necessary. * Fix unrelated test to match the code This test started failing, but it was not testing NODE_PATH in the first place. It just happened to include its output, and it's not empty on Travis. The test used to have some copy pasta from how we calculate nodePaths, so I just synced that copy pasta. * Nitty nit * Exclude empty items from the array They are empty if NODE_PATH is empty, and split() returns ['']. * fs.realpathSync() can return undefined * Fix test to match last change
1 parent c0ffbbc commit ac09860

File tree

7 files changed

+75
-3
lines changed

7 files changed

+75
-3
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
3+
*
4+
* This source code is licensed under the BSD-style license found in the
5+
* LICENSE file in the root directory of this source tree. An additional grant
6+
* of patent rights can be found in the PATENTS file in the same directory.
7+
*
8+
* @emails oncall+jsinfra
9+
*/
10+
'use strict';
11+
12+
const runJest = require('../runJest');
13+
14+
test('supports NODE_PATH', () => {
15+
const result = runJest('node_path', [], {
16+
nodePath: ['../node_path/src'],
17+
});
18+
expect(result.status).toBe(0);
19+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/**
2+
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
3+
*
4+
* This source code is licensed under the BSD-style license found in the
5+
* LICENSE file in the root directory of this source tree. An additional grant
6+
* of patent rights can be found in the PATENTS file in the same directory.
7+
*/
8+
'use strict';
9+
10+
test('can require by absolute path', () => {
11+
expect(require('path/file.js')).toBe(42);
12+
});
+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"jest": {
3+
"testEnvironment": "node",
4+
"transform": {
5+
"^.+\\.jsx?$": "../../packages/babel-jest"
6+
}
7+
}
8+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
/**
2+
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
3+
*
4+
* This source code is licensed under the BSD-style license found in the
5+
* LICENSE file in the root directory of this source tree. An additional grant
6+
* of patent rights can be found in the PATENTS file in the same directory.
7+
*/
8+
'use strict';
9+
10+
module.exports = 42;

integration_tests/runJest.js

+10-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ const {fileExists} = require('./utils');
1616
const JEST_PATH = path.resolve(__dirname, '../packages/jest-cli/bin/jest.js');
1717

1818
type RunJestOptions = {
19+
nodePath?: string,
1920
skipPkgJsonCheck?: boolean, // don't complain if can't find package.json
2021
};
2122

@@ -45,7 +46,15 @@ function runJest(
4546
);
4647
}
4748

48-
const result = spawnSync(JEST_PATH, args || [], {cwd: dir});
49+
const env = options.nodePath
50+
? Object.assign({}, process.env, {
51+
NODE_PATH: options.nodePath,
52+
})
53+
: process.env;
54+
const result = spawnSync(JEST_PATH, args || [], {
55+
cwd: dir,
56+
env,
57+
});
4958

5059
result.stdout = result.stdout && result.stdout.toString();
5160
result.stderr = result.stderr && result.stderr.toString();

packages/jest-resolve/src/__tests__/resolve-test.js

+7-1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
'use strict';
1212

13+
const fs = require('fs');
1314
const path = require('path');
1415
const ModuleMap = require('jest-haste-map').ModuleMap;
1516
const Resolver = require('../');
@@ -41,8 +42,13 @@ describe('isCoreModule', () => {
4142

4243
describe('findNodeModule', () => {
4344
it('is possible to override the default resolver', () => {
45+
const cwd = process.cwd();
46+
const resolvedCwd = fs.realpathSync(cwd) || cwd;
4447
const nodePaths = process.env.NODE_PATH
45-
? process.env.NODE_PATH.split(path.delimiter)
48+
? process.env.NODE_PATH
49+
.split(path.delimiter)
50+
.filter(Boolean)
51+
.map(p => path.resolve(resolvedCwd, p))
4652
: null;
4753

4854
jest.mock('../__mocks__/userResolver');

packages/jest-resolve/src/index.js

+9-1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import type {Path} from 'types/Config';
1212
import type {ModuleMap} from 'types/HasteMap';
1313

14+
const fs = require('fs');
1415
const path = require('path');
1516
const nodeModulesPaths = require('resolve/lib/node-modules-paths');
1617
const isBuiltinModule = require('is-builtin-module');
@@ -49,8 +50,15 @@ export type ResolveModuleConfig = {|
4950

5051
const NATIVE_PLATFORM = 'native';
5152

53+
// We might be inside a symlink.
54+
const cwd = process.cwd();
55+
const resolvedCwd = fs.realpathSync(cwd) || cwd;
5256
const nodePaths = process.env.NODE_PATH
53-
? process.env.NODE_PATH.split(path.delimiter)
57+
? process.env.NODE_PATH
58+
.split(path.delimiter)
59+
.filter(Boolean)
60+
// The resolver expects absolute paths.
61+
.map(p => path.resolve(resolvedCwd, p))
5462
: null;
5563

5664
class Resolver {

0 commit comments

Comments
 (0)