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.

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

Already on GitHub? Sign in to your account

Revert "Improve AMP_CONFIG handling during build" #35819

Merged
merged 1 commit into from
Aug 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading