refactor: make watcher a child of the fs package (#412)

* refactor: make watcher a child of the fs package

* test: add unit tests for fs/watcher

* fix: use proper conditional in scripts/circle/install.sh

* test: do not test watcher against watchman in appveyor

* fix: cd back into build directory after installing watchman

* fix: use -d to determine if watch is a directory in scrits/circle/install.sh

circle.yml

@@ -13,3 +13,5 @@ machine:
 dependencies:
   override:
     - bash -e scripts/circle/install.sh
+  cache_directories:
+    - /home/ubuntu/watchman

scripts/circle/install.sh

@@ -1,4 +1,27 @@
 #!/bin/bash -e
+set -x
+set -e
+
+cd ../
+
+# Install Watchman
+if [ -d watchman ]; then
+  cd watchman
+  sudo make install
+else
+  git clone https://github.com/facebook/watchman.git
+  cd watchman
+  git checkout v4.7.0
+
+  ./autogen.sh
+  ./configure
+  make
+  sudo make install
+fi
+
+cd ../lux
+
+# Install Node Modules
 rm -rf node_modules
 npm install
 npm link

src/constants.js

@@ -12,3 +12,5 @@ export const DATABASE_URL = ENV.DATABASE_URL;
 export const LUX_CONSOLE = ENV.LUX_CONSOLE || false;
 export const PLATFORM = platform();
 export const BACKSLASH = /\\/g;
+export const CIRCLECI = ENV.CIRCLECI;
+export const APPVEYOR = ENV.APPVEYOR;

src/packages/cli/commands/serve.js

@@ -3,9 +3,9 @@ import { cyan } from 'chalk';
 
 import { CWD, PORT, NODE_ENV } from '../../../constants';
 import Logger from '../../logger';
-import Watcher from '../../watcher';
 import { createLoader } from '../../loader';
 import { createCluster } from '../../pm';
+import { watch } from '../../fs';
 
 import { build } from './build';
 
@@ -26,7 +26,7 @@ export async function serve({
   const logger = new Logger(logging);
 
   if (hot) {
-    const watcher = await new Watcher(CWD);
+    const watcher = await watch(CWD);
 
     watcher.on('change', async (changed) => {
       await build(useStrict);

src/packages/fs/index.js

@@ -3,6 +3,7 @@ import fs from 'fs';
 import { join as joinPath, resolve as resolvePath } from 'path';
 import type { Stats } from 'fs'; // eslint-disable-line no-duplicate-imports
 
+import Watcher from './watcher';
 import createResolver from './utils/create-resolver';
 import createPathRemover from './utils/create-path-remover';
 import type { fs$readOpts, fs$writeOpts } from './interfaces';
@@ -17,7 +18,9 @@ export type { fs$ParsedPath } from './interfaces';
 /**
  * @private
  */
-export const { watch } = fs;
+export function watch(path: string): Promise<Watcher> {
+  return new Watcher(path);
+}
 
 /**
  * @private

src/packages/fs/test/fs.test.js

@@ -1,14 +1,15 @@
 // @flow
+import { join } from 'path';
 
 import { expect } from 'chai';
 import { it, describe, before, after, beforeEach, afterEach } from 'mocha';
-import { join } from 'path';
 import { spy } from 'sinon';
+import type { Spy } from 'sinon';
 
-import { createTmpDir, getTmpFile, createTmpFiles } from './utils';
+import Watcher from '../watcher';
 import * as fs from '../index';
 
-import type { Spy } from 'sinon';
+import { createTmpDir, getTmpFile, createTmpFiles } from './utils';
 
 describe('module "fs"', () => {
 
@@ -57,7 +58,7 @@ describe('module "fs"', () => {
     }
   });
 
-  describe('#mkdir', () => {
+  describe('#mkdir()', () => {
     it('delegates to node fs#mkdir', async () => {
       const dirPath = join(tmpDirPath, 'test-mkdir');
       await fs.mkdir(dirPath);
@@ -69,23 +70,23 @@ describe('module "fs"', () => {
     });
   });
 
-  describe('#rmdir', () => {
+  describe('#rmdir()', () => {
     it('delegates to node fs#rmdir', async () => {
       await fs.rmdir(tmpDirPath);
       expect(spies['rmdir'].calledWith(tmpDirPath)).to.be.true;
     });
     it('returns a promise', returnsPromiseSpec('rmdir', tmpDirPath));
   });
 
-  describe('#readdir', () => {
+  describe('#readdir()', () => {
     it('delegates to node fs#readdir', async () => {
       await fs.readdir(tmpDirPath);
       expect(spies['readdir'].calledWith(tmpDirPath)).to.be.true;
     });
     it('returns a promise', returnsPromiseSpec('readdir', tmpDirPath));
   });
 
-  describe('#readFile', () => {
+  describe('#readFile()', () => {
     let tmpFilePath: string;
 
     beforeEach(async () => {
@@ -100,7 +101,7 @@ describe('module "fs"', () => {
     it('returns a promise', returnsPromiseSpec('readFile', tmpFilePath));
   });
 
-  describe('#writeFile', () => {
+  describe('#writeFile()', () => {
     let tmpFilePath: string;
 
     beforeEach(async () => {
@@ -115,7 +116,7 @@ describe('module "fs"', () => {
     it('returns a promise', returnsPromiseSpec('writeFile', tmpFilePath));
   });
 
-  describe('#appendFile', () => {
+  describe('#appendFile()', () => {
     let tmpFilePath: string;
 
     beforeEach(async () => {
@@ -130,7 +131,7 @@ describe('module "fs"', () => {
     it('returns a promise', returnsPromiseSpec('appendFile', tmpFilePath));
   });
 
-  describe('#stat', () => {
+  describe('#stat()', () => {
     let tmpFilePath: string;
 
     beforeEach(async () => {
@@ -145,7 +146,7 @@ describe('module "fs"', () => {
     it('returns a promise', returnsPromiseSpec('stat', tmpFilePath));
   });
 
-  describe('#unlink', () => {
+  describe('#unlink()', () => {
     let tmpFilePath: string;
 
     beforeEach(async () => {
@@ -159,6 +160,26 @@ describe('module "fs"', () => {
     });
     it('returns a promise', returnsPromiseSpec('unlink', tmpFilePath));
   });
+
+  describe('#watch()', () => {
+    const watchPath = join('tmp', `lux-${Date.now()}`);
+    let result;
+
+    before(async () => {
+      await fs.mkdirRec(join(watchPath, 'app'));
+    });
+
+    after(async () => {
+      result.destroy();
+      await fs.rmrf(watchPath);
+    });
+
+    it('resolves with an instance of Watcher', async () => {
+      result = await fs.watch(watchPath);
+
+      expect(result).to.be.an.instanceof(Watcher);
+    });
+  });
 });
 
 function returnsPromiseSpec(

src/packages/fs/test/watcher.test.js

@@ -0,0 +1,75 @@
+// @flow
+import { join as joinPath } from 'path';
+
+import { expect } from 'chai';
+import { it, describe, after, before } from 'mocha';
+
+import Watcher from '../watcher';
+import { APPVEYOR } from '../../../constants';
+
+import { rmrf, mkdirRec, writeFile } from '../index';
+
+describe('module "fs"', () => {
+  const tmpDirPath = joinPath('tmp', `lux-${Date.now()}`);
+  const tmpAppPath = joinPath(tmpDirPath, 'app');
+
+  before(async () => {
+    await mkdirRec(tmpAppPath);
+  });
+
+  after(async () => {
+    await rmrf(tmpDirPath);
+  });
+
+  describe('class Watcher', () => {
+    if (!APPVEYOR) {
+      describe('- client Watchman', () => {
+        let subject;
+
+        before(async () => {
+          subject = await new Watcher(tmpDirPath);
+        });
+
+        describe('event "change"', () => {
+          it('is called when a file is modified', async () => {
+            subject.once('change', files => {
+              expect(files).to.be.an('array');
+            });
+
+            await writeFile(joinPath(tmpAppPath, 'index.js'), '');
+          });
+        });
+
+        describe('#destroy()', () => {
+          it('does not throw an error', () => {
+            expect(() => subject.destroy()).to.not.throw(Error);
+          });
+        });
+      });
+    }
+
+    describe('- client FSWatcher', () => {
+      let subject;
+
+      before(async () => {
+        subject = await new Watcher(tmpDirPath, false);
+      });
+
+      describe('event "change"', () => {
+        it('is called when a file is modified', async () => {
+          subject.once('change', files => {
+            expect(files).to.be.an('array');
+          });
+
+          await writeFile(joinPath(tmpAppPath, 'index.js'), '');
+        });
+      });
+
+      describe('#destroy()', () => {
+        it('does not throw an error', () => {
+          expect(() => subject.destroy()).to.not.throw(Error);
+        });
+      });
+    });
+  });
+});

src/packages/watcher/index.js → src/packages/fs/watcher/index.js

@@ -1,6 +1,6 @@
 // @flow
 import EventEmitter from 'events';
-import { FSWatcher } from 'fs';
+import type { FSWatcher } from 'fs';
 
 import { Client } from 'fb-watchman';
 
@@ -14,18 +14,18 @@ class Watcher extends EventEmitter {
 
   client: Client | FSWatcher;
 
-  constructor(path: string): Promise<Watcher> {
+  constructor(path: string, useWatchman: boolean = true): Promise<Watcher> {
     super();
-    return initialize(this, path);
+    return initialize(this, path, useWatchman);
   }
 
   destroy() {
     const { client } = this;
 
-    if (client instanceof FSWatcher) {
-      client.close();
-    } else if (client instanceof Client) {
+    if (client instanceof Client) {
       client.end();
+    } else {
+      client.close();
     }
   }
 }

src/packages/watcher/initialize.js → src/packages/fs/watcher/initialize.js

@@ -1,12 +1,14 @@
 // @flow
 import { join as joinPath } from 'path';
-import type { FSWatcher } from 'fs';
+import { watch as nativeWatch } from 'fs';
+import type { FSWatcher } from 'fs'; // eslint-disable-line no-duplicate-imports
 
 import { Client } from 'fb-watchman';
 
-import * as fs from '../fs';
-import exec from '../../utils/exec';
-import tryCatch from '../../utils/try-catch';
+import exec from '../../../utils/exec';
+import tryCatch from '../../../utils/try-catch';
+import isJSFile from '../utils/is-js-file';
+import { freezeProps } from '../../freezeable';
 
 import type { Watcher$Client } from './interfaces'; // eslint-disable-line max-len, no-unused-vars
 
@@ -18,10 +20,10 @@ const SUBSCRIPTION_NAME = 'lux-watcher';
  * @private
  */
 function fallback(instance: Watcher, path: string): FSWatcher {
-  return fs.watch(path, {
+  return nativeWatch(path, {
     recursive: true
   }, (type, name) => {
-    if (fs.isJSFile(name)) {
+    if (isJSFile(name)) {
       instance.emit('change', [{ name, type }]);
     }
   });
@@ -103,35 +105,28 @@ function setupWatchmen(instance: Watcher, path: string): Promise<Client> {
  */
 export default async function initialize<T: Watcher>(
   instance: T,
-  path: string
+  path: string,
+  useWatchman: boolean
 ): Promise<T> {
   const appPath = joinPath(path, 'app');
   let client;
 
-  await tryCatch(async () => {
-    await exec('which watchman');
-    client = await setupWatchmen(instance, appPath);
-  }, () => {
-    client = fallback(instance, appPath);
-  });
-
-  if (client) {
-    Object.defineProperties(instance, {
-      path: {
-        value: appPath,
-        writable: false,
-        enumerable: true,
-        configurable: false
-      },
-
-      client: {
-        value: client,
-        writable: false,
-        enumerable: true,
-        configurable: false
-      }
+  if (useWatchman) {
+    await tryCatch(async () => {
+      await exec('which watchman');
+      client = await setupWatchmen(instance, appPath);
     });
   }
 
+  Object.assign(instance, {
+    path: appPath,
+    client: client || fallback(instance, appPath)
+  });
+
+  freezeProps(instance, true,
+    'path',
+    'client'
+  );
+
   return instance;
 }