From 7d13c8e777d9cfb0e0754420846006ff33c4a52e Mon Sep 17 00:00:00 2001 From: Raghu Simha Date: Wed, 29 Jan 2020 17:36:04 -0500 Subject: [PATCH 1/3] Apply `AMP_CONFIG` by default during `gulp dist` --- build-system/tasks/dist.js | 4 +++- build-system/tasks/firebase.js | 9 ++++++--- build-system/tasks/helpers.js | 30 ++++++++++++++++++++---------- 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/build-system/tasks/dist.js b/build-system/tasks/dist.js index cc86a73f2333..a7dd613fb6b3 100644 --- a/build-system/tasks/dist.js +++ b/build-system/tasks/dist.js @@ -429,7 +429,8 @@ module.exports = { /* eslint "google-camelcase/google-camelcase": 0 */ -dist.description = 'Build production binaries'; +dist.description = + 'Compiles AMP production binaries and applies AMP_CONFIG to runtime files'; dist.flags = { pseudo_names: ' Compiles with readable names. ' + @@ -438,6 +439,7 @@ dist.flags = { ' Outputs compiled code with whitespace. ' + 'Great for debugging production code.', fortesting: ' Compiles production binaries for local testing', + no_amp_config: ' Compiles production binaries without applying AMP_CONFIG', config: ' Sets the runtime\'s AMP_CONFIG to one of "prod" or "canary"', single_pass: "Compile AMP's primary JS bundles in a single invocation", extensions: ' Builds only the listed extensions.', diff --git a/build-system/tasks/firebase.js b/build-system/tasks/firebase.js index b17a4c97b047..82db707e2329 100644 --- a/build-system/tasks/firebase.js +++ b/build-system/tasks/firebase.js @@ -18,10 +18,10 @@ const colors = require('ansi-colors'); const fs = require('fs-extra'); const log = require('fancy-log'); const path = require('path'); +const {applyAmpConfig} = require('./helpers'); const {build} = require('./build'); const {clean} = require('./clean'); const {dist} = require('./dist'); -const {enableLocalTesting} = require('./helpers'); async function walk(dest) { const filelist = []; @@ -80,8 +80,11 @@ async function firebase() { if (argv.fortesting) { await Promise.all([ - enableLocalTesting('firebase/dist.3p/current/integration.js'), - enableLocalTesting('firebase/dist/amp.js'), + applyAmpConfig( + 'firebase/dist.3p/current/integration.js', + /* localDev */ true + ), + applyAmpConfig('firebase/dist/amp.js', /* localDev */ true), ]); } diff --git a/build-system/tasks/helpers.js b/build-system/tasks/helpers.js index 75cba87bb5b0..a59aa13e9f49 100644 --- a/build-system/tasks/helpers.js +++ b/build-system/tasks/helpers.js @@ -295,16 +295,21 @@ function compileMinifiedJs(srcDir, srcFilename, destDir, options) { endBuildStep('Minified', name, timeInfo.startTime); }) .then(() => { - if (argv.fortesting && MINIFIED_TARGETS.includes(minifiedName)) { - return enableLocalTesting(`${destDir}/${minifiedName}`); + if (!argv.no_amp_config && MINIFIED_TARGETS.includes(minifiedName)) { + return applyAmpConfig( + `${destDir}/${minifiedName}`, + /* localDev */ !!argv.fortesting + ); } }) .then(() => { - if (!argv.fortesting || !options.singlePassCompilation) { + if (argv.no_amp_config || !options.singlePassCompilation) { return; } return Promise.all( - altMainBundles.map(({name}) => enableLocalTesting(`dist/${name}.js`)) + altMainBundles.map(({name}) => + applyAmpConfig(`dist/${name}.js`, /* localDev */ !!argv.fortesting) + ) ); }); } @@ -443,7 +448,10 @@ function compileUnminifiedJs(srcDir, srcFilename, destDir, options) { }) .then(() => { if (UNMINIFIED_TARGETS.includes(destFilename)) { - return enableLocalTesting(`${destDir}/${destFilename}`); + return applyAmpConfig( + `${destDir}/${destFilename}`, + /* localDev */ true + ); } }); } @@ -555,12 +563,14 @@ function printNobuildHelp() { } /** - * Enables runtime to be used for local testing by writing AMP_CONFIG to file. - * Called at the end of "gulp build" and "gulp dist --fortesting". + * Writes AMP_CONFIG to a runtime file. Optionally enables localDev mode and + * fortesting mode. Called by "gulp build" and "gulp dist" while building + * various runtime files. * @param {string} targetFile File to which the config is to be written. + * @param {boolean} localDev Whether or not to enable local development. * @return {!Promise} */ -async function enableLocalTesting(targetFile) { +async function applyAmpConfig(targetFile, localDev) { const config = argv.config === 'canary' ? 'canary' : 'prod'; const baseConfigFile = 'build-system/global-configs/' + config + '-config.json'; @@ -570,7 +580,7 @@ async function enableLocalTesting(targetFile) { config, targetFile, baseConfigFile, - /* opt_localDev */ true, + /* opt_localDev */ localDev, /* opt_localBranch */ true, /* opt_branch */ false, /* opt_fortesting */ !!argv.fortesting @@ -696,6 +706,7 @@ function transferSrcsToTempDir(options = {}) { } module.exports = { + applyAmpConfig, BABELIFY_GLOBAL_TRANSFORM, BABELIFY_PLUGINS, bootstrapThirdPartyFrames, @@ -706,7 +717,6 @@ module.exports = { compileTs, devDependencies, doBuildJs, - enableLocalTesting, endBuildStep, hostname, mkdirSync, From bec0bd9946f86935219f935d9e12bc3839686ead Mon Sep 17 00:00:00 2001 From: Raghu Simha Date: Thu, 30 Jan 2020 11:23:27 -0500 Subject: [PATCH 2/3] Update documentation --- contributing/TESTING.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contributing/TESTING.md b/contributing/TESTING.md index cc64d1046f82..11fd63370f9c 100644 --- a/contributing/TESTING.md +++ b/contributing/TESTING.md @@ -48,7 +48,8 @@ Before running these commands, make sure you have Node.js, yarn, and Gulp instal | `gulp --extensions=minimal_set` | Runs "watch" and "serve", after building the extensions needed to load `article.amp.html`. | | `gulp --extensions_from=examples/foo.amp.html` | Runs "watch" and "serve", after building only extensions from the listed examples. | | `gulp --noextensions` | Runs "watch" and "serve" without building any extensions. | -| `gulp dist` | Builds production binaries. | +| `gulp dist` | Builds production binaries and applies AMP_CONFIG to runtime files. | +| `gulp dist --no_amp_config` | Builds production binaries without applying AMP_CONFIG to runtime files. | | `gulp dist --extensions=amp-foo,amp-bar` | Builds production binaries, with only the listed extensions. | | `gulp dist --extensions=minimal_set` | Builds production binaries, with only the extensions needed to load `article.amp.html`. | | `gulp dist --extensions_from=examples/foo.amp.html` | Builds production binaries, with only extensions from the listed examples. | From aea588133e74fdcc025b16eb3f43d1be6e89f386 Mon Sep 17 00:00:00 2001 From: Raghu Simha Date: Thu, 30 Jan 2020 12:28:13 -0500 Subject: [PATCH 3/3] Change `--no_amp_config` to `--noconfig` --- build-system/tasks/dist.js | 2 +- build-system/tasks/helpers.js | 4 ++-- contributing/TESTING.md | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/build-system/tasks/dist.js b/build-system/tasks/dist.js index a7dd613fb6b3..95e819397236 100644 --- a/build-system/tasks/dist.js +++ b/build-system/tasks/dist.js @@ -439,7 +439,7 @@ dist.flags = { ' Outputs compiled code with whitespace. ' + 'Great for debugging production code.', fortesting: ' Compiles production binaries for local testing', - no_amp_config: ' Compiles production binaries without applying AMP_CONFIG', + noconfig: ' Compiles production binaries without applying AMP_CONFIG', config: ' Sets the runtime\'s AMP_CONFIG to one of "prod" or "canary"', single_pass: "Compile AMP's primary JS bundles in a single invocation", extensions: ' Builds only the listed extensions.', diff --git a/build-system/tasks/helpers.js b/build-system/tasks/helpers.js index a59aa13e9f49..911ee31e1aa0 100644 --- a/build-system/tasks/helpers.js +++ b/build-system/tasks/helpers.js @@ -295,7 +295,7 @@ function compileMinifiedJs(srcDir, srcFilename, destDir, options) { endBuildStep('Minified', name, timeInfo.startTime); }) .then(() => { - if (!argv.no_amp_config && MINIFIED_TARGETS.includes(minifiedName)) { + if (!argv.noconfig && MINIFIED_TARGETS.includes(minifiedName)) { return applyAmpConfig( `${destDir}/${minifiedName}`, /* localDev */ !!argv.fortesting @@ -303,7 +303,7 @@ function compileMinifiedJs(srcDir, srcFilename, destDir, options) { } }) .then(() => { - if (argv.no_amp_config || !options.singlePassCompilation) { + if (argv.noconfig || !options.singlePassCompilation) { return; } return Promise.all( diff --git a/contributing/TESTING.md b/contributing/TESTING.md index 11fd63370f9c..fb34b5334d63 100644 --- a/contributing/TESTING.md +++ b/contributing/TESTING.md @@ -49,7 +49,7 @@ Before running these commands, make sure you have Node.js, yarn, and Gulp instal | `gulp --extensions_from=examples/foo.amp.html` | Runs "watch" and "serve", after building only extensions from the listed examples. | | `gulp --noextensions` | Runs "watch" and "serve" without building any extensions. | | `gulp dist` | Builds production binaries and applies AMP_CONFIG to runtime files. | -| `gulp dist --no_amp_config` | Builds production binaries without applying AMP_CONFIG to runtime files. | +| `gulp dist --noconfig` | Builds production binaries without applying AMP_CONFIG to runtime files. | | `gulp dist --extensions=amp-foo,amp-bar` | Builds production binaries, with only the listed extensions. | | `gulp dist --extensions=minimal_set` | Builds production binaries, with only the extensions needed to load `article.amp.html`. | | `gulp dist --extensions_from=examples/foo.amp.html` | Builds production binaries, with only extensions from the listed examples. |