fix dependency name clash bug (jestjs#6687)

* dependency name clash

* Update changelog

CHANGELOG.md

@@ -9,6 +9,7 @@
 ### Features
 
 - `[jest-cli]` Watch plugins now have access to a broader range of global configuration options in their `updateConfigAndRun` callbacks, so they can provide a wider set of extra features ([#6473](https://github.com/facebook/jest/pull/6473))
+- `[jest-snapshot]` `babel-traverse` is now passed to `jest-snapshot` explicitly to avoid unnecessary requires in every test
 
 ### Fixes
 

e2e/Utils.js

@@ -42,7 +42,7 @@ const run = (cmd: string, cwd?: Path) => {
 const linkJestPackage = (packageName: string, cwd: Path) => {
   const packagesDir = path.resolve(__dirname, '../packages');
   const packagePath = path.resolve(packagesDir, packageName);
-  const destination = path.resolve(cwd, 'node_modules/');
+  const destination = path.resolve(cwd, 'node_modules/', packageName);
   mkdirp.sync(destination);
   rimraf.sync(destination);
   fs.symlinkSync(packagePath, destination, 'dir');

e2e/__tests__/dependency_clash.test.js

@@ -0,0 +1,83 @@
+/**
+ * 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
+ */
+
+const path = require('path');
+const {
+  cleanup,
+  createEmptyPackage,
+  linkJestPackage,
+  writeFiles,
+} = require('../Utils');
+const runJest = require('../runJest');
+const os = require('os');
+const mkdirp = require('mkdirp');
+const fs = require('fs');
+const ConditionalTest = require('../../scripts/ConditionalTest');
+
+ConditionalTest.skipSuiteOnWindows();
+
+// doing test in a temp directory because we don't want jest node_modules affect it
+const tempDir = path.resolve(os.tmpdir(), 'clashing-dependencies-test');
+const thirdPartyDir = path.resolve(tempDir, 'third-party');
+
+beforeEach(() => {
+  cleanup(tempDir);
+  createEmptyPackage(tempDir);
+  mkdirp(path.join(thirdPartyDir, 'node_modules'));
+  linkJestPackage('babel-jest', thirdPartyDir);
+});
+
+// This test case is checking that when having both
+// `invariant` package from npm and `invariant.js` that provides `invariant`
+// module we can still require the right invariant. This is pretty specific
+// use case and in the future we should probably delete this test.
+// see: https://github.com/facebook/jest/pull/6687
+test('fails with syntax error on flow types', () => {
+  const babelFileThatRequiresInvariant = require.resolve(
+    'babel-traverse/lib/path/index.js',
+  );
+
+  expect(fs.existsSync(babelFileThatRequiresInvariant)).toBe(true);
+  // make sure the babel depenency that depends on `invariant` from npm still
+  // exists, otherwise the test will pass regardless of whether the bug still
+  // exists or no.
+  expect(fs.readFileSync(babelFileThatRequiresInvariant).toString()).toMatch(
+    /invariant/,
+  );
+  writeFiles(tempDir, {
+    '.babelrc': `
+      {
+        "plugins": [
+          "${require.resolve('babel-plugin-transform-flow-strip-types')}"
+        ]
+      }
+    `,
+    '__tests__/test.js': `
+      const invariant = require('invariant');
+      test('haii', () => expect(invariant(false, 'haii')).toBe('haii'));
+    `,
+    'invariant.js': `/**
+      * @providesModule invariant
+      * @flow
+      */
+      const invariant = (condition: boolean, message: string) => message;
+      module.exports = invariant;
+    `,
+    'jest.config.js': `module.exports = {
+      transform: {'.*\\.js': './third-party/node_modules/babel-jest'},
+    };`,
+  });
+  const {stderr, status} = runJest(tempDir, ['--no-cache', '--no-watchman']);
+  // make sure there are no errors that lead to invariant.js (if we were to
+  // require a wrong `invariant.js` we'd have a syntax error, because jest
+  // internals wouldn't be able to parse flow annotations)
+  expect(stderr).not.toMatch('invariant');
+  expect(stderr).toMatch('PASS');
+  expect(status).toBe(0);
+});

packages/jest-circus/package.json

@@ -8,6 +8,7 @@
   "license": "MIT",
   "main": "build/index.js",
   "dependencies": {
+    "babel-traverse": "^6.0.0",
     "chalk": "^2.0.1",
     "co": "^4.6.0",
     "expect": "^23.4.0",

packages/jest-circus/src/legacy_code_todo_rewrite/jest_adapter.js

@@ -33,8 +33,14 @@ const jestAdapter = async (
       expand: globalConfig.expand,
     });
 
+  const getPrettier = () =>
+    config.prettierPath ? require(config.prettierPath) : null;
+  const getBabelTraverse = () => require('babel-traverse').default;
+
   const {globals, snapshotState} = initialize({
     config,
+    getBabelTraverse,
+    getPrettier,
     globalConfig,
     localRequire: runtime.requireModule.bind(runtime),
     parentProcess: process,

packages/jest-circus/src/legacy_code_todo_rewrite/jest_adapter_init.js

@@ -23,12 +23,16 @@ import globals from '../index';
 const Promise = getOriginalPromise();
 export const initialize = ({
   config,
+  getPrettier,
+  getBabelTraverse,
   globalConfig,
   localRequire,
   parentProcess,
   testPath,
 }: {
   config: ProjectConfig,
+  getPrettier: () => null | any,
+  getBabelTraverse: () => Function,
   globalConfig: GlobalConfig,
   localRequire: Path => any,
   testPath: Path,
@@ -94,8 +98,8 @@ export const initialize = ({
   const {expand, updateSnapshot} = globalConfig;
   const snapshotState = new SnapshotState(testPath, {
     expand,
-    getPrettier: () =>
-      config.prettierPath ? localRequire(config.prettierPath) : null,
+    getBabelTraverse,
+    getPrettier,
     updateSnapshot,
   });
   setState({snapshotState, testPath});

packages/jest-jasmine2/package.json

@@ -8,6 +8,7 @@
   "license": "MIT",
   "main": "build/index.js",
   "dependencies": {
+    "babel-traverse": "^6.0.0",
     "chalk": "^2.0.1",
     "co": "^4.6.0",
     "expect": "^23.4.0",

packages/jest-jasmine2/src/setup_jest_globals.js

@@ -100,6 +100,7 @@ export default ({
   const {expand, updateSnapshot} = globalConfig;
   const snapshotState = new SnapshotState(testPath, {
     expand,
+    getBabelTraverse: () => require('babel-traverse').default,
     getPrettier: () =>
       config.prettierPath ? localRequire(config.prettierPath) : null,
     updateSnapshot,

packages/jest-snapshot/package.json

@@ -8,7 +8,6 @@
   "license": "MIT",
   "main": "build/index.js",
   "dependencies": {
-    "babel-traverse": "^6.0.0",
     "babel-types": "^6.0.0",
     "chalk": "^2.0.1",
     "jest-diff": "^23.2.0",

packages/jest-snapshot/src/State.js

@@ -25,6 +25,7 @@ import {saveInlineSnapshots, type InlineSnapshot} from './inline_snapshots';
 export type SnapshotStateOptions = {|
   updateSnapshot: SnapshotUpdateState,
   getPrettier: () => null | any,
+  getBabelTraverse: () => Function,
   snapshotPath?: string,
   expand?: boolean,
 |};
@@ -46,6 +47,7 @@ export default class SnapshotState {
   _snapshotPath: Path;
   _inlineSnapshots: Array<InlineSnapshot>;
   _uncheckedKeys: Set<string>;
+  _getBabelTraverse: () => Function;
   _getPrettier: () => null | any;
   added: number;
   expand: boolean;
@@ -61,6 +63,7 @@ export default class SnapshotState {
     );
     this._snapshotData = data;
     this._dirty = dirty;
+    this._getBabelTraverse = options.getBabelTraverse;
     this._getPrettier = options.getPrettier;
     this._inlineSnapshots = [];
     this._uncheckedKeys = new Set(Object.keys(this._snapshotData));
@@ -122,7 +125,8 @@ export default class SnapshotState {
       }
       if (hasInlineSnapshots) {
         const prettier = this._getPrettier(); // Load lazily
-        saveInlineSnapshots(this._inlineSnapshots, prettier);
+        const babelTraverse = this._getBabelTraverse(); // Load lazily
+        saveInlineSnapshots(this._inlineSnapshots, prettier, babelTraverse);
       }
       status.saved = true;
     } else if (!hasExternalSnapshots && fs.existsSync(this._snapshotPath)) {

packages/jest-snapshot/src/__tests__/inline_snapshots.test.js

@@ -3,6 +3,8 @@
  *
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
  */
 
 jest.mock('fs');
@@ -11,6 +13,7 @@ jest.mock('prettier');
 const fs = require('fs');
 const path = require('path');
 const prettier = require('prettier');
+const babelTraverse = require('babel-traverse').default;
 
 const {saveInlineSnapshots} = require('../inline_snapshots');
 
@@ -23,6 +26,7 @@ beforeEach(() => {
   fs.writeFileSync = jest.fn();
   fs.readFileSync = jest.fn();
   fs.existsSync = jest.fn(() => true);
+  // $FlowFixMe mock
   fs.statSync = jest.fn(filePath => ({
     isDirectory: () => !filePath.endsWith('.js'),
   }));
@@ -40,7 +44,9 @@ afterEach(() => {
 
 test('saveInlineSnapshots() replaces empty function call with a template literal', () => {
   const filename = path.join(__dirname, 'my.test.js');
-  fs.readFileSync = jest.fn(() => `expect(1).toMatchInlineSnapshot();\n`);
+  fs.readFileSync = (jest.fn(
+    () => `expect(1).toMatchInlineSnapshot();\n`,
+  ): any);
 
   saveInlineSnapshots(
     [
@@ -50,6 +56,7 @@ test('saveInlineSnapshots() replaces empty function call with a template literal
       },
     ],
     prettier,
+    babelTraverse,
   );
 
   expect(fs.writeFileSync).toHaveBeenCalledWith(
@@ -58,11 +65,14 @@ test('saveInlineSnapshots() replaces empty function call with a template literal
   );
 });
 
+// $FlowFixMe test.each is not in flow-typed yet
 test.each([['babylon'], ['flow'], ['typescript']])(
   'saveInlineSnapshots() replaces existing template literal - %s parser',
   parser => {
     const filename = path.join(__dirname, 'my.test.js');
-    fs.readFileSync = jest.fn(() => 'expect(1).toMatchInlineSnapshot(`2`);\n');
+    fs.readFileSync = (jest.fn(
+      () => 'expect(1).toMatchInlineSnapshot(`2`);\n',
+    ): any);
 
     prettier.resolveConfig.sync.mockReturnValue({parser});
 
@@ -74,6 +84,7 @@ test.each([['babylon'], ['flow'], ['typescript']])(
         },
       ],
       prettier,
+      babelTraverse,
     );
 
     expect(prettier.resolveConfig.sync.mock.results[0].value).toEqual({parser});
@@ -87,9 +98,9 @@ test.each([['babylon'], ['flow'], ['typescript']])(
 
 test('saveInlineSnapshots() replaces existing template literal with property matchers', () => {
   const filename = path.join(__dirname, 'my.test.js');
-  fs.readFileSync = jest.fn(
+  fs.readFileSync = (jest.fn(
     () => 'expect(1).toMatchInlineSnapshot({}, `2`);\n',
-  );
+  ): any);
 
   saveInlineSnapshots(
     [
@@ -99,6 +110,7 @@ test('saveInlineSnapshots() replaces existing template literal with property mat
       },
     ],
     prettier,
+    babelTraverse,
   );
 
   expect(fs.writeFileSync).toHaveBeenCalledWith(
@@ -109,7 +121,9 @@ test('saveInlineSnapshots() replaces existing template literal with property mat
 
 test('saveInlineSnapshots() throws if frame does not match', () => {
   const filename = path.join(__dirname, 'my.test.js');
-  fs.readFileSync = jest.fn(() => 'expect(1).toMatchInlineSnapshot();\n');
+  fs.readFileSync = (jest.fn(
+    () => 'expect(1).toMatchInlineSnapshot();\n',
+  ): any);
 
   const save = () =>
     saveInlineSnapshots(
@@ -120,20 +134,24 @@ test('saveInlineSnapshots() throws if frame does not match', () => {
         },
       ],
       prettier,
+      babelTraverse,
     );
 
   expect(save).toThrowError(/Couldn't locate all inline snapshots./);
 });
 
 test('saveInlineSnapshots() throws if multiple calls to to the same location', () => {
   const filename = path.join(__dirname, 'my.test.js');
-  fs.readFileSync = jest.fn(() => 'expect(1).toMatchInlineSnapshot();\n');
+  fs.readFileSync = (jest.fn(
+    () => 'expect(1).toMatchInlineSnapshot();\n',
+  ): any);
 
   const frame = {column: 11, file: filename, line: 1};
   const save = () =>
     saveInlineSnapshots(
       [{frame, snapshot: `1`}, {frame, snapshot: `2`}],
       prettier,
+      babelTraverse,
     );
 
   expect(save).toThrowError(
@@ -143,10 +161,12 @@ test('saveInlineSnapshots() throws if multiple calls to to the same location', (
 
 test('saveInlineSnapshots() uses escaped backticks', () => {
   const filename = path.join(__dirname, 'my.test.js');
-  fs.readFileSync = jest.fn(() => 'expect("`").toMatchInlineSnapshot();\n');
+  fs.readFileSync = (jest.fn(
+    () => 'expect("`").toMatchInlineSnapshot();\n',
+  ): any);
 
   const frame = {column: 13, file: filename, line: 1};
-  saveInlineSnapshots([{frame, snapshot: '`'}], prettier);
+  saveInlineSnapshots([{frame, snapshot: '`'}], prettier, babelTraverse);
 
   expect(fs.writeFileSync).toHaveBeenCalledWith(
     filename,