Fix npm3 module resolution + more caching.

Summary:This fixes unmock resolution for node_modules when using npm3. The way this is solved is by checking whether the parent dependency is unmocked and both modules are within a `node_modules` folder.

This has taken a while, mainly because it required #599 to be merged. I also added some more caching that makes test runs even faster. The react-native test suite now completes in about 6s (down from 7.5-8s). There might be more in the near future ;)

This fixes #554, #730, facebook/react#5183, facebook/relay#832 and will be part of 0.9.0. I'll publish 0.9.0-fb3 today with this fix.
Closes #732

Differential Revision: D2959610

fb-gh-sync-id: c374b7a2bcdfddf768905356a08948d9156eb028
shipit-source-id: c374b7a2bcdfddf768905356a08948d9156eb028

.gitignore

@@ -2,7 +2,9 @@
 *.swp
 .DS_STORE
 .haste_cache
-node_modules
+/node_modules
+/examples/**/node_modules/
+/website/node_modules
 npm-debug.log
 build
 website/core/metadata.js

CHANGELOG.md

@@ -10,7 +10,9 @@
 * Added a "no tests found" message if no tests can be found.
 * Debounce `--watch` re-runs to not trigger test runs during a
   branch switch in version control.
-* Fixed test runtime error reporting and stack traces
+* Fixed mocking of boolean values.
+* Fixed unmocking behavior with npm3.
+* Fixed test runtime error reporting and stack traces.
 
 ## 0.8.2
 

package.json

@@ -1,7 +1,7 @@
 {
   "name": "jest-cli",
   "description": "Painless JavaScript Unit Testing.",
-  "version": "0.9.0-fb2",
+  "version": "0.9.0-fb3",
   "main": "src/jest.js",
   "dependencies": {
     "chalk": "^1.1.1",

src/HasteModuleLoader/HasteModuleLoader.js

@@ -17,8 +17,8 @@ const transform = require('../lib/transform');
 
 const NODE_PATH =
   (process.env.NODE_PATH ? process.env.NODE_PATH.split(path.delimiter) : null);
+const NODE_MODULES = path.sep + 'node_modules' + path.sep;
 const IS_PATH_BASED_MODULE_NAME = /^(?:\.\.?\/|\/)/;
-const VENDOR_PATH = path.resolve(__dirname, '../../vendor');
 
 const mockParentModule = {
   id: 'mockParent',
@@ -27,20 +27,9 @@ const mockParentModule = {
 
 const normalizedIDCache = Object.create(null);
 const moduleNameCache = Object.create(null);
+const shouldMockModuleCache = Object.create(null);
 
-const isFile = file => {
-  let stat;
-  try {
-    stat = fs.statSync(file);
-  } catch (err) {
-    if (err && err.code === 'ENOENT') {
-      return false;
-    }
-  }
-  return stat.isFile() || stat.isFIFO();
-};
-
-let _configUnmockListRegExpCache = null;
+const unmockRegExpCache = new WeakMap();
 
 class Loader {
   constructor(config, environment, moduleMap) {
@@ -53,7 +42,6 @@ class Loader {
     this._isCurrentlyExecutingManualMock = null;
     this._mockMetaDataCache = Object.create(null);
     this._shouldAutoMock = true;
-    this._configShouldMockModuleNames = Object.create(null);
     this._extensions = config.moduleFileExtensions.map(ext => '.' + ext);
 
     this._modules = moduleMap.modules;
@@ -63,22 +51,11 @@ class Loader {
       this._CoverageCollector = require(config.coverageCollector);
     }
 
-    if (_configUnmockListRegExpCache === null) {
-      _configUnmockListRegExpCache = new WeakMap();
-    }
-
-    if (
-      !config.unmockedModulePathPatterns ||
-      config.unmockedModulePathPatterns.length === 0
-    ) {
-      this._unmockListRegExps = [];
-    } else {
-      this._unmockListRegExps = _configUnmockListRegExpCache.get(config);
-      if (!this._unmockListRegExps) {
-        this._unmockListRegExps = config.unmockedModulePathPatterns
-          .map(unmockPathRe => new RegExp(unmockPathRe));
-        _configUnmockListRegExpCache.set(config, this._unmockListRegExps);
-      }
+    this._unmockedList = unmockRegExpCache.get(config);
+    if (!this._unmockedList && config.unmockedModulePathPatterns) {
+      this._unmockedList =
+        new RegExp(config.unmockedModulePathPatterns.join('|'));
+      unmockRegExpCache.set(config, this._unmockedList);
     }
 
     // Workers communicate the config as JSON so we have to create a regex
@@ -135,11 +112,6 @@ class Loader {
       modulePath = this._resolveModuleName(currPath, moduleName);
     }
 
-    // Always natively require the jasmine runner.
-    if (modulePath.indexOf(VENDOR_PATH) === 0) {
-      return require(modulePath);
-    }
-
     if (!modulePath) {
       throw new Error(`Cannot find module '${moduleName}' from '${currPath}'`);
     }
@@ -289,7 +261,7 @@ class Loader {
     );
     let moduleContent = transform(filename, this._config);
     let collectorStore;
-    if (shouldCollectCoverage && !filename.includes('/node_modules/')) {
+    if (shouldCollectCoverage && !filename.includes(NODE_MODULES)) {
       if (!collectors[filename]) {
         collectors[filename] = new this._CoverageCollector(
           moduleContent,
@@ -373,15 +345,15 @@ class Loader {
     // Check if the resolver knows about this module
     if (this._modules[moduleName]) {
       return this._modules[moduleName];
-    } else {
-      // Otherwise it is likely a node_module.
-      const key = currPath + ' : ' + moduleName;
-      if (moduleNameCache[key]) {
-        return moduleNameCache[key];
-      }
-      moduleNameCache[key] = this._resolveNodeModule(currPath, moduleName);
+    }
+
+    // Otherwise it is likely a node_module.
+    const key = currPath + ' : ' + moduleName;
+    if (moduleNameCache[key]) {
       return moduleNameCache[key];
     }
+    moduleNameCache[key] = this._resolveNodeModule(currPath, moduleName);
+    return moduleNameCache[key];
   }
 
   _resolveNodeModule(currPath, moduleName) {
@@ -390,9 +362,7 @@ class Loader {
       return resolve.sync(moduleName, {
         basedir,
         extensions: this._extensions,
-        isFile,
         paths: NODE_PATH,
-        readFileSync: fs.readFileSync,
       });
     } catch (e) {
       const parts = moduleName.split('/');
@@ -447,10 +417,7 @@ class Loader {
       moduleType = 'user';
       if (
         IS_PATH_BASED_MODULE_NAME.test(moduleName) ||
-        (
-          this._getModule(moduleName) === undefined &&
-          this._getMockModule(moduleName) === undefined
-        )
+        (!this._getModule(moduleName) && !this._getMockModule(moduleName))
       ) {
         realAbsPath = this._resolveModuleName(currPath, moduleName);
         if (realAbsPath == null) {
@@ -491,16 +458,18 @@ class Loader {
     const moduleID = this._getNormalizedModuleID(currPath, moduleName);
     if (moduleID in this._explicitShouldMock) {
       return this._explicitShouldMock[moduleID];
-    } else if (resolve.isCore(moduleName)) {
+    }
+
+    if (resolve.isCore(moduleName)) {
       return false;
-    } else if (this._shouldAutoMock) {
+    }
+
+    if (this._shouldAutoMock) {
       // See if the module is specified in the config as a module that should
       // never be mocked
-      if (moduleName in this._configShouldMockModuleNames) {
-        return this._configShouldMockModuleNames[moduleName];
-      } else if (this._unmockListRegExps.length > 0) {
-        this._configShouldMockModuleNames[moduleName] = true;
-
+      if (moduleName in shouldMockModuleCache) {
+        return shouldMockModuleCache[moduleName];
+      } else if (this._unmockedList) {
         const manualMockResource = this._getMockModule(moduleName);
         let modulePath;
         try {
@@ -521,27 +490,33 @@ class Loader {
           // all module environments are complete (meaning you can't just
           // write a manual mock as a substitute for a real module).
           if (manualMockResource) {
+            shouldMockModuleCache[moduleName] = true;
             return true;
           }
           throw e;
         }
-        let unmockRegExp;
 
-        // Never mock the jasmine environment.
-        if (modulePath.indexOf(VENDOR_PATH) === 0) {
+        const realPath = fs.realpathSync(modulePath);
+        if (this._unmockedList.test(realPath)) {
+          shouldMockModuleCache[moduleName] = false;
+        } else if (
+          currPath.includes(NODE_MODULES) &&
+          realPath.includes(NODE_MODULES) &&
+          (
+            !shouldMockModuleCache[moduleName] ||
+            this._unmockedList.test(currPath)
+          )
+        ) {
+          // If the dependency should normally be mocked but the parent
+          // module is a node module that is not being mocked, the target module
+          // should be unmocked too. This transitive behavior is useful for flat
+          // package managers, like npm3.
           return false;
+        } else {
+          shouldMockModuleCache[moduleName] = true;
         }
 
-        const realPath = fs.realpathSync(modulePath);
-        this._configShouldMockModuleNames[moduleName] = true;
-        for (let i = 0; i < this._unmockListRegExps.length; i++) {
-          unmockRegExp = this._unmockListRegExps[i];
-          if (unmockRegExp.test(modulePath) ||
-              unmockRegExp.test(realPath)) {
-            return this._configShouldMockModuleNames[moduleName] = false;
-          }
-        }
-        return this._configShouldMockModuleNames[moduleName];
+        return shouldMockModuleCache[moduleName];
       }
       return true;
     } else {

src/HasteModuleLoader/__tests__/HasteModuleLoader-requireModule-test.js

@@ -26,6 +26,7 @@ describe('HasteModuleLoader', function() {
     cacheDirectory: global.CACHE_DIRECTORY,
     name: 'HasteModuleLoader-requireModule-tests',
     rootDir,
+    unmockedModulePathPatterns: ['npm3-main-dep'],
   });
 
   function buildLoader() {
@@ -143,51 +144,38 @@ describe('HasteModuleLoader', function() {
       });
     });
 
-    describe('features I want to remove, but must exist for now', function() {
-      /**
-       * I'd like to kill this and make all tests use something more explicit
-       * when they want a manual mock, like:
-       *
-       *   require.mock('MyManualMock');
-       *   const ManuallyMocked = require('ManuallyMocked');
-       *
-       *   --or--
-       *
-       *   const ManuallyMocked = require.manualMock('ManuallyMocked');
-       *
-       * For now, however, this is built-in and many tests rely on it, so we
-       * must support it until we can do some cleanup.
-       */
-      pit('provides manual mock when real module doesnt exist', function() {
-        return buildLoader().then(function(loader) {
-          const exports = loader.requireModule(
-            rootPath,
-            'ExclusivelyManualMock'
-          );
-          expect(exports.isExclusivelyManualMockModule).toBe(true);
-        });
+    pit('provides manual mock when real module doesnt exist', function() {
+      return buildLoader().then(function(loader) {
+        const exports = loader.requireModule(
+          rootPath,
+          'ExclusivelyManualMock'
+        );
+        expect(exports.isExclusivelyManualMockModule).toBe(true);
       });
+    });
+
+    pit(
+      'doesnt override real modules with manual mocks when explicitly ' +
+        'marked with .dontMock()',
+        function() {
+          return buildLoader().then(function(loader) {
+            const root = loader.requireModule(rootPath, './root.js');
+            root.jest.resetModuleRegistry();
+            root.jest.dontMock('ManuallyMocked');
+            const exports = loader.requireModule(rootPath, 'ManuallyMocked');
+            expect(exports.isManualMockModule).toBe(false);
+          });
+        }
+    );
 
-      /**
-       * requireModule() should *always* return the real module. Mocks should
-       * only be returned by requireMock().
-       *
-       * See the 'overrides real modules with manual mock when one exists' test
-       * for more info on why I want to kill this feature.
-       */
-      pit(
-        'doesnt override real modules with manual mocks when explicitly ' +
-          'marked with .dontMock()',
-          function() {
-            return buildLoader().then(function(loader) {
-              const root = loader.requireModule(rootPath, './root.js');
-              root.jest.resetModuleRegistry();
-              root.jest.dontMock('ManuallyMocked');
-              const exports = loader.requireModule(rootPath, 'ManuallyMocked');
-              expect(exports.isManualMockModule).toBe(false);
-            });
-          }
-      );
+    pit('unmocks transitive dependencies in node_modules by default', () => {
+      return buildLoader().then(loader => {
+        const nodeModule = loader.requireModule(rootPath, 'npm3-main-dep');
+        expect(nodeModule()).toEqual({
+          isMocked: false,
+          transitiveNPM3Dep: 'npm3-transitive-dep',
+        });
+      });
     });
   });
 });

src/HasteModuleLoader/__tests__/test_root/ManuallyMocked.js

@@ -8,4 +8,6 @@
  * @providesModule ManuallyMocked
  */
 
+'use strict';
+
 exports.isManualMockModule = false;

src/HasteModuleLoader/__tests__/test_root/__mocks__/ExclusivelyManualMock.js

@@ -5,4 +5,7 @@
  * 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';
+
 exports.isExclusivelyManualMockModule = true;

src/HasteModuleLoader/__tests__/test_root/__mocks__/ManuallyMocked.js

@@ -5,6 +5,7 @@
  * 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';
 
 let OnlyRequiredFromMock;

src/HasteModuleLoader/__tests__/test_root/root.js

@@ -8,6 +8,8 @@
  * @providesModule root
  */
 
+'use strict';
+
 require('ManuallyMocked');
 require('ModuleWithSideEffects');
 require('RegularModule');

src/HasteModuleLoader/__tests__/test_root/subdir1/MyModule.js

@@ -5,4 +5,7 @@
  * 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';
+
 exports.modulePath = 'subdir1/MyModule.js';