🏗🚮🐛 Assorted gulp compilation fixes (#22135)

ampproject · May 3, 2019 · b814e5d · b814e5d
1 parent e228909
commit b814e5d
Show file tree

Hide file tree

Showing 7 changed files with 32 additions and 66 deletions.
diff --git a/build-system/tasks/css.js b/build-system/tasks/css.js
@@ -92,15 +92,12 @@ function compileCss(watch, opt_compileAll) {
    * @param {string} outCss
    */
   function writeCssEntryPoint(path, outJs, outCss) {
-    const startTime = Date.now();
-
     return jsifyCssAsync(`css/${path}`)
-        .then(css => writeCss(css, path, outJs, outCss))
-        .then(() => {
-          endBuildStep('Recompiled CSS in', path, startTime);
-        });
+        .then(css => writeCss(css, path, outJs, outCss));
   }
 
+  const startTime = Date.now();
+
   // Used by `gulp test --local-changes` to map CSS files to JS files.
   fs.writeFileSync('EXTENSIONS_CSS_MAP', JSON.stringify(extensions));
 
@@ -116,7 +113,9 @@ function compileCss(watch, opt_compileAll) {
     bundleOnlyIfListedInFiles: false,
     compileOnlyCss: true,
     compileAll: opt_compileAll,
-  }));
+  })).then(() => {
+    endBuildStep('Recompiled all CSS files into', 'build/', startTime);
+  });
 }
 
 module.exports = {

diff --git a/build-system/tasks/dist.js b/build-system/tasks/dist.js
@@ -51,14 +51,6 @@ const {maybeUpdatePackages} = require('./update-packages');
 const {green, cyan} = colors;
 const argv = require('minimist')(process.argv.slice(2));
 
-// Minified targets to which AMP_CONFIG must be written.
-const minifiedRuntimeTarget = 'dist/v0.js';
-const minifiedShadowRuntimeTarget = 'dist/shadow-v0.js';
-const minifiedAdsTarget = 'dist/amp4ads-v0.js';
-// TODO(#18934, erwinm): temporary fix.
-//const minifiedRuntimeEsmTarget = 'dist/v0-esm.js';
-const minified3pTarget = 'dist.3p/current-min/f.js';
-
 /**
  * Dist Build
  * @return {!Promise}
@@ -119,19 +111,16 @@ async function dist() {
       }).then(() => {
         if (argv.fortesting) {
           return Promise.all([
-            enableLocalTesting(minifiedRuntimeTarget),
-            enableLocalTesting(minifiedAdsTarget),
-            enableLocalTesting(minifiedShadowRuntimeTarget),
-          ]).then(() => {
-            if (!argv.single_pass) {
-              // TODO(#18934, erwinm): temporary fix.
-              //return enableLocalTesting(minifiedRuntimeEsmTarget)
-              return enableLocalTesting(minifiedShadowRuntimeTarget)
-                  .then(() => {
-                    return enableLocalTesting(minifiedAdsTarget);
-                  });
-            }
-          });
+            enableLocalTesting('dist/v0.js'),
+            enableLocalTesting('dist/amp4ads-v0.js'),
+            enableLocalTesting('dist/shadow-v0.js'),
+          ]);
+          // TODO(#18934, erwinm): Re-enable when the ESM build is fixed.
+          // .then(() => {
+          //   if (!argv.single_pass) {
+          //     return enableLocalTesting('dist/v0-esm.js')
+          //   }
+          // });
         }
       }).then(() => {
         if (argv.esm) {
@@ -145,7 +134,7 @@ async function dist() {
         }
       }).then(() => {
         if (argv.fortesting) {
-          return enableLocalTesting(minified3pTarget);
+          return enableLocalTesting('dist.3p/current-min/f.js');
         }
       }).then(() => exitCtrlcHandler(handlerProcess));
 }

diff --git a/build-system/tasks/extension-helpers.js b/build-system/tasks/extension-helpers.js
@@ -21,7 +21,7 @@ const minimatch = require('minimatch');
 const watch = require('gulp-watch');
 const wrappers = require('../compile-wrappers');
 const {aliasBundles, extensionBundles, verifyExtensionBundles, verifyExtensionAliasBundles} = require('../../bundles.config');
-const {compileJs, endBuildStep, mkdirSync} = require('./helpers');
+const {compileJs, mkdirSync} = require('./helpers');
 const {isTravisBuild} = require('../travis');
 const {jsifyCssAsync} = require('./jsify-css');
 
@@ -355,10 +355,7 @@ function buildExtension(
   if (hasCss) {
     mkdirSync('build');
     mkdirSync('build/css');
-    const startTime = Date.now();
-    promise = buildExtensionCss(path, name, version, options).then(() => {
-      endBuildStep('Recompiled CSS in', `${name}/${version}`, startTime);
-    });
+    promise = buildExtensionCss(path, name, version, options);
     if (options.compileOnlyCss) {
       return promise;
     }

diff --git a/build-system/tasks/firebase.js b/build-system/tasks/firebase.js
@@ -58,9 +58,9 @@ async function modifyThirdPartyUrl() {
 async function firebase() {
   if (!argv.nobuild) {
     if (argv.min) {
-      dist();
+      await dist();
     } else {
-      build();
+      await build();
     }
   }
   await fs.mkdirp('firebase');

diff --git a/build-system/tasks/helpers.js b/build-system/tasks/helpers.js
@@ -64,13 +64,6 @@ const EXTENSION_BUNDLE_MAP = {
 const hostname = argv.hostname || 'cdn.ampproject.org';
 const hostname3p = argv.hostname3p || '3p.ampproject.net';
 
-// Unminified targets to which AMP_CONFIG must be written.
-const unminifiedRuntimeTarget = 'dist/amp.js';
-const unminifiedShadowRuntimeTarget = 'dist/amp-shadow.js';
-const unminifiedAdsTarget = 'dist/amp-inabox.js';
-const unminifiedRuntimeEsmTarget = 'dist/amp-esm.js';
-const unminified3pTarget = 'dist.3p/current/integration.js';
-
 /**
  * Compile and optionally minify the stylesheets and the scripts
  * and drop them in the dist folder
@@ -343,16 +336,7 @@ function compileJs(srcDir, srcFilename, destDir, options) {
         });
   }
 
-  const startTime = Date.now();
-  let bundler = browserify(entryPoint, {debug: true})
-      .transform(babelify)
-      .once('transform', () => {
-        let name = srcFilename;
-        if (options.name && options.version) {
-          name = `${options.name}-${options.version}.js`;
-        }
-        endBuildStep('Transformed', name, startTime);
-      });
+  let bundler = browserify(entryPoint, {debug: true}).transform(babelify);
   if (options.watch) {
     bundler = watchify(bundler);
   }
@@ -430,17 +414,17 @@ function compileJs(srcDir, srcFilename, destDir, options) {
         .then(() => {
           if (process.env.NODE_ENV === 'development') {
             if (destFilename === 'amp.js') {
-              return enableLocalTesting(unminifiedRuntimeTarget);
+              return enableLocalTesting('dist/amp.js');
             } else if (destFilename === 'amp-esm.js') {
-              return enableLocalTesting(unminifiedRuntimeEsmTarget);
+              return enableLocalTesting('dist/amp-esm.js');
             } else if (destFilename === 'amp4ads-v0.js') {
-              return enableLocalTesting(unminifiedAdsTarget);
+              return enableLocalTesting('dist/amp4ads-v0.js');
             } else if (destFilename === 'integration.js') {
-              return enableLocalTesting(unminified3pTarget);
+              return enableLocalTesting('dist.3p/current/integration.js');
             } else if (destFilename === 'amp-shadow.js') {
-              return enableLocalTesting(unminifiedShadowRuntimeTarget);
+              return enableLocalTesting('dist/amp-shadow.js');
             } else if (destFilename === 'amp-inabox.js') {
-              return enableLocalTesting(unminifiedAdsTarget);
+              return enableLocalTesting('dist/amp-inabox.js');
             } else {
               return Promise.resolve();
             }
@@ -531,7 +515,7 @@ function printNobuildHelp() {
  * Called at the end of "gulp build" and "gulp dist --fortesting".
  * @param {string} targetFile File to which the config is to be written.
  */
-function enableLocalTesting(targetFile) {
+async function enableLocalTesting(targetFile) {
   const config = (argv.config === 'canary') ? 'canary' : 'prod';
   const baseConfigFile =
       'build-system/global-configs/' + config + '-config.json';

diff --git a/build-system/tasks/prepend-global/index.js b/build-system/tasks/prepend-global/index.js
@@ -200,9 +200,6 @@ function removeConfig(target) {
       .then(file => {
         let contents = file.toString();
         if (numConfigs(contents) == 0) {
-          if (!isTravisBuild()) {
-            log('No configs found in', cyan(target));
-          }
           return Promise.resolve();
         }
         sanityCheck(contents);

diff --git a/build-system/tasks/runtime-test/index.js b/build-system/tasks/runtime-test/index.js
@@ -535,12 +535,12 @@ async function runTests() {
   }
 }
 
-function test() {
+async function test() {
   if (!argv.nobuild) {
     if (argv.unit || argv.a4a || argv['local-changes']) {
-      css();
+      await css();
     } else {
-      build();
+      await build();
     }
   }
   // TODO(alanorozco): Come up with a more elegant check?