From 15a0f07c5f2e3febc0ef4a38934cc15044d75942 Mon Sep 17 00:00:00 2001 From: Amjad Masad Date: Fri, 26 Jun 2015 16:17:16 -0700 Subject: [PATCH] [react-packager] Enable watchman fs crawl Summary: @public Now that watchman perf issue was fixed we can enable watchman-based fs crawling which is faster than node. This showed an existing issue with some files missing from the blacklist which I addressed. Test Plan: ./fbrnios.sh run click around and scroll all the apps --- blacklist.js | 4 ++-- .../__tests__/DependencyGraph-test.js | 3 ++- .../src/DependencyResolver/crawlers/index.js | 14 ++------------ .../src/DependencyResolver/crawlers/watchman.js | 2 +- react-packager/src/FileWatcher/index.js | 10 +++++++--- 5 files changed, 14 insertions(+), 19 deletions(-) diff --git a/blacklist.js b/blacklist.js index a2ba7167..af2e2879 100644 --- a/blacklist.js +++ b/blacklist.js @@ -24,7 +24,7 @@ var platformBlacklists = { ios: [ 'node_modules/react-tools/src/browser/ui/React.js', 'node_modules/react-tools/src/browser/eventPlugins/ResponderEventPlugin.js', - // 'node_modules/react-tools/src/vendor/core/ExecutionEnvironment.js', + 'node_modules/react-tools/src/vendor/core/ExecutionEnvironment.js', '.web.js', '.android.js', ], @@ -32,7 +32,7 @@ var platformBlacklists = { 'node_modules/react-tools/src/browser/ui/React.js', 'node_modules/react-tools/src/browser/eventPlugins/ResponderEventPlugin.js', 'node_modules/react-tools/src/browser/ReactTextComponent.js', - // 'node_modules/react-tools/src/vendor/core/ExecutionEnvironment.js', + 'node_modules/react-tools/src/vendor/core/ExecutionEnvironment.js', '.web.js', '.ios.js', ], diff --git a/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js b/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js index 673d9c58..471e6069 100644 --- a/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js +++ b/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js @@ -2353,7 +2353,8 @@ describe('DependencyGraph', function() { } callbacks.push(callback); return this; - } + }, + isWatchman: () => Promise.resolve(false), }; }); diff --git a/react-packager/src/DependencyResolver/crawlers/index.js b/react-packager/src/DependencyResolver/crawlers/index.js index 71290af4..fe755bcb 100644 --- a/react-packager/src/DependencyResolver/crawlers/index.js +++ b/react-packager/src/DependencyResolver/crawlers/index.js @@ -1,21 +1,11 @@ 'use strict'; const nodeCrawl = require('./node'); -//const watchmanCrawl = require('./watchman'); +const watchmanCrawl = require('./watchman'); function crawl(roots, options) { - return nodeCrawl(roots, options); - - // Although, in theory, watchman should be much faster; - // there is currently a bottleneck somewhere in the - // encoding/decoding that is causing it to be slower - // than node crawling. However, this should be fixed soon. - // https://github.com/facebook/watchman/issues/113 - /* const {fileWatcher} = options; return fileWatcher.isWatchman().then(isWatchman => { - - console.log(isWatchman); if (!isWatchman) { return false; } @@ -30,7 +20,7 @@ function crawl(roots, options) { } return nodeCrawl(roots, options); - });*/ + }); } module.exports = crawl; diff --git a/react-packager/src/DependencyResolver/crawlers/watchman.js b/react-packager/src/DependencyResolver/crawlers/watchman.js index d6479a51..1871e3ea 100644 --- a/react-packager/src/DependencyResolver/crawlers/watchman.js +++ b/react-packager/src/DependencyResolver/crawlers/watchman.js @@ -36,7 +36,7 @@ function watchmanRecReadDir(roots, {ignore, fileWatcher, exts}) { } } - const cmd = Promise.promisify(watcher.client.command.bind(watcher.client)); + const cmd = Promise.denodeify(watcher.client.command.bind(watcher.client)); return cmd(['query', watchedRoot, { 'suffix': exts, 'expression': ['allof', ['type', 'f'], 'exists', dirExpr], diff --git a/react-packager/src/FileWatcher/index.js b/react-packager/src/FileWatcher/index.js index 46b4667e..d9ed7f32 100644 --- a/react-packager/src/FileWatcher/index.js +++ b/react-packager/src/FileWatcher/index.js @@ -12,9 +12,11 @@ const EventEmitter = require('events').EventEmitter; const sane = require('sane'); const Promise = require('promise'); const exec = require('child_process').exec; +const _ = require('underscore'); const MAX_WAIT_TIME = 25000; +// TODO(amasad): can we use watchman version command instead?r const detectingWatcherClass = new Promise(function(resolve) { exec('which watchman', function(err, out) { if (err || out.length === 0) { @@ -79,9 +81,11 @@ class FileWatcher extends EventEmitter { static createDummyWatcher() { const ev = new EventEmitter(); - ev.end = function() { - return Promise.resolve(); - }; + _.extend(ev, { + isWatchman: () => Promise.resolve(false), + end: () => Promise.resolve(), + }); + return ev; } }