From fa7f970266ff6353238d4d252c277433f88edc1e Mon Sep 17 00:00:00 2001 From: Anton Nikolaev Date: Tue, 31 Dec 2019 04:36:37 -0800 Subject: [PATCH] Plugin change detection is unreliable on Windows VM Summary: On Windows VM when "yarn start" is executed and compilation is in progress for some plugin, fs.watch randomly fires "changed" events for different files of other plugins. This leads to infinite attempts to rebuild the same plugin again and again, and this process never ends, so "yarn start" is almost unusable: {F225467225} I've tried to fix this by using watchman instead of fs.watch and on my tests with Windows build it works well: {F225467508} Also as watchman is more careful about opening file handles, hopefully this change will fix "too many files opened" problem as Michel suggested here https://fb.workplace.com/groups/flippersupport/permalink/764157990731528/ and here https://github.com/facebook/flipper/issues/699. Reviewed By: mweststrate Differential Revision: D19216026 fbshipit-source-id: acc53ae0d003a7936730e6423ac4dbca84f089c8 --- scripts/build-utils.js | 2 +- scripts/start-dev-server.js | 21 +++++--- static/compilePlugins.js | 49 +++++++++-------- static/watchman.js | 103 ++++++++++++++++++++++++++++++++++++ 4 files changed, 146 insertions(+), 29 deletions(-) create mode 100644 static/watchman.js diff --git a/scripts/build-utils.js b/scripts/build-utils.js index f78c9ca4e..e2953ba1d 100644 --- a/scripts/build-utils.js +++ b/scripts/build-utils.js @@ -66,7 +66,7 @@ function compile(buildFolder, entry) { ), }, resolver: { - blacklistRE: /\/(sonar|flipper|flipper-public)\/(dist|doctor)\/|(\.native\.js$)/, + blacklistRE: /(\/|\\)(sonar|flipper|flipper-public)(\/|\\)(dist|doctor)(\/|\\)|(\.native\.js$)/, }, }, { diff --git a/scripts/start-dev-server.js b/scripts/start-dev-server.js index 26201aa9d..32cf74b27 100644 --- a/scripts/start-dev-server.js +++ b/scripts/start-dev-server.js @@ -19,6 +19,7 @@ const http = require('http'); const path = require('path'); const Metro = require('../static/node_modules/metro'); const fs = require('fs'); +const Watchman = require('../static/watchman'); const convertAnsi = new Convert(); @@ -67,7 +68,7 @@ function startMetroServer(app) { ), }, resolver: { - blacklistRE: /\/(sonar|flipper|flipper-public)\/(dist|doctor)\/|(\.native\.js$)/, + blacklistRE: /(\/|\\)(sonar|flipper|flipper-public)(\/|\\)(dist|doctor)(\/|\\)|(\.native\.js$)/, }, watch: true, }).then(metroBundlerServer => { @@ -114,7 +115,7 @@ function startAssetServer(port) { }); } -function addWebsocket(server) { +async function addWebsocket(server) { const io = socketIo(server); // notify connected clients that there's errors in the console @@ -126,9 +127,17 @@ function addWebsocket(server) { // refresh the app on changes to the src folder // this can be removed once metroServer notifies us about file changes - fs.watch(path.join(__dirname, '..', 'src'), () => { - io.emit('refresh'); - }); + const watchman = new Watchman(path.resolve(__dirname, '..', 'src')); + await watchman.initialize(); + await watchman.startWatchFiles( + '/', + resp => { + io.emit('refresh'); + }, + { + excludes: ['**/__tests__/**/*', '**/node_modules/**/*', '**/.*'], + }, + ); return io; } @@ -186,7 +195,7 @@ function outputScreen(socket) { (async () => { const port = await detect(DEFAULT_PORT); const {app, server} = await startAssetServer(port); - const socket = addWebsocket(server); + const socket = await addWebsocket(server); await startMetroServer(app); outputScreen(socket); launchElectron({ diff --git a/static/compilePlugins.js b/static/compilePlugins.js index 22e7f6507..bf219aecc 100644 --- a/static/compilePlugins.js +++ b/static/compilePlugins.js @@ -16,6 +16,7 @@ const recursiveReaddir = require('recursive-readdir'); const expandTilde = require('expand-tilde'); const pMap = require('p-map'); const HOME_DIR = require('os').homedir(); +const Watchman = require('./watchman'); const DEFAULT_COMPILE_OPTIONS = { force: false, @@ -47,7 +48,7 @@ module.exports = async ( return dynamicPlugins; }; -function watchChanges( +async function watchChanges( plugins, reloadCallback, pluginCache, @@ -58,6 +59,9 @@ function watchChanges( const delayedCompilation = {}; const kCompilationDelayMillis = 1000; + const rootDir = path.resolve(__dirname, '..'); + const watchman = new Watchman(rootDir); + await watchman.initialize(); Object.values(plugins) // no hot reloading for plugins in .flipper folder. This is to prevent // Flipper from reloading, while we are doing changes on thirdparty plugins. @@ -65,26 +69,27 @@ function watchChanges( plugin => !plugin.rootDir.startsWith(path.join(HOME_DIR, '.flipper')), ) .map(plugin => - fs.watch(plugin.rootDir, {recursive: true}, (eventType, filename) => { - // only recompile for changes in not hidden files. Watchman might create - // a file called .watchman-cookie - if ( - filename && - !filename.startsWith('.') && - !filename.includes('__tests__') && - !delayedCompilation[plugin.name] - ) { - delayedCompilation[plugin.name] = setTimeout(() => { - delayedCompilation[plugin.name] = null; - // eslint-disable-next-line no-console - console.log(`🕵️‍ Detected changes in ${plugin.name}`); - const watchOptions = Object.assign(options, {force: true}); - compilePlugin(plugin, pluginCache, watchOptions).then( - reloadCallback, - ); - }, kCompilationDelayMillis); - } - }), + watchman.startWatchFiles( + path.relative(rootDir, plugin.rootDir), + resp => { + // only recompile for changes in not hidden files. Watchman might create + // a file called .watchman-cookie + if (!delayedCompilation[plugin.name]) { + delayedCompilation[plugin.name] = setTimeout(() => { + delayedCompilation[plugin.name] = null; + // eslint-disable-next-line no-console + console.log(`🕵️‍ Detected changes in ${plugin.name}`); + const watchOptions = Object.assign(options, {force: true}); + compilePlugin(plugin, pluginCache, watchOptions).then( + reloadCallback, + ); + }, kCompilationDelayMillis); + } + }, + { + excludes: ['**/__tests__/**/*', '**/node_modules/**/*', '**/.*'], + }, + ), ); } function hash(string) { @@ -215,7 +220,7 @@ async function compilePlugin( }, resolver: { sourceExts: ['tsx', 'ts', 'js'], - blacklistRE: /\/(sonar|flipper|flipper-public)\/(dist|doctor)\/|(\.native\.js$)/, + blacklistRE: /(\/|\\)(sonar|flipper|flipper-public)(\/|\\)(dist|doctor)(\/|\\)|(\.native\.js$)/, }, }, { diff --git a/static/watchman.js b/static/watchman.js new file mode 100644 index 000000000..7f01d7f30 --- /dev/null +++ b/static/watchman.js @@ -0,0 +1,103 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + */ + +const watchman = require('fb-watchman'); +const uuid = require('uuid'); +const path = require('path'); + +module.exports = class Watchman { + constructor(rootDir) { + this.rootDir = rootDir; + } + + async initialize() { + if (this.client) { + return; + } + this.client = new watchman.Client(); + this.client.setMaxListeners(250); + return new Promise((resolve, reject) => { + this.client.capabilityCheck( + {optional: [], required: ['relative_root']}, + error => { + if (error) { + this.client.end(); + delete this.client; + return reject(error); + } + this.client.command( + ['watch-project', this.rootDir], + (error, resp) => { + if (error) { + this.client.end(); + delete this.client; + return reject(error); + } + if ('warning' in resp) { + console.warn(resp.warning); + } + this.watch = resp.watch; + this.relativeRoot = resp.relative_path; + resolve(); + }, + ); + }, + ); + }); + } + + async startWatchFiles(relativeDir, handler, options) { + if (!this.watch) { + throw new Error( + 'Watchman is not initialized, please call "initialize" function and wait for the returned promise completion before calling "startWatchFiles".', + ); + } + options = Object.assign({excludes: []}, options); + return new Promise((resolve, reject) => { + this.client.command(['clock', this.watch], (error, resp) => { + if (error) { + return reject(error); + } + + const {clock} = resp; + + const sub = { + expression: [ + 'allof', + ['not', ['type', 'd']], + ...options.excludes.map(e => ['not', ['match', e, 'wholename']]), + ], + fields: ['name'], + since: clock, + relative_root: this.relativeRoot + ? path.join(this.relativeRoot, relativeDir) + : relativeDir, + }; + + const id = uuid.v4(); + + this.client.command( + ['subscribe', this.watch, id, sub], + (error, resp) => { + if (error) { + return reject(error); + } + this.client.on('subscription', resp => { + if (resp.subscription !== id || !resp.files) { + return; + } + handler(resp); + }); + resolve(); + }, + ); + }); + }); + } +};