Skip to content

Commit

Permalink
Revert "Improve AMP_CONFIG handling during build (#35773)" (#35819)
Browse files Browse the repository at this point in the history
  • Loading branch information
samouri committed Aug 26, 2021
1 parent 0b4682b commit 84ce446
Show file tree
Hide file tree
Showing 9 changed files with 148 additions and 106 deletions.
13 changes: 4 additions & 9 deletions build-system/compile/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ const {checkForUnknownDeps} = require('./check-for-unknown-deps');
const {CLOSURE_SRC_GLOBS} = require('./sources');
const {cpus} = require('os');
const {cyan, green} = require('../common/colors');
const {getAmpConfigForFile} = require('../tasks/prepend-global');
const {log, logLocalDev} = require('../common/logging');
const {postClosureBabel} = require('./post-closure-babel');
const {preClosureBabel} = require('./pre-closure-babel');
Expand Down Expand Up @@ -186,9 +185,9 @@ function getSrcs(entryModuleFilenames, options) {
*
* @param {string} outputFilename
* @param {!OptionsDef} options
* @return {!Promise<!Object>}
* @return {!Object}
*/
async function generateCompilerOptions(outputFilename, options) {
function generateCompilerOptions(outputFilename, options) {
// Determine externs
let externs = options.externs || [];
if (!options.noAddDeps) {
Expand Down Expand Up @@ -221,11 +220,10 @@ async function generateCompilerOptions(outputFilename, options) {
if (argv.pseudo_names) {
define.push('PSEUDO_NAMES=true');
}
const ampConfig = await getAmpConfigForFile(outputFilename, options);
let wrapper = options.wrapper
? options.wrapper.replace('<%= contents %>', '%output%')
: `(function(){%output%})();`;
wrapper = `${ampConfig}${wrapper}\n\n//# sourceMappingURL=${outputFilename}.map`;
wrapper = `${wrapper}\n\n//# sourceMappingURL=${outputFilename}.map`;

/**
* TODO(#28387) write a type for this.
Expand Down Expand Up @@ -402,10 +400,7 @@ async function compile(
}
const destFile = `${outputDir}/${outputFilename}`;
const sourcemapFile = `${destFile}.map`;
const compilerOptions = await generateCompilerOptions(
outputFilename,
options
);
const compilerOptions = generateCompilerOptions(outputFilename, options);
const srcs = options.noAddDeps
? entryModuleFilenames.concat(options.extraGlobs || [])
: getSrcs(entryModuleFilenames, options);
Expand Down
21 changes: 19 additions & 2 deletions build-system/compile/post-closure-babel.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,21 @@ const path = require('path');
const Remapping = require('@ampproject/remapping');
const terser = require('terser');
const {CompilationLifecycles, debug} = require('./debug-compilation-lifecycle');
const {jsBundles} = require('./bundles.config');

/** @type {Remapping.default} */
const remapping = /** @type {*} */ (Remapping);

let mainBundles;

/**
* Minify passed string.
*
* @param {string} code
* @param {string} filename
* @return {Promise<Object<string, terser.SourceMapOptions['content']>>}
*/
async function terserMinify(code) {
async function terserMinify(code, filename) {
const options = {
mangle: false,
compress: {
Expand All @@ -31,6 +35,19 @@ async function terserMinify(code) {
},
sourceMap: true,
};
const basename = path.basename(filename, argv.esm ? '.mjs' : '.js');
if (!mainBundles) {
mainBundles = Object.keys(jsBundles).map((key) => {
const bundle = jsBundles[key];
if (bundle.options && bundle.options.minifiedName) {
return path.basename(bundle.options.minifiedName, '.js');
}
return path.basename(key, '.js');
});
}
if (mainBundles.includes(basename)) {
options.output.preamble = ';';
}
const minified = await terser.minify(code, options);

return {
Expand Down Expand Up @@ -61,7 +78,7 @@ async function postClosureBabel(file) {
}

debug(CompilationLifecycles['closured-pre-terser'], file, code, babelMap);
const {compressed, terserMap} = await terserMinify(code);
const {compressed, terserMap} = await terserMinify(code, path.basename(file));
await fs.outputFile(file, compressed);

const closureMap = await fs.readJson(`${file}.map`, 'utf-8');
Expand Down
2 changes: 1 addition & 1 deletion build-system/pr-check/module-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

const argv = require('minimist')(process.argv.slice(2));
const {MINIFIED_TARGETS} = require('../tasks/prepend-global');
const {MINIFIED_TARGETS} = require('../tasks/helpers');
const {runCiJob} = require('./ci-job');
const {skipDependentJobs, timedExecOrDie} = require('./utils');
const {Targets, buildTargetsInclude} = require('./build-targets');
Expand Down
2 changes: 1 addition & 1 deletion build-system/pr-check/nomodule-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const {
timedExecOrDie,
timedExecOrThrow,
} = require('./utils');
const {MINIFIED_TARGETS} = require('../tasks/prepend-global');
const {MINIFIED_TARGETS} = require('../tasks/helpers');
const {runCiJob} = require('./ci-job');
const {Targets, buildTargetsInclude} = require('./build-targets');

Expand Down
69 changes: 63 additions & 6 deletions build-system/tasks/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ const wrappers = require('../compile/compile-wrappers');
const {
VERSION: internalRuntimeVersion,
} = require('../compile/internal-version');
const {applyConfig, removeConfig} = require('./prepend-global');
const {closureCompile} = require('../compile/compile');
const {cyan, green, red} = require('../common/colors');
const {getAmpConfigForFile} = require('./prepend-global');
const {getEsbuildBabelPlugin} = require('../common/esbuild-babel');
const {getSourceRoot} = require('../compile/helpers');
const {isCiBuild} = require('../common/ci');
Expand Down Expand Up @@ -52,6 +52,16 @@ const EXTENSION_BUNDLE_MAP = {
],
};

/**
* List of unminified targets to which AMP_CONFIG should be written
*/
const UNMINIFIED_TARGETS = ['alp.max', 'amp-inabox', 'amp-shadow', 'amp'];

/**
* List of minified targets to which AMP_CONFIG should be written
*/
const MINIFIED_TARGETS = ['alp', 'amp4ads-v0', 'shadow-v0', 'v0'];

/**
* Used while building the 3p frame
**/
Expand Down Expand Up @@ -310,6 +320,15 @@ async function compileMinifiedJs(srcDir, srcFilename, destDir, options) {
name += ` → ${maybeToEsmName(options.latestName)}`;
}
endBuildStep('Minified', name, timeInfo.startTime);

const target = path.basename(minifiedName, path.extname(minifiedName));
if (!argv.noconfig && MINIFIED_TARGETS.includes(target)) {
await applyAmpConfig(
maybeToEsmName(`${destDir}/${minifiedName}`),
/* localDev */ options.fortesting,
/* fortesting */ options.fortesting
);
}
}

/**
Expand Down Expand Up @@ -375,6 +394,16 @@ async function finishBundle(
: destFilename;
endBuildStep(logPrefix, loggingName, startTime);
}

const targets = options.minify ? MINIFIED_TARGETS : UNMINIFIED_TARGETS;
const target = path.basename(destFilename, path.extname(destFilename));
if (targets.includes(target)) {
await applyAmpConfig(
path.join(destDir, destFilename),
/* localDev */ true,
/* fortesting */ options.fortesting
);
}
}

/**
Expand Down Expand Up @@ -405,7 +434,7 @@ async function esbuildCompile(srcDir, srcFilename, destDir, options) {
* @return {Object}
*/
function splitWrapper() {
const wrapper = options.wrapper ?? wrappers.none;
const wrapper = options.wrapper || wrappers.none;
const sentinel = '<%= contents %>';
const start = wrapper.indexOf(sentinel);
return {
Expand All @@ -414,10 +443,6 @@ async function esbuildCompile(srcDir, srcFilename, destDir, options) {
};
}
const {banner, footer} = splitWrapper();
const config = await getAmpConfigForFile(srcFilename, options);
if (config) {
banner.js = config + banner.js;
}

const babelPlugin = getEsbuildBabelPlugin(
options.minify ? 'minified' : 'unminified',
Expand Down Expand Up @@ -470,6 +495,7 @@ async function esbuildCompile(srcDir, srcFilename, destDir, options) {
fs.outputFile(`${destFile}.map`, map),
]);

// TODO: finishBundle should operate in-mem instead of reading/writing from FS.
await finishBundle(srcFilename, destDir, destFilename, options, startTime);
}

Expand Down Expand Up @@ -545,6 +571,8 @@ async function minify(code, map, {mangle} = {mangle: false}) {
beautify: !!argv.pretty_print,
// eslint-disable-next-line google-camelcase/google-camelcase
keep_quoted_props: true,
// // TODO: only add preamble for mainBundles?
preamble: ';',
},
sourceMap: {content: map},
module: !!argv.esm,
Expand Down Expand Up @@ -704,6 +732,33 @@ async function maybePrintCoverageMessage(covPath = '') {
await open(url, {wait: false});
}

/**
* Writes AMP_CONFIG to a runtime file. Optionally enables localDev mode and
* fortesting mode. Called by "amp build" and "amp 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.
* @param {boolean} fortesting Whether or not to enable testing mode.
* @return {!Promise}
*/
async function applyAmpConfig(targetFile, localDev, fortesting) {
const config = argv.config === 'canary' ? 'canary' : 'prod';
const baseConfigFile =
'build-system/global-configs/' + config + '-config.json';

await removeConfig(targetFile);
await applyConfig(
config,
targetFile,
baseConfigFile,
/* opt_localDev */ localDev,
/* opt_localBranch */ true,
/* opt_branch */ undefined,
/* opt_fortesting */ fortesting
);
}

/**
* Copies frame.html to output folder, replaces js references to minified
* copies, and generates symlink to it.
Expand Down Expand Up @@ -816,6 +871,8 @@ function shouldUseClosure() {
}

module.exports = {
MINIFIED_TARGETS,
applyAmpConfig,
bootstrapThirdPartyFrames,
compileAllJs,
compileCoreRuntime,
Expand Down

0 comments on commit 84ce446

Please sign in to comment.