New file watching implementation

Summary: This is the next incremental step to rewrite node-haste. I apologize for the size of this diff but there is really no smaller way to do this. The current architecture passes a single file watcher instance into many classes that each subscribe to file changes. It's really hard to keep track of this. The new implementation reduces the listeners to two (will eventually be just one!) - one in DependencyGraph and one in it's parent's parent's parent (ugh! This doesn't make any sense). This should make it much more straightforward to understand what happens when a file changes. I was able to remove a bunch of tests because jest's watcher takes care of things like ignore patterns. Some of the tests were specifically testing for whether the change events were invoked and they are now much more straightforward as well by manually invoking the `processFileChange` methods. (Relanding a fixed version of D4161662) Reviewed By: kentaromiura Differential Revision: D4194378 fbshipit-source-id: 8c008247a911573f6b5f6b0b374d50d38f62a4f5
bvaughn · Nov 17, 2016 · a9338d6 · a9338d6
1 parent 84553eb
commit a9338d6
Show file tree

Hide file tree

Showing 20 changed files with 191 additions and 741 deletions.
diff --git a/docs/GettingStarted.md b/docs/GettingStarted.md
@@ -362,8 +362,6 @@ If everything is set up correctly, you should see your new app running in your A
 
 <block class="windows android" />
 
-> If you hit a `ERROR  Watcher took too long to load`, try increasing the timeout in [this file](https://github.com/facebook/react-native/blob/5fa33f3d07f8595a188f6fe04d6168a6ede1e721/packager/react-packager/src/DependencyResolver/FileWatcher/index.js#L16) (under your `node_modules/react-native/`).
-
 > If you're targeting API level 23, the app might crash on first launch with an error smilar to `Unable to add window android.view.ViewRootImpl$W@c51fa6 -- permission denied for this window type`. To fix this, you need to go to `System settings > Apps > Configure apps > Draw over other apps` and grant the permission for the app.
 
 NOTE: Many React Native modules haven't been tested on Marshmallow and might break. Please throughly test the app if you target API level 23 and file a bug report if you find that something is broken.
@@ -467,7 +465,7 @@ if (window.location.hash !== '' && window.location.hash !== 'content') { // cont
           }
           // We would have broken out if both targetPlatform and devOS hadn't been filled.
           display('os', devOS);
-          display('platform', targetPlatform);      
+          display('platform', targetPlatform);
           foundHash = true;
           break;
         }

diff --git a/local-cli/bundle/buildBundle.js b/local-cli/bundle/buildBundle.js
@@ -55,8 +55,8 @@ function buildBundle(args, config, output = outputBundle, packagerInstance) {
       getTransformOptionsModulePath: config.getTransformOptionsModulePath,
       transformModulePath: transformModulePath,
       extraNodeModules: config.extraNodeModules,
-      nonPersistent: true,
       resetCache: args.resetCache,
+      watch: false,
     };
 
     packagerInstance = new Server(options);

diff --git a/local-cli/server/runServer.js b/local-cli/server/runServer.js
@@ -8,12 +8,14 @@
  */
 'use strict';
 
+const InspectorProxy = require('./util/inspectorProxy.js');
 const ReactPackager = require('../../packager/react-packager');
 
 const attachHMRServer = require('./util/attachHMRServer');
 const connect = require('connect');
 const copyToClipBoardMiddleware = require('./middleware/copyToClipBoardMiddleware');
 const cpuProfilerMiddleware = require('./middleware/cpuProfilerMiddleware');
+const defaultAssetExts = require('../../packager/defaults').assetExts;
 const getDevToolsMiddleware = require('./middleware/getDevToolsMiddleware');
 const heapCaptureMiddleware = require('./middleware/heapCaptureMiddleware.js');
 const http = require('http');
@@ -25,10 +27,8 @@ const openStackFrameInEditorMiddleware = require('./middleware/openStackFrameInE
 const path = require('path');
 const statusPageMiddleware = require('./middleware/statusPageMiddleware.js');
 const systraceProfileMiddleware = require('./middleware/systraceProfileMiddleware.js');
-const webSocketProxy = require('./util/webSocketProxy.js');
-const InspectorProxy = require('./util/inspectorProxy.js');
-const defaultAssetExts = require('../../packager/defaults').assetExts;
 const unless = require('./middleware/unless');
+const webSocketProxy = require('./util/webSocketProxy.js');
 
 function runServer(args, config, readyCallback) {
   var wsProxy = null;
@@ -86,17 +86,17 @@ function getPackagerServer(args, config) {
     undefined;
 
   return ReactPackager.createServer({
-    nonPersistent: args.nonPersistent,
-    projectRoots: args.projectRoots,
+    assetExts: defaultAssetExts.concat(args.assetExts),
+    assetRoots: args.assetRoots,
     blacklistRE: config.getBlacklistRE(),
     cacheVersion: '3',
-    getTransformOptionsModulePath: config.getTransformOptionsModulePath,
-    transformModulePath: transformModulePath,
     extraNodeModules: config.extraNodeModules,
-    assetRoots: args.assetRoots,
-    assetExts: defaultAssetExts.concat(args.assetExts),
+    getTransformOptionsModulePath: config.getTransformOptionsModulePath,
+    projectRoots: args.projectRoots,
     resetCache: args.resetCache,
+    transformModulePath: transformModulePath,
     verbose: args.verbose,
+    watch: !args.nonPersistent,
   });
 }
 

diff --git a/packager/package.json b/packager/package.json
@@ -5,32 +5,5 @@
   "repository": {
     "type": "git",
     "url": "[email protected]:facebook/react-native.git"
-  },
-  "engines": {
-    "node": ">=4"
-  },
-  "jest": {
-    "setupEnvScriptFile": "jest/setup.js",
-    "testPathIgnorePatterns": [
-      "/node_modules/"
-    ],
-    "testFileExtensions": [
-      "js"
-    ],
-    "unmockedModulePathPatterns": [
-      "source-map"
-    ]
-  },
-  "scripts": {
-    "test": "jest",
-    "lint": "node linter.js Examples/",
-    "start": "./packager.sh"
-  },
-  "dependencies": {
-    "wordwrap": "^1.0.0"
-  },
-  "devDependencies": {
-    "jest-cli": "git://github.com/facebook/jest#0.5.x",
-    "eslint": "0.9.2"
   }
 }
diff --git a/packager/react-packager/index.js b/packager/react-packager/index.js
@@ -45,26 +45,14 @@ function createServer(options) {
     enableDebug();
   }
 
+  options = Object.assign({}, options);
+  delete options.verbose;
   var Server = require('./src/Server');
-  return new Server(omit(options, ['verbose']));
+  return new Server(options);
 }
 
 function createNonPersistentServer(options) {
   Logger.disablePrinting();
-  // Don't start the filewatcher or the cache.
-  if (options.nonPersistent == null) {
-    options.nonPersistent = true;
-  }
-
+  options.watch = !options.nonPersistent;
   return createServer(options);
 }
-
-function omit(obj, blacklistedKeys) {
-  return Object.keys(obj).reduce((clone, key) => {
-    if (blacklistedKeys.indexOf(key) === -1) {
-      clone[key] = obj[key];
-    }
-
-    return clone;
-  }, {});
-}
diff --git a/packager/react-packager/src/AssetServer/__tests__/AssetServer-test.js b/packager/react-packager/src/AssetServer/__tests__/AssetServer-test.js
@@ -15,25 +15,22 @@ jest.mock('fs');
 
 const AssetServer = require('../');
 const crypto = require('crypto');
-const {EventEmitter} = require('events');
 const fs = require('fs');
 
 const {objectContaining} = jasmine;
 
 describe('AssetServer', () => {
-  let fileWatcher;
   beforeEach(() => {
     const NodeHaste = require('../../node-haste');
-    NodeHaste.getAssetDataFromName = require.requireActual('../../node-haste/lib/getAssetDataFromName');
-    fileWatcher = new EventEmitter();
+    NodeHaste.getAssetDataFromName =
+      require.requireActual('../../node-haste/lib/getAssetDataFromName');
   });
 
   describe('assetServer.get', () => {
     it('should work for the simple case', () => {
       const server = new AssetServer({
         projectRoots: ['/root'],
         assetExts: ['png'],
-        fileWatcher,
       });
 
       fs.__setMockFilesystem({
@@ -59,7 +56,6 @@ describe('AssetServer', () => {
       const server = new AssetServer({
         projectRoots: ['/root'],
         assetExts: ['png'],
-        fileWatcher,
       });
 
       fs.__setMockFilesystem({
@@ -97,7 +93,6 @@ describe('AssetServer', () => {
       const server = new AssetServer({
         projectRoots: ['/root'],
         assetExts: ['png', 'jpg'],
-        fileWatcher,
       });
 
       fs.__setMockFilesystem({
@@ -124,7 +119,6 @@ describe('AssetServer', () => {
       const server = new AssetServer({
         projectRoots: ['/root'],
         assetExts: ['png'],
-        fileWatcher,
       });
 
       fs.__setMockFilesystem({
@@ -147,7 +141,6 @@ describe('AssetServer', () => {
       const server = new AssetServer({
         projectRoots: ['/root'],
         assetExts: ['png'],
-        fileWatcher,
       });
 
       fs.__setMockFilesystem({
@@ -179,7 +172,6 @@ describe('AssetServer', () => {
       const server = new AssetServer({
         projectRoots: ['/root', '/root2'],
         assetExts: ['png'],
-        fileWatcher,
       });
 
       fs.__setMockFilesystem({
@@ -208,7 +200,6 @@ describe('AssetServer', () => {
       const server = new AssetServer({
         projectRoots: ['/root'],
         assetExts: ['png'],
-        fileWatcher,
       });
 
       fs.__setMockFilesystem({
@@ -241,7 +232,6 @@ describe('AssetServer', () => {
       const server = new AssetServer({
         projectRoots: ['/root'],
         assetExts: ['png', 'jpeg'],
-        fileWatcher,
       });
 
       fs.__setMockFilesystem({
@@ -271,15 +261,14 @@ describe('AssetServer', () => {
     });
 
     describe('hash:', () => {
-      let server, fileSystem;
+      let server, mockFS;
       beforeEach(() => {
         server = new AssetServer({
           projectRoots: ['/root'],
           assetExts: ['jpg'],
-          fileWatcher,
         });
 
-        fileSystem = {
+        mockFS = {
           'root': {
             imgs: {
               '[email protected]': 'b1 image',
@@ -290,13 +279,13 @@ describe('AssetServer', () => {
           }
         };
 
-       fs.__setMockFilesystem(fileSystem);
+       fs.__setMockFilesystem(mockFS);
       });
 
       it('uses the file contents to build the hash', () => {
         const hash = crypto.createHash('md5');
-        for (const name in fileSystem.root.imgs) {
-          hash.update(fileSystem.root.imgs[name]);
+        for (const name in mockFS.root.imgs) {
+          hash.update(mockFS.root.imgs[name]);
         }
 
         return server.getAssetData('imgs/b.jpg').then(data =>
@@ -306,8 +295,8 @@ describe('AssetServer', () => {
 
       it('changes the hash when the passed-in file watcher emits an `all` event', () => {
         return server.getAssetData('imgs/b.jpg').then(initialData => {
-          fileSystem.root.imgs['[email protected]'] = 'updated data';
-          fileWatcher.emit('all', 'arbitrary', '/root', 'imgs/[email protected]');
+          mockFS.root.imgs['[email protected]'] = 'updated data';
+          server.onFileChange('all', '/root/imgs/[email protected]');
           return server.getAssetData('imgs/b.jpg').then(data =>
             expect(data.hash).not.toEqual(initialData.hash)
           );

diff --git a/packager/react-packager/src/AssetServer/index.js b/packager/react-packager/src/AssetServer/index.js
@@ -42,10 +42,6 @@ const validateOpts = declareOpts({
     type: 'array',
     required: true,
   },
-  fileWatcher: {
-    type: 'object',
-    required: true,
-  }
 });
 
 class AssetServer {
@@ -55,9 +51,6 @@ class AssetServer {
     this._assetExts = opts.assetExts;
     this._hashes = new Map();
     this._files = new Map();
-
-    opts.fileWatcher
-      .on('all', (type, root, file) => this._onFileChange(type, root, file));
   }
 
   get(assetPath, platform = null) {
@@ -84,7 +77,6 @@ class AssetServer {
       data.scales = record.scales;
       data.files = record.files;
 
-
       if (this._hashes.has(assetPath)) {
         data.hash = this._hashes.get(assetPath);
         return data;
@@ -106,9 +98,8 @@ class AssetServer {
     });
   }
 
-  _onFileChange(type, root, file) {
-    const asset = this._files.get(path.join(root, file));
-    this._hashes.delete(asset);
+  onFileChange(type, filePath) {
+    this._hashes.delete(this._files.get(filePath));
   }
 
   /**
@@ -187,7 +178,10 @@ class AssetServer {
       }
 
       const rootsString = roots.map(s => `'${s}'`).join(', ');
-      throw new Error(`'${debugInfoFile}' could not be found, because '${dir}' is not a subdirectory of any of the roots  (${rootsString})`);
+      throw new Error(
+        `'${debugInfoFile}' could not be found, because '${dir}' is not a ` +
+        `subdirectory of any of the roots  (${rootsString})`,
+      );
     });
   }
 

diff --git a/packager/react-packager/src/Bundler/index.js b/packager/react-packager/src/Bundler/index.js
@@ -79,10 +79,6 @@ const validateOpts = declareOpts({
     type: 'object',
     required: false,
   },
-  nonPersistent: {
-    type: 'boolean',
-    default: false,
-  },
   assetRoots: {
     type: 'array',
     required: false,
@@ -91,9 +87,9 @@ const validateOpts = declareOpts({
     type: 'array',
     default: ['png'],
   },
-  fileWatcher: {
-    type: 'object',
-    required: true,
+  watch: {
+    type: 'boolean',
+    default: false,
   },
   assetServer: {
     type: 'object',
@@ -124,10 +120,9 @@ type Options = {
   resetCache: boolean,
   transformModulePath: string,
   extraNodeModules: {},
-  nonPersistent: boolean,
   assetRoots: Array<string>,
   assetExts: Array<string>,
-  fileWatcher: {},
+  watch: boolean,
   assetServer: AssetServer,
   transformTimeoutInterval: ?number,
   allowBundleUpdates: boolean,
@@ -191,7 +186,7 @@ class Bundler {
       blacklistRE: opts.blacklistRE,
       cache: this._cache,
       extraNodeModules: opts.extraNodeModules,
-      fileWatcher: opts.fileWatcher,
+      watch: opts.watch,
       minifyCode: this._transformer.minify,
       moduleFormat: opts.moduleFormat,
       polyfillModuleNames: opts.polyfillModuleNames,
@@ -217,9 +212,12 @@ class Bundler {
     }
   }
 
-  kill() {
+  end() {
     this._transformer.kill();
-    return this._cache.end();
+    return Promise.all([
+      this._cache.end(),
+      this.getResolver().getDependencyGraph().getWatcher().end(),
+    ]);
   }
 
   bundle(options: {