🏗 Debounce all watch operations in the AMP dev toolchain (#27738)

build-system/tasks/css.js

@@ -14,12 +14,18 @@
  * limitations under the License.
  */
 
+const debounce = require('debounce');
 const file = require('gulp-file');
 const fs = require('fs-extra');
 const gulp = require('gulp');
 const gulpWatch = require('gulp-watch');
+const {
+  endBuildStep,
+  mkdirSync,
+  toPromise,
+  watchDebounceDelay,
+} = require('./helpers');
 const {buildExtensions, extensions} = require('./extension-helpers');
-const {endBuildStep, mkdirSync, toPromise} = require('./helpers');
 const {jsifyCssAsync} = require('./jsify-css');
 const {maybeUpdatePackages} = require('./update-packages');
 
@@ -75,9 +81,10 @@ const cssEntryPoints = [
  */
 function compileCss(watch) {
   if (watch) {
-    gulpWatch('css/**/*.css', function () {
+    const watchFunc = () => {
       compileCss();
-    });
+    };
+    gulpWatch('css/**/*.css', debounce(watchFunc, watchDebounceDelay));
   }
 
   /**

build-system/tasks/dist.js

@@ -25,7 +25,6 @@ const {
   compileCoreRuntime,
   compileJs,
   endBuildStep,
-  hostname,
   maybeToEsmName,
   mkdirSync,
   printConfigHelp,
@@ -55,13 +54,24 @@ const {VERSION} = require('../compile/internal-version');
 const {green, cyan} = colors;
 const argv = require('minimist')(process.argv.slice(2));
 
+/**
+ * Files that must be built for amp-web-push
+ */
 const WEB_PUSH_PUBLISHER_FILES = [
   'amp-web-push-helper-frame',
   'amp-web-push-permission-dialog',
 ];
 
+/**
+ * Versions that must be built for amp-web-push
+ */
 const WEB_PUSH_PUBLISHER_VERSIONS = ['0.1'];
 
+/**
+ * Used while building the experiments page.
+ */
+const hostname = argv.hostname || 'cdn.ampproject.org';
+
 /**
  * Prints a useful help message prior to the gulp dist task
  */

build-system/tasks/extension-helpers.js

@@ -15,6 +15,7 @@
  */
 
 const colors = require('ansi-colors');
+const debounce = require('debounce');
 const fs = require('fs-extra');
 const log = require('fancy-log');
 const watch = require('gulp-watch');
@@ -24,7 +25,7 @@ const {
   extensionBundles,
   verifyExtensionBundles,
 } = require('../compile/bundles.config');
-const {endBuildStep} = require('./helpers');
+const {endBuildStep, watchDebounceDelay} = require('./helpers');
 const {isTravisBuild} = require('../common/travis');
 const {jsifyCssAsync} = require('./jsify-css');
 const {maybeToEsmName, compileJs, mkdirSync} = require('./helpers');
@@ -380,7 +381,7 @@ async function doBuildExtension(extensions, extension, options) {
  * @param {?Object} options
  */
 function watchExtension(path, name, version, latestVersion, hasCss, options) {
-  watch(path + '/**/*', function () {
+  const watchFunc = function () {
     const bundleComplete = buildExtension(
       name,
       version,
@@ -391,7 +392,8 @@ function watchExtension(path, name, version, latestVersion, hasCss, options) {
     if (options.onWatchBuild) {
       options.onWatchBuild(bundleComplete);
     }
-  });
+  };
+  watch(path + '/**/*', debounce(watchFunc, watchDebounceDelay));
 }
 
 /**

build-system/tasks/helpers.js

@@ -18,6 +18,7 @@ const babelify = require('babelify');
 const browserify = require('browserify');
 const buffer = require('vinyl-buffer');
 const colors = require('ansi-colors');
+const debounce = require('debounce');
 const del = require('del');
 const file = require('gulp-file');
 const fs = require('fs-extra');
@@ -92,9 +93,16 @@ const MINIFIED_TARGETS = [
   'v0.js',
 ].map(maybeToEsmName);
 
-const hostname = argv.hostname || 'cdn.ampproject.org';
+/**
+ * Used while building the 3p frame
+ **/
 const hostname3p = argv.hostname3p || '3p.ampproject.net';
 
+/**
+ * Used to debounce file edits during watch to prevent races.
+ */
+const watchDebounceDelay = 1000;
+
 /**
  * @param {!Object} jsBundles
  * @param {string} name
@@ -131,9 +139,10 @@ async function bootstrapThirdPartyFrames(watch, minify) {
   });
   if (watch) {
     thirdPartyFrames.forEach((frameObject) => {
-      gulpWatch(frameObject.max, function () {
+      const watchFunc = () => {
         thirdPartyBootstrap(frameObject.max, frameObject.min, minify);
-      });
+      };
+      gulpWatch(frameObject.max, debounce(watchFunc, watchDebounceDelay));
     });
   }
   await Promise.all(promises);
@@ -253,14 +262,15 @@ async function compileMinifiedJs(srcDir, srcFilename, destDir, options) {
   const minifiedName = maybeToEsmName(options.minifiedName);
 
   if (options.watch) {
-    gulpWatch(entryPoint, async function () {
+    const watchFunc = async () => {
       const compileComplete = await doCompileMinifiedJs(
         /* continueOnError */ true
       );
       if (options.onWatchBuild) {
         options.onWatchBuild(compileComplete);
       }
-    });
+    };
+    gulpWatch(entryPoint, debounce(watchFunc, watchDebounceDelay));
   }
 
   async function doCompileMinifiedJs(continueOnError) {
@@ -390,13 +400,14 @@ function compileUnminifiedJs(srcDir, srcFilename, destDir, options) {
   });
 
   if (options.watch) {
-    bundler = watchify(bundler);
-    bundler.on('update', () => {
+    const watchFunc = () => {
       const bundleComplete = performBundle(/* continueOnError */ true);
       if (options.onWatchBuild) {
         options.onWatchBuild(bundleComplete);
       }
-    });
+    };
+    bundler = watchify(bundler);
+    bundler.on('update', debounce(watchFunc, watchDebounceDelay));
   }
 
   /**
@@ -673,10 +684,10 @@ module.exports = {
   compileTs,
   doBuildJs,
   endBuildStep,
-  hostname,
   maybeToEsmName,
   mkdirSync,
   printConfigHelp,
   printNobuildHelp,
   toPromise,
+  watchDebounceDelay,
 };

build-system/tasks/lint.js

@@ -18,6 +18,7 @@
 const argv = require('minimist')(process.argv.slice(2));
 const colors = require('ansi-colors');
 const config = require('../test-configs/config');
+const debounce = require('gulp-debounce');
 const eslint = require('gulp-eslint');
 const eslintIfFixed = require('gulp-eslint-if-fixed');
 const globby = require('globby');
@@ -30,40 +31,40 @@ const {getFilesChanged, logOnSameLine} = require('../common/utils');
 const {gitDiffNameOnlyMaster} = require('../common/git');
 const {isTravisBuild} = require('../common/travis');
 const {maybeUpdatePackages} = require('./update-packages');
-
-const isWatching = argv.watch || argv.w || false;
-const options = {
-  fix: false,
-  quiet: argv.quiet || false,
-};
+const {watchDebounceDelay} = require('./helpers');
 
 const rootDir = path.dirname(path.dirname(__dirname));
 
 /**
  * Initializes the linter stream based on globs
+ *
  * @param {!Object} globs
  * @param {!Object} streamOptions
  * @return {!ReadableStream}
  */
 function initializeStream(globs, streamOptions) {
   let stream = gulp.src(globs, streamOptions);
-  if (isWatching) {
+  if (argv.watch) {
     const watcher = lazypipe().pipe(watch, globs);
-    stream = stream.pipe(watcher());
+    stream = stream.pipe(watcher()).pipe(debounce({wait: watchDebounceDelay}));
   }
   return stream;
 }
 
 /**
  * Runs the linter on the given stream using the given options.
+ *
  * @param {!ReadableStream} stream
- * @param {!Object} options
  * @return {boolean}
  */
-function runLinter(stream, options) {
+function runLinter(stream) {
   if (!isTravisBuild()) {
     log(colors.green('Starting linter...'));
   }
+  const options = {
+    fix: argv.fix,
+    quiet: argv.quiet,
+  };
   const fixedFiles = {};
   return stream
     .pipe(eslint(options))
@@ -181,13 +182,11 @@ function getFilesToLint(files) {
 
 /**
  * Run the eslinter on the src javascript and log the output
- * @return {!Stream} Readable stream
+ *
+ * @return {!ReadableStream}
  */
 function lint() {
   maybeUpdatePackages();
-  if (argv.fix) {
-    options.fix = true;
-  }
   let filesToLint = config.lintGlobs;
   if (argv.files) {
     filesToLint = getFilesToLint(argv.files.split(','));
@@ -199,8 +198,7 @@ function lint() {
     }
     filesToLint = getFilesToLint(lintableFiles);
   }
-  const stream = initializeStream(filesToLint, {base: rootDir});
-  return runLinter(stream, options);
+  return runLinter(initializeStream(filesToLint, {base: rootDir}));
 }
 
 module.exports = {

build-system/tasks/presubmit-checks.js

@@ -1143,6 +1143,7 @@ const forbiddenTermsSrcInclusive = {
       'build-system/server/shadow-viewer.js',
       'build-system/server/variable-substitution.js',
       'build-system/tasks/check-links.js',
+      'build-system/tasks/dist.js',
       'build-system/tasks/extension-generator/index.js',
       'build-system/tasks/helpers.js',
       'build-system/tasks/performance/helpers.js',

build-system/tasks/serve.js

@@ -16,6 +16,7 @@
 'use strict';
 
 const connect = require('gulp-connect');
+const debounce = require('debounce');
 const globby = require('globby');
 const header = require('connect-header');
 const log = require('fancy-log');
@@ -34,6 +35,7 @@ const {cyan, green, red} = require('ansi-colors');
 const {distNailgunPort, stopNailgunServer} = require('./nailgun');
 const {exec} = require('../common/exec');
 const {logServeMode, setServeMode} = require('../server/app-utils');
+const {watchDebounceDelay} = require('./helpers');
 
 const argv = minimist(process.argv.slice(2), {string: ['rtv']});
 
@@ -196,9 +198,10 @@ async function serve() {
  */
 async function doServe(lazyBuild = false) {
   createCtrlcHandler('serve');
-  watch(serverFiles, async () => {
+  const watchFunc = async () => {
     await restartServer();
-  });
+  };
+  watch(serverFiles, debounce(watchFunc, watchDebounceDelay));
   if (argv.new_server) {
     buildNewServer();
   }

build-system/tasks/vendor-configs.js

@@ -16,6 +16,7 @@
 
 const minimist = require('minimist');
 const argv = minimist(process.argv.slice(2));
+const debounce = require('debounce');
 const globby = require('globby');
 const gulp = require('gulp');
 const gulpif = require('gulp-if');
@@ -24,6 +25,7 @@ const jsonlint = require('gulp-jsonlint');
 const jsonminify = require('gulp-jsonminify');
 const rename = require('gulp-rename');
 const {endBuildStep, toPromise} = require('./helpers');
+const {watchDebounceDelay} = require('./helpers');
 
 /**
  * Entry point for 'gulp vendor-configs'
@@ -45,9 +47,10 @@ async function vendorConfigs(opt_options) {
   if (options.watch) {
     // Do not set watchers again when we get called by the watcher.
     const copyOptions = {...options, watch: false, calledByWatcher: true};
-    gulpWatch(srcPath, function () {
+    const watchFunc = () => {
       vendorConfigs(copyOptions);
-    });
+    };
+    gulpWatch(srcPath, debounce(watchFunc, watchDebounceDelay));
   }
 
   const startTime = Date.now();

package.json

@@ -60,6 +60,7 @@
     "commander": "5.0.0",
     "connect-header": "0.0.5",
     "cssnano": "4.1.10",
+    "debounce": "1.2.0",
     "del": "5.1.0",
     "derequire": "2.1.0",
     "dev-null": "0.1.1",
@@ -97,6 +98,7 @@
     "gulp-ava": "3.0.0",
     "gulp-babel": "8.0.0",
     "gulp-connect": "5.7.0",
+    "gulp-debounce": "1.0.2",
     "gulp-eslint": "6.0.0",
     "gulp-eslint-if-fixed": "1.0.0",
     "gulp-file": "0.4.0",

yarn.lock

@@ -5482,6 +5482,11 @@ dateformat@^1.0.7-1.2.3:
     get-stdin "^4.0.1"
     meow "^3.3.0"
 
+debounce@1.2.0:
+  version "1.2.0"
+  resolved "https://registry.yarnpkg.com/debounce/-/debounce-1.2.0.tgz#44a540abc0ea9943018dc0eaa95cce87f65cd131"
+  integrity sha512-mYtLl1xfZLi1m4RtQYlZgJUNQjl4ZxVnHzIR8nLLgi4q1YT8o/WM+MK/f8yfcc9s5Ir5zRaPZyZU6xs1Syoocg==
+
 debug-fabulous@1.X:
   version "1.1.0"
   resolved "https://registry.yarnpkg.com/debug-fabulous/-/debug-fabulous-1.1.0.tgz#af8a08632465224ef4174a9f06308c3c2a1ebc8e"
@@ -7988,6 +7993,13 @@ gulp-connect@5.7.0:
     serve-static "^1.13.2"
     tiny-lr "^1.1.1"
 
+gulp-debounce@1.0.2:
+  version "1.0.2"
+  resolved "https://registry.yarnpkg.com/gulp-debounce/-/gulp-debounce-1.0.2.tgz#0b943e812973e3003f980d88fc0d77e7996d9db9"
+  integrity sha1-C5Q+gSlz4wA/mA2I/A1355ltnbk=
+  dependencies:
+    through "^2.3.8"
+
 gulp-eslint-if-fixed@1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/gulp-eslint-if-fixed/-/gulp-eslint-if-fixed-1.0.0.tgz#ebad1f2a92e2c6ed793b01daaf0081dcaaca0e26"