From 29dd4d41af6f6cf81637cda58b14da6ed0753743 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 25 May 2017 09:58:16 +0100 Subject: [PATCH] Fix NODE_PATH resolving for relative paths (#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 --- integration_tests/__tests__/node_path-test.js | 19 +++++++++++++++++++ .../node_path/__tests__/node_path-test.js | 12 ++++++++++++ integration_tests/node_path/package.json | 8 ++++++++ integration_tests/node_path/src/path/file.js | 10 ++++++++++ integration_tests/runJest.js | 11 ++++++++++- .../src/__tests__/resolve-test.js | 8 +++++++- packages/jest-resolve/src/index.js | 10 +++++++++- 7 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 integration_tests/__tests__/node_path-test.js create mode 100644 integration_tests/node_path/__tests__/node_path-test.js create mode 100644 integration_tests/node_path/package.json create mode 100644 integration_tests/node_path/src/path/file.js diff --git a/integration_tests/__tests__/node_path-test.js b/integration_tests/__tests__/node_path-test.js new file mode 100644 index 000000000000..9933e60feb6b --- /dev/null +++ b/integration_tests/__tests__/node_path-test.js @@ -0,0 +1,19 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @emails oncall+jsinfra + */ +'use strict'; + +const runJest = require('../runJest'); + +test('supports NODE_PATH', () => { + const result = runJest('node_path', [], { + nodePath: ['../node_path/src'], + }); + expect(result.status).toBe(0); +}); diff --git a/integration_tests/node_path/__tests__/node_path-test.js b/integration_tests/node_path/__tests__/node_path-test.js new file mode 100644 index 000000000000..654a83571f46 --- /dev/null +++ b/integration_tests/node_path/__tests__/node_path-test.js @@ -0,0 +1,12 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ +'use strict'; + +test('can require by absolute path', () => { + expect(require('path/file.js')).toBe(42); +}); diff --git a/integration_tests/node_path/package.json b/integration_tests/node_path/package.json new file mode 100644 index 000000000000..ab33604cd518 --- /dev/null +++ b/integration_tests/node_path/package.json @@ -0,0 +1,8 @@ +{ + "jest": { + "testEnvironment": "node", + "transform": { + "^.+\\.jsx?$": "../../packages/babel-jest" + } + } +} diff --git a/integration_tests/node_path/src/path/file.js b/integration_tests/node_path/src/path/file.js new file mode 100644 index 000000000000..a04fdb41a5f4 --- /dev/null +++ b/integration_tests/node_path/src/path/file.js @@ -0,0 +1,10 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ +'use strict'; + +module.exports = 42; diff --git a/integration_tests/runJest.js b/integration_tests/runJest.js index 0d768a55d913..8fd4e1882d04 100644 --- a/integration_tests/runJest.js +++ b/integration_tests/runJest.js @@ -16,6 +16,7 @@ const {fileExists} = require('./utils'); const JEST_PATH = path.resolve(__dirname, '../packages/jest-cli/bin/jest.js'); type RunJestOptions = { + nodePath?: string, skipPkgJsonCheck?: boolean, // don't complain if can't find package.json }; @@ -45,7 +46,15 @@ function runJest( ); } - const result = spawnSync(JEST_PATH, args || [], {cwd: dir}); + const env = options.nodePath + ? Object.assign({}, process.env, { + NODE_PATH: options.nodePath, + }) + : process.env; + const result = spawnSync(JEST_PATH, args || [], { + cwd: dir, + env, + }); result.stdout = result.stdout && result.stdout.toString(); result.stderr = result.stderr && result.stderr.toString(); diff --git a/packages/jest-resolve/src/__tests__/resolve-test.js b/packages/jest-resolve/src/__tests__/resolve-test.js index 13d261bbe6de..4e8b03eae601 100644 --- a/packages/jest-resolve/src/__tests__/resolve-test.js +++ b/packages/jest-resolve/src/__tests__/resolve-test.js @@ -10,6 +10,7 @@ 'use strict'; +const fs = require('fs'); const path = require('path'); const ModuleMap = require('jest-haste-map').ModuleMap; const Resolver = require('../'); @@ -41,8 +42,13 @@ describe('isCoreModule', () => { describe('findNodeModule', () => { it('is possible to override the default resolver', () => { + const cwd = process.cwd(); + const resolvedCwd = fs.realpathSync(cwd) || cwd; const nodePaths = process.env.NODE_PATH - ? process.env.NODE_PATH.split(path.delimiter) + ? process.env.NODE_PATH + .split(path.delimiter) + .filter(Boolean) + .map(p => path.resolve(resolvedCwd, p)) : null; jest.mock('../__mocks__/userResolver'); diff --git a/packages/jest-resolve/src/index.js b/packages/jest-resolve/src/index.js index 61c968307ff8..f3fd87cabe1d 100644 --- a/packages/jest-resolve/src/index.js +++ b/packages/jest-resolve/src/index.js @@ -11,6 +11,7 @@ import type {Path} from 'types/Config'; import type {ModuleMap} from 'types/HasteMap'; +const fs = require('fs'); const path = require('path'); const nodeModulesPaths = require('resolve/lib/node-modules-paths'); const isBuiltinModule = require('is-builtin-module'); @@ -49,8 +50,15 @@ export type ResolveModuleConfig = {| const NATIVE_PLATFORM = 'native'; +// We might be inside a symlink. +const cwd = process.cwd(); +const resolvedCwd = fs.realpathSync(cwd) || cwd; const nodePaths = process.env.NODE_PATH - ? process.env.NODE_PATH.split(path.delimiter) + ? process.env.NODE_PATH + .split(path.delimiter) + .filter(Boolean) + // The resolver expects absolute paths. + .map(p => path.resolve(resolvedCwd, p)) : null; class Resolver {