Improve AMP_CONFIG handling during build (#35773)

* applyAmpConfig: also update sourcemaps * no stone unturned: AMP_CONFIG should be part of the wrapper * update comments * update comment + fix bug * another bugfix * allow newline between ; and AMP_CONFIG * retain logs for dist|build * smol clean * Update build-system/tasks/prepend-global/index.js Co-authored-by: Raghu Simha <rsimha@amp.dev> * Update build-system/tasks/prepend-global/index.js Co-authored-by: Raghu Simha <rsimha@amp.dev> * target --> filename Co-authored-by: Raghu Simha <rsimha@amp.dev>
ampproject · Aug 25, 2021 · 4e191f0 · 4e191f0
1 parent 6b5c951
commit 4e191f0
Show file tree

Hide file tree

Showing 9 changed files with 106 additions and 148 deletions.
diff --git a/build-system/compile/compile.js b/build-system/compile/compile.js
@@ -8,6 +8,7 @@ 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');
@@ -185,9 +186,9 @@ function getSrcs(entryModuleFilenames, options) {
  *
  * @param {string} outputFilename
  * @param {!OptionsDef} options
- * @return {!Object}
+ * @return {!Promise<!Object>}
  */
-function generateCompilerOptions(outputFilename, options) {
+async function generateCompilerOptions(outputFilename, options) {
   // Determine externs
   let externs = options.externs || [];
   if (!options.noAddDeps) {
@@ -220,10 +221,11 @@ 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 = `${wrapper}\n\n//# sourceMappingURL=${outputFilename}.map`;
+  wrapper = `${ampConfig}${wrapper}\n\n//# sourceMappingURL=${outputFilename}.map`;
 
   /**
    * TODO(#28387) write a type for this.
@@ -400,7 +402,10 @@ async function compile(
   }
   const destFile = `${outputDir}/${outputFilename}`;
   const sourcemapFile = `${destFile}.map`;
-  const compilerOptions = generateCompilerOptions(outputFilename, options);
+  const compilerOptions = await generateCompilerOptions(
+    outputFilename,
+    options
+  );
   const srcs = options.noAddDeps
     ? entryModuleFilenames.concat(options.extraGlobs || [])
     : getSrcs(entryModuleFilenames, options);

diff --git a/build-system/compile/post-closure-babel.js b/build-system/compile/post-closure-babel.js
@@ -6,21 +6,17 @@ 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, filename) {
+async function terserMinify(code) {
   const options = {
     mangle: false,
     compress: {
@@ -35,19 +31,6 @@ async function terserMinify(code, filename) {
     },
     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 {
@@ -78,7 +61,7 @@ async function postClosureBabel(file) {
   }
 
   debug(CompilationLifecycles['closured-pre-terser'], file, code, babelMap);
-  const {compressed, terserMap} = await terserMinify(code, path.basename(file));
+  const {compressed, terserMap} = await terserMinify(code);
   await fs.outputFile(file, compressed);
 
   const closureMap = await fs.readJson(`${file}.map`, 'utf-8');

diff --git a/build-system/pr-check/module-tests.js b/build-system/pr-check/module-tests.js
@@ -5,7 +5,7 @@
  */
 
 const argv = require('minimist')(process.argv.slice(2));
-const {MINIFIED_TARGETS} = require('../tasks/helpers');
+const {MINIFIED_TARGETS} = require('../tasks/prepend-global');
 const {runCiJob} = require('./ci-job');
 const {skipDependentJobs, timedExecOrDie} = require('./utils');
 const {Targets, buildTargetsInclude} = require('./build-targets');

diff --git a/build-system/pr-check/nomodule-tests.js b/build-system/pr-check/nomodule-tests.js
@@ -10,7 +10,7 @@ const {
   timedExecOrDie,
   timedExecOrThrow,
 } = require('./utils');
-const {MINIFIED_TARGETS} = require('../tasks/helpers');
+const {MINIFIED_TARGETS} = require('../tasks/prepend-global');
 const {runCiJob} = require('./ci-job');
 const {Targets, buildTargetsInclude} = require('./build-targets');
 

diff --git a/build-system/tasks/helpers.js b/build-system/tasks/helpers.js
@@ -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');
@@ -52,16 +52,6 @@ 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
  **/
@@ -320,15 +310,6 @@ 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
-    );
-  }
 }
 
 /**
@@ -394,16 +375,6 @@ 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
-    );
-  }
 }
 
 /**
@@ -434,7 +405,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 {
@@ -443,6 +414,10 @@ 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',
@@ -495,7 +470,6 @@ 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);
   }
 
@@ -571,8 +545,6 @@ 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,
@@ -732,33 +704,6 @@ 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.
@@ -871,8 +816,6 @@ function shouldUseClosure() {
 }
 
 module.exports = {
-  MINIFIED_TARGETS,
-  applyAmpConfig,
   bootstrapThirdPartyFrames,
   compileAllJs,
   compileCoreRuntime,