🏗♻️ Refactor / clean up the watch logic used for extensions

build-system/tasks/build.js

@@ -67,7 +67,7 @@ async function performBuild(watch) {
       buildAlp({watch}),
       buildExaminer({watch}),
       buildWebWorker({watch}),
-      buildExtensions({bundleOnlyIfListedInFiles: !watch, watch}),
+      buildExtensions({watch}),
       compileAllUnminifiedTargets(watch),
     ]);
   });

build-system/tasks/css.js

@@ -127,7 +127,6 @@ function compileCss(watch, opt_compileAll) {
   return promise
     .then(() =>
       buildExtensions({
-        bundleOnlyIfListedInFiles: false,
         compileOnlyCss: true,
         compileAll: opt_compileAll,
       })

build-system/tasks/dist.js

@@ -18,12 +18,10 @@ const colors = require('ansi-colors');
 const file = require('gulp-file');
 const fs = require('fs-extra');
 const gulp = require('gulp');
-const gulpWatch = require('gulp-watch');
 const log = require('fancy-log');
 const {
   buildAlp,
   buildExaminer,
-  buildExperiments,
   buildWebWorker,
   compileAllMinifiedTargets,
   compileJs,
@@ -267,12 +265,58 @@ function buildWebPushPublisherFile(version, fileName, watch, options) {
     });
 }
 
+/**
+ * Build all the AMP experiments.html/js.
+ *
+ * @param {!Object} options
+ */
+function buildExperiments(options) {
+  options = options || {};
+  const path = 'tools/experiments';
+  const htmlPath = path + '/experiments.html';
+  const jsPath = path + '/experiments.js';
+
+  // Build HTML.
+  const html = fs.readFileSync(htmlPath, 'utf8');
+  const minHtml = html.replace(
+    '/dist.tools/experiments/experiments.js',
+    `https://${hostname}/v0/experiments.js`
+  );
+  gulp
+    .src(htmlPath)
+    .pipe(file('experiments.cdn.html', minHtml))
+    .pipe(gulp.dest('dist.tools/experiments/'));
+
+  // Build JS.
+  const js = fs.readFileSync(jsPath, 'utf8');
+  const builtName = 'experiments.max.js';
+  const minifiedName = 'experiments.js';
+  return toPromise(
+    gulp
+      .src(path + '/*.js')
+      .pipe(file(builtName, js))
+      .pipe(gulp.dest('build/experiments/'))
+  ).then(function() {
+    return compileJs(
+      './build/experiments/',
+      builtName,
+      './dist.tools/experiments/',
+      {
+        watch: false,
+        minify: options.minify || argv.minify,
+        includePolyfills: true,
+        minifiedName,
+      }
+    );
+  });
+}
+
 /**
  * Build "Login Done" page.
  *
  * @param {!Object} options
  */
-async function buildLoginDone(options) {
+function buildLoginDone(options) {
   return buildLoginDoneVersion('0.1', options);
 }
 
@@ -282,28 +326,12 @@ async function buildLoginDone(options) {
  * @param {string} version
  * @param {!Object} options
  */
-async function buildLoginDoneVersion(version, options) {
+function buildLoginDoneVersion(version, options) {
   options = options || {};
   const path = `extensions/amp-access/${version}/`;
   const buildDir = `build/all/amp-access-${version}/`;
   const htmlPath = path + 'amp-login-done.html';
   const jsPath = path + 'amp-login-done.js';
-  let {watch} = options;
-  if (watch === undefined) {
-    watch = argv.watch || argv.w;
-  }
-
-  // Building extensions is a 2 step process because of the renaming
-  // and CSS inlining. This watcher watches the original file, copies
-  // it to the destination and adds the CSS.
-  if (watch) {
-    // Do not set watchers again when we get called by the watcher.
-    const copy = Object.create(options);
-    copy.watch = false;
-    gulpWatch(path + '/*', function() {
-      buildLoginDoneVersion(version, copy);
-    });
-  }
 
   // Build HTML.
   const html = fs.readFileSync(htmlPath, 'utf8');
@@ -346,15 +374,11 @@ async function buildLoginDoneVersion(version, options) {
 }
 
 module.exports = {
-  buildExperiments,
-  buildLoginDone,
   dist,
 };
 
 /* eslint "google-camelcase/google-camelcase": 0 */
 
-buildExperiments.description = 'Builds experiments.html/js';
-buildLoginDone.description = 'Builds login-done.html/js';
 dist.description = 'Build production binaries';
 dist.flags = {
   pseudo_names:

build-system/tasks/extension-helpers.js

@@ -17,7 +17,6 @@
 const colors = require('ansi-colors');
 const fs = require('fs-extra');
 const log = require('fancy-log');
-const minimatch = require('minimatch');
 const watch = require('gulp-watch');
 const wrappers = require('../compile-wrappers');
 const {
@@ -56,7 +55,6 @@ const MINIMAL_EXTENSION_SET = [
  *   loadPriority: ?string,
  *   cssBinaries: ?Array<string>,
  *   extraGlobs?Array<string>,
- *   bundleOnlyIfListedInFiles: ?boolean
  * }}
  */
 const ExtensionOption = {}; // eslint-disable-line no-unused-vars
@@ -357,34 +355,22 @@ function buildExtension(
   if (options.compileOnlyCss && !hasCss) {
     return Promise.resolve();
   }
+  // Use a separate watcher for extensions to copy / inline CSS and compile JS
+  // instead of relying on the watcher used by compileUnminifiedJs, which only
+  // recompiles JS.
   const path = 'extensions/' + name + '/' + version;
-  const jsPath = path + '/' + name + '.js';
-  const jsTestPath = path + '/test/test-' + name + '.js';
-  if (argv.files && options.bundleOnlyIfListedInFiles) {
-    const passedFiles = Array.isArray(argv.files) ? argv.files : [argv.files];
-    const shouldBundle = passedFiles.some(glob => {
-      return minimatch(jsPath, glob) || minimatch(jsTestPath, glob);
-    });
-    if (!shouldBundle) {
-      return Promise.resolve();
-    }
-  }
-  // Building extensions is a 2 step process because of the renaming
-  // and CSS inlining. This watcher watches the original file, copies
-  // it to the destination and adds the CSS.
+  const optionsCopy = Object.create(options);
   if (options.watch) {
-    // Do not set watchers again when we get called by the watcher.
-    const copy = Object.create(options);
-    copy.watch = false;
+    optionsCopy.watch = false;
     watch(path + '/*', function() {
-      buildExtension(name, version, latestVersion, hasCss, copy);
+      buildExtension(name, version, latestVersion, hasCss, optionsCopy);
     });
   }
   let promise = Promise.resolve();
   if (hasCss) {
     mkdirSync('build');
     mkdirSync('build/css');
-    promise = buildExtensionCss(path, name, version, options);
+    promise = buildExtensionCss(path, name, version, optionsCopy);
     if (options.compileOnlyCss) {
       return promise;
     }
@@ -393,7 +379,7 @@ function buildExtension(
     if (argv.single_pass) {
       return Promise.resolve();
     } else {
-      return buildExtensionJs(path, name, version, latestVersion, options);
+      return buildExtensionJs(path, name, version, latestVersion, optionsCopy);
     }
   });
 }

build-system/tasks/helpers.js

@@ -475,14 +475,7 @@ function compileUnminifiedJs(srcDir, srcFilename, destDir, options) {
       });
   }
 
-  // Due to the two step build process for extensions, compileJs() is called
-  // twice, once with options.watch set to true and, once with it set to false.
-  // However, we do not need to call rebundle() twice. This avoids the duplicate
-  // compile seen when you run `gulp watch` and touch a file.
-  // TODO (rsimha): Figure out why this is needed and simplify buildExtension().
-  return options.watch === false
-    ? Promise.resolve()
-    : performBundle(/* failOnError */ true);
+  return performBundle(/* failOnError */ true);
 }
 
 /**
@@ -675,68 +668,6 @@ function thirdPartyBootstrap(input, outputName, shouldMinify) {
   });
 }
 
-/**
- * Build all the AMP experiments.html/js.
- *
- * @param {!Object} options
- */
-async function buildExperiments(options) {
-  options = options || {};
-  const path = 'tools/experiments';
-  const htmlPath = path + '/experiments.html';
-  const jsPath = path + '/experiments.js';
-  let {watch} = options;
-  if (watch === undefined) {
-    watch = argv.watch || argv.w;
-  }
-
-  // Building extensions is a 2 step process because of the renaming
-  // and CSS inlining. This watcher watches the original file, copies
-  // it to the destination and adds the CSS.
-  if (watch) {
-    // Do not set watchers again when we get called by the watcher.
-    const copy = Object.create(options);
-    copy.watch = false;
-    gulpWatch(path + '/*', function() {
-      buildExperiments(copy);
-    });
-  }
-
-  // Build HTML.
-  const html = fs.readFileSync(htmlPath, 'utf8');
-  const minHtml = html.replace(
-    '/dist.tools/experiments/experiments.js',
-    `https://${hostname}/v0/experiments.js`
-  );
-  gulp
-    .src(htmlPath)
-    .pipe(file('experiments.cdn.html', minHtml))
-    .pipe(gulp.dest('dist.tools/experiments/'));
-
-  // Build JS.
-  const js = fs.readFileSync(jsPath, 'utf8');
-  const builtName = 'experiments.max.js';
-  const minifiedName = 'experiments.js';
-  return toPromise(
-    gulp
-      .src(path + '/*.js')
-      .pipe(file(builtName, js))
-      .pipe(gulp.dest('build/experiments/'))
-  ).then(function() {
-    return compileJs(
-      './build/experiments/',
-      builtName,
-      './dist.tools/experiments/',
-      {
-        watch: false,
-        minify: options.minify || argv.minify,
-        includePolyfills: true,
-        minifiedName,
-      }
-    );
-  });
-}
-
 /**
  * Build ALP JS.
  *
@@ -815,7 +746,6 @@ function toPromise(readable) {
 module.exports = {
   buildAlp,
   buildExaminer,
-  buildExperiments,
   buildWebWorker,
   compileAllMinifiedTargets,
   compileAllUnminifiedTargets,

gulpfile.js

@@ -17,11 +17,6 @@
 /* global require */
 
 const gulp = require('gulp-help')(require('gulp'));
-const {
-  buildExperiments,
-  buildLoginDone,
-  dist,
-} = require('./build-system/tasks/dist');
 const {
   compileAccessExpr,
   compileBindExpr,
@@ -48,6 +43,7 @@ const {css} = require('./build-system/tasks/css');
 const {csvifySize} = require('./build-system/tasks/csvify-size');
 const {depCheck} = require('./build-system/tasks/dep-check');
 const {devDashboardTests} = require('./build-system/tasks/dev-dashboard-tests');
+const {dist} = require('./build-system/tasks/dist');
 const {e2e} = require('./build-system/tasks/e2e');
 const {firebase} = require('./build-system/tasks/firebase');
 const {getZindex} = require('./build-system/tasks/get-zindex');
@@ -71,8 +67,6 @@ const {visualDiff} = require('./build-system/tasks/visual-diff');
 gulp.task('ava', ava);
 gulp.task('babel-plugin-tests', babelPluginTests);
 gulp.task('build', build);
-gulp.task('build-experiments', buildExperiments);
-gulp.task('build-login-done', buildLoginDone);
 gulp.task('bundle-size', bundleSize);
 gulp.task('caches-json', cachesJson);
 gulp.task('changelog', changelog);