Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

馃彈 Apply AMP_CONFIG to runtime files during gulp dist #26554

Merged
merged 3 commits into from Feb 3, 2020
Merged

馃彈 Apply AMP_CONFIG to runtime files during gulp dist #26554

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7d13c8e
Apply `AMP_CONFIG` by default during `gulp dist`
rsimha Jan 29, 2020
bec0bd9
Update documentation
rsimha Jan 30, 2020
aea5881
Change `--no_amp_config` to `--noconfig`
rsimha Jan 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion build-system/tasks/dist.js
View file
Open in desktop
Expand Up @@ -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. ' +
Expand All @@ -438,6 +439,7 @@ dist.flags = {
' Outputs compiled code with whitespace. ' +
'Great for debugging production code.',
fortesting: ' Compiles production binaries for local testing',
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.',
Expand Down
9 changes: 6 additions & 3 deletions build-system/tasks/firebase.js
View file
Open in desktop
Expand Up @@ -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 = [];
Expand Down Expand Up @@ -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),
]);
}

Expand Down
30 changes: 20 additions & 10 deletions build-system/tasks/helpers.js
View file
Open in desktop
Expand Up @@ -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.noconfig && MINIFIED_TARGETS.includes(minifiedName)) {
return applyAmpConfig(
`${destDir}/${minifiedName}`,
/* localDev */ !!argv.fortesting
);
rsimha marked this conversation as resolved.
Show resolved Hide resolved
}
})
.then(() => {
if (!argv.fortesting || !options.singlePassCompilation) {
if (argv.noconfig || !options.singlePassCompilation) {
return;
}
return Promise.all(
altMainBundles.map(({name}) => enableLocalTesting(`dist/${name}.js`))
altMainBundles.map(({name}) =>
applyAmpConfig(`dist/${name}.js`, /* localDev */ !!argv.fortesting)
)
);
});
}
Expand Down Expand Up @@ -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
);
}
});
}
Expand Down Expand Up @@ -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';
Expand All @@ -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
Expand Down Expand Up @@ -696,6 +706,7 @@ function transferSrcsToTempDir(options = {}) {
}

module.exports = {
applyAmpConfig,
BABELIFY_GLOBAL_TRANSFORM,
BABELIFY_PLUGINS,
bootstrapThirdPartyFrames,
Expand All @@ -706,7 +717,6 @@ module.exports = {
compileTs,
devDependencies,
doBuildJs,
enableLocalTesting,
endBuildStep,
hostname,
mkdirSync,
Expand Down
3 changes: 2 additions & 1 deletion contributing/TESTING.md
View file
Open in desktop
Expand Up @@ -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 --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. |
Expand Down