From 66ac049c38148f62ecebc3a34c7b3cf595227362 Mon Sep 17 00:00:00 2001 From: Raghu Simha Date: Mon, 3 Jun 2019 15:02:52 -0400 Subject: [PATCH 1/5] Move buildExperiments to dist.js --- build-system/tasks/dist.js | 63 ++++++++++++++++++++++++++++++++++- build-system/tasks/helpers.js | 63 ----------------------------------- 2 files changed, 62 insertions(+), 64 deletions(-) diff --git a/build-system/tasks/dist.js b/build-system/tasks/dist.js index 1304fd8d6e65..8ec6bd7bc6c1 100644 --- a/build-system/tasks/dist.js +++ b/build-system/tasks/dist.js @@ -23,7 +23,6 @@ const log = require('fancy-log'); const { buildAlp, buildExaminer, - buildExperiments, buildWebWorker, compileAllMinifiedTargets, compileJs, @@ -267,6 +266,68 @@ function buildWebPushPublisherFile(version, fileName, watch, options) { }); } +/** + * 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 "Login Done" page. * diff --git a/build-system/tasks/helpers.js b/build-system/tasks/helpers.js index 818b337c666e..5fa774cb41b9 100644 --- a/build-system/tasks/helpers.js +++ b/build-system/tasks/helpers.js @@ -675,68 +675,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 +753,6 @@ function toPromise(readable) { module.exports = { buildAlp, buildExaminer, - buildExperiments, buildWebWorker, compileAllMinifiedTargets, compileAllUnminifiedTargets, From 822994739412e83146281ca04ec5c7849ac87a9f Mon Sep 17 00:00:00 2001 From: Raghu Simha Date: Mon, 3 Jun 2019 16:22:03 -0400 Subject: [PATCH 2/5] Remove unused watch logic from buildLoginDoneVersion and buildExperiments --- build-system/tasks/dist.js | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/build-system/tasks/dist.js b/build-system/tasks/dist.js index 8ec6bd7bc6c1..bc7895fe4f0d 100644 --- a/build-system/tasks/dist.js +++ b/build-system/tasks/dist.js @@ -18,7 +18,6 @@ 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, @@ -276,22 +275,6 @@ async function buildExperiments(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'); @@ -349,22 +332,6 @@ async function buildLoginDoneVersion(version, options) { 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'); From cc90111b120d8906efbafd1d8c3f6a3c433cca27 Mon Sep 17 00:00:00 2001 From: Raghu Simha Date: Mon, 3 Jun 2019 16:23:58 -0400 Subject: [PATCH 3/5] Simplify the watch logic for extensions --- build-system/tasks/extension-helpers.js | 17 ++++++++--------- build-system/tasks/helpers.js | 9 +-------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/build-system/tasks/extension-helpers.js b/build-system/tasks/extension-helpers.js index 6cba6dd623b4..494260f180a0 100644 --- a/build-system/tasks/extension-helpers.js +++ b/build-system/tasks/extension-helpers.js @@ -369,22 +369,21 @@ function buildExtension( 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. + // 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 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 +392,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); } }); } diff --git a/build-system/tasks/helpers.js b/build-system/tasks/helpers.js index 5fa774cb41b9..d7f6b5b6a17d 100644 --- a/build-system/tasks/helpers.js +++ b/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); } /** From c7c023432bf1cc35f3d4e2df65f8a7a038bd29bd Mon Sep 17 00:00:00 2001 From: Raghu Simha Date: Mon, 3 Jun 2019 17:25:33 -0400 Subject: [PATCH 4/5] Remove obsolete bundleOnlyIfListedInFiles, since we now have --extensions --- build-system/tasks/build.js | 2 +- build-system/tasks/css.js | 1 - build-system/tasks/extension-helpers.js | 15 +-------------- 3 files changed, 2 insertions(+), 16 deletions(-) diff --git a/build-system/tasks/build.js b/build-system/tasks/build.js index 46f342f522b9..7e75a19a21a7 100644 --- a/build-system/tasks/build.js +++ b/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), ]); }); diff --git a/build-system/tasks/css.js b/build-system/tasks/css.js index b9a4ca455325..1b914882811f 100644 --- a/build-system/tasks/css.js +++ b/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, }) diff --git a/build-system/tasks/extension-helpers.js b/build-system/tasks/extension-helpers.js index 494260f180a0..3e368e936b87 100644 --- a/build-system/tasks/extension-helpers.js +++ b/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, * extraGlobs?Array, - * bundleOnlyIfListedInFiles: ?boolean * }} */ const ExtensionOption = {}; // eslint-disable-line no-unused-vars @@ -357,21 +355,10 @@ function buildExtension( if (options.compileOnlyCss && !hasCss) { return Promise.resolve(); } - 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(); - } - } // 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 optionsCopy = Object.create(options); if (options.watch) { optionsCopy.watch = false; From ca16027924c611e1a398c322cc74fe4b3b9d2754 Mon Sep 17 00:00:00 2001 From: Raghu Simha Date: Mon, 3 Jun 2019 18:34:58 -0400 Subject: [PATCH 5/5] Remove obsolete `gulp` tasks `build-experiments` and `build-login-done` --- build-system/tasks/dist.js | 10 +++------- gulpfile.js | 8 +------- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/build-system/tasks/dist.js b/build-system/tasks/dist.js index bc7895fe4f0d..4709316042cd 100644 --- a/build-system/tasks/dist.js +++ b/build-system/tasks/dist.js @@ -270,7 +270,7 @@ function buildWebPushPublisherFile(version, fileName, watch, options) { * * @param {!Object} options */ -async function buildExperiments(options) { +function buildExperiments(options) { options = options || {}; const path = 'tools/experiments'; const htmlPath = path + '/experiments.html'; @@ -316,7 +316,7 @@ async function buildExperiments(options) { * * @param {!Object} options */ -async function buildLoginDone(options) { +function buildLoginDone(options) { return buildLoginDoneVersion('0.1', options); } @@ -326,7 +326,7 @@ 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}/`; @@ -374,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: diff --git a/gulpfile.js b/gulpfile.js index 0d481cbfc0bc..02373cded386 100644 --- a/gulpfile.js +++ b/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);