🏗♻️ Refactor and speed up unminified builds (#22635)

ampproject · Jun 3, 2019 · 250c06e · 250c06e
1 parent 22f66ae
commit 250c06e
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 69 deletions.
diff --git a/build-system/tasks/helpers.js b/build-system/tasks/helpers.js
@@ -23,14 +23,12 @@ const file = require('gulp-file');
 const fs = require('fs-extra');
 const gulp = require('gulp');
 const gulpWatch = require('gulp-watch');
-const lazypipe = require('lazypipe');
 const log = require('fancy-log');
 const path = require('path');
 const regexpSourcemaps = require('gulp-regexp-sourcemaps');
 const rename = require('gulp-rename');
 const source = require('vinyl-source-stream');
 const sourcemaps = require('gulp-sourcemaps');
-const touch = require('touch');
 const watchify = require('watchify');
 const wrappers = require('../compile-wrappers');
 const {altMainBundles} = require('../../bundles.config');
@@ -417,66 +415,47 @@ function finishBundle(srcFilename, destDir, destFilename, options) {
  */
 function compileUnminifiedJs(srcDir, srcFilename, destDir, options) {
   const entryPoint = path.join(srcDir, srcFilename);
-  let bundler = browserify(entryPoint, {debug: true}).transform(babelify);
-  if (options.watch) {
-    bundler = watchify(bundler);
-  }
-
-  // Default wrapper for `gulp build`.
-  // We don't need an explicit function wrapper like we do for `gulp dist`
-  // because Babel handles that for you.
+  const destFilename = options.toName || srcFilename;
   const wrapper = options.wrapper || wrappers.none;
   const devWrapper = wrapper.replace('<%= contents %>', '$1');
 
-  const lazybuild = lazypipe()
-    .pipe(
-      source,
-      srcFilename
-    )
-    .pipe(buffer)
-    .pipe(
-      sourcemaps.init.bind(sourcemaps),
-      {loadMaps: true}
-    )
-    .pipe(
-      regexpSourcemaps,
-      /\$internalRuntimeVersion\$/g,
-      internalRuntimeVersion,
-      'runtime-version'
-    )
-    .pipe(
-      regexpSourcemaps,
-      /([^]+)/,
-      devWrapper,
-      'wrapper'
-    );
+  let bundler = browserify({
+    entries: entryPoint,
+    debug: true,
+  }).transform(babelify);
 
-  const lazywrite = lazypipe()
-    .pipe(
-      sourcemaps.write.bind(sourcemaps),
-      './'
-    )
-    .pipe(
-      gulp.dest.bind(gulp),
-      destDir
-    );
+  if (options.watch) {
+    bundler = watchify(bundler);
+    bundler.on('update', () => performBundle(/* failOnError */ false));
+  }
 
-  const destFilename = options.toName || srcFilename;
   /**
    * @param {boolean} failOnError
    * @return {Promise}
    */
-  function rebundle(failOnError) {
-    const startTime = Date.now();
+  function performBundle(failOnError) {
+    let startTime;
     return toPromise(
       bundler
         .bundle()
+        .once('readable', () => (startTime = Date.now()))
         .on('error', err =>
           handleBundleError(err, failOnError, srcFilename, startTime)
         )
-        .pipe(lazybuild())
+        .pipe(source(srcFilename))
+        .pipe(buffer())
+        .pipe(sourcemaps.init({loadMaps: true}))
+        .pipe(
+          regexpSourcemaps(
+            /\$internalRuntimeVersion\$/g,
+            internalRuntimeVersion,
+            'runtime-version'
+          )
+        )
+        .pipe(regexpSourcemaps(/([^]+)/, devWrapper, 'wrapper'))
         .pipe(rename(destFilename))
-        .pipe(lazywrite())
+        .pipe(sourcemaps.write('.'))
+        .pipe(gulp.dest(destDir))
         .on('end', () =>
           finishBundle(srcFilename, destDir, destFilename, options)
         )
@@ -496,27 +475,14 @@ function compileUnminifiedJs(srcDir, srcFilename, destDir, options) {
       });
   }
 
-  if (options.watch) {
-    bundler.on('update', function() {
-      rebundle(/* failOnError */ false);
-      // Touch file in unit test set. This triggers rebundling of tests because
-      // karma only considers changes to tests files themselves re-bundle
-      // worthy.
-      touch('test/_init_tests.js');
-    });
-  }
-
-  if (options.watch === false) {
-    // Due to the two step build process, 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.
-    return Promise.resolve();
-  } else {
-    // This is the default options.watch === true case, and also covers the
-    // `gulp build` / `gulp dist` cases where options.watch is undefined.
-    return rebundle(/* failOnError */ true);
-  }
+  // 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);
 }
 
 /**

diff --git a/package.json b/package.json
@@ -167,7 +167,6 @@
     "text-table": "0.2.0",
     "through2": "3.0.1",
     "topological-sort": "0.3.0",
-    "touch": "3.1.0",
     "traverse": "0.6.6",
     "try-net-connect": "3.0.2",
     "tsickle": "0.35.0",

diff --git a/yarn.lock b/yarn.lock
@@ -14876,7 +14876,7 @@ topological-sort@0.3.0:
   resolved "https://registry.yarnpkg.com/topological-sort/-/topological-sort-0.3.0.tgz#3ca539982c71a13f0a7b3be2ad4ae4dd437a1e3b"
   integrity sha512-BmO2t72jjJ3B9PZC3LTHyojgNynOKEvWG4GiYqyfSMUjtO6+yZK0oozlMYVA+tC6YvS3NZh4OPx6QaolEYOg6A==
 
-touch@3.1.0, touch@^3.1.0:
+touch@^3.1.0:
   version "3.1.0"
   resolved "https://registry.yarnpkg.com/touch/-/touch-3.1.0.tgz#fe365f5f75ec9ed4e56825e0bb76d24ab74af83b"
   integrity sha512-WBx8Uy5TLtOSRtIq+M03/sKDrXCLHxwDcquSP2c43Le03/9serjQBIztjRz6FkJez9D/hleyAXTBGLwwZUw9lA==