🐛 Make prepending AMP_CONFIG sourcemap aware (#37640)

* Trace through babel without requiring babelMaps handling

I don't know why I didn't do this originally, it's sooo much simpler. This has babel output an inline sourcemap, which is fed into esbuild. Esbuild will do an initial sourcemap-aware bundling, and output a new map that correctly points through babel into our real source files.

* Fix prepending AMP_CONFIG with esbuild+terser output

build-system/babel-config/minified-config.js

@@ -77,7 +77,7 @@ function getMinifiedConfig() {
   return {
     compact: false,
     plugins,
-    sourceMaps: 'both',
+    sourceMaps: true,
     presets: [presetTypescript, presetEnv],
     retainLines: true,
     assumptions: {

build-system/babel-config/unminified-config.js

@@ -55,7 +55,7 @@ function getUnminifiedConfig() {
     compact: false,
     plugins: unminifiedPlugins,
     presets: unminifiedPresets,
-    sourceMaps: 'both',
+    sourceMaps: true,
     assumptions: {
       constantSuper: true,
       noClassCalls: true,

build-system/common/esbuild-babel.js

@@ -2,11 +2,18 @@ const babel = require('@babel/core');
 const path = require('path');
 const {debug} = require('../compile/debug-compilation-lifecycle');
 const {TransformCache, batchedRead, md5} = require('./transform-cache');
+const Remapping = require('@ampproject/remapping');
+
+/** @type {Remapping.default} */
+const remapping = /** @type {*} */ (Remapping);
+
+const argv = require('minimist')(process.argv.slice(2));
 
 /**
  * @typedef {{
  *   filename: string,
  *   code: string,
+ *   map: Object,
  * }}
  */
 let CacheMessageDef;
@@ -33,6 +40,8 @@ function getEsbuildBabelPlugin(
   enableCache,
   {preSetup = () => {}, postLoad = () => {}} = {}
 ) {
+  const babelMaps = new Map();
+
   /**
    * @param {string} filename
    * @param {string} contents
@@ -57,14 +66,14 @@ function getEsbuildBabelPlugin(
       .then((result) => {
         const {code, map} = /** @type {!babel.BabelFileResult} */ (result);
         debug('post-babel', filename, code, map);
-        return {filename, code: code || ''};
+        return {filename, code: code || '', map};
       });
 
     if (enableCache) {
       transformCache.set(hash, promise);
     }
 
-    return promise.finally(postLoad);
+    return promise;
   }
 
   return {
@@ -73,12 +82,21 @@ function getEsbuildBabelPlugin(
     async setup(build) {
       preSetup();
 
+      const {initialOptions} = build;
+      const {sourcemap} = initialOptions;
+      const inlineSourcemap = sourcemap === 'inline' || sourcemap === 'both';
+      if (inlineSourcemap) {
+        initialOptions.sorucemap = true;
+      }
+
       build.onLoad(
         {filter: /\.(cjs|mjs|js|jsx|ts|tsx)$/, namespace: ''},
         async (file) => {
           const filename = file.path;
-          const babelOptions =
-            babel.loadOptions({caller: {name: callerName}, filename}) || {};
+          const babelOptions = /** @type {*} */ (
+            babel.loadOptions({caller: {name: callerName}, filename}) || {}
+          );
+          babelOptions.sourceMaps = true;
 
           const {contents, hash} = await batchedRead(filename);
           const rehash = md5(
@@ -97,11 +115,89 @@ function getEsbuildBabelPlugin(
             rehash,
             getFileBabelOptions(babelOptions, filename)
           );
+
+          babelMaps.set(filename, transformed.map);
+          postLoad?.();
           return {contents: transformed.code};
         }
       );
+
+      build.onEnd(async (result) => {
+        const {outputFiles} = result;
+        const code = outputFiles.find(({path}) => !path.endsWith('.map'));
+        const map = outputFiles.find(({path}) => path.endsWith('.map'));
+
+        if (!map) {
+          return;
+        }
+
+        const root = path.dirname(map.path);
+        const remapped = remapping(
+          map.text,
+          (f, ctx) => {
+            // The Babel tranformed file and the original file have the same
+            // path, which makes it difficult to distinguish during remapping's
+            // load phase. To prevent an infinite recursion, we check if the
+            // importer is ourselves (which is nonsensical) and early exit.
+            if (f === ctx.importer) {
+              return null;
+            }
+
+            const file = path.join(root, f);
+            const map = babelMaps.get(file);
+            if (!map) {
+              if (file.includes('/node_modules/')) {
+                // Excuse node_modules since they may have been marked external
+                // (and so not processed by babel).
+                return null;
+              }
+              throw new Error(`failed to find sourcemap for babel file "${f}"`);
+            }
+            return map;
+          },
+          !argv.full_sourcemaps
+        );
+
+        const sourcemapJson = remapped.toString();
+        replaceOutputFile(outputFiles, map, sourcemapJson);
+        if (inlineSourcemap) {
+          const base64 = Buffer.from(sourcemapJson).toString('base64');
+          replaceOutputFile(
+            outputFiles,
+            code,
+            code.text.replace(
+              /sourceMappingURL=.*/,
+              `sourceMappingURL=data:application/json;charset=utf-8;base64,${base64}`
+            )
+          );
+        }
+      });
+    },
+  };
+}
+
+/**
+ * @param {Array<import('esbuild').OutputFile>} outputFiles
+ * @param {import('esbuild').OutputFile} original
+ * @param {string} text
+ */
+function replaceOutputFile(outputFiles, original, text) {
+  const index = outputFiles.indexOf(original);
+  if (index === -1) {
+    throw new Error(`couldn't find outputFile ${original.path}`);
+  }
+
+  let contents;
+  const file = {
+    path: original.path,
+    text,
+    get contents() {
+      // eslint-disable-next-line local/no-forbidden-terms
+      const te = new TextEncoder();
+      return (contents ||= te.encode(text));
     },
   };
+  outputFiles[index] = file;
 }
 
 const CJS_TRANSFORMS = new Set([

build-system/tasks/check-sourcemaps.js

@@ -113,7 +113,7 @@ function checkSourcemapMappings(sourcemapJson, map) {
   }
 
   // See https://www.npmjs.com/package/sourcemap-codec#usage.
-  const firstLineMapping = decode(sourcemapJson.mappings)[0][0];
+  const firstLineMapping = decode(sourcemapJson.mappings)[1][0];
   const [, sourceIndex = 0, sourceCodeLine = 0, sourceCodeColumn] =
     firstLineMapping;
 

build-system/tasks/helpers.js

@@ -236,11 +236,7 @@ function maybeToNpmEsmName(name) {
  * @param {string} destFilename
  */
 function handleBundleError(err, continueOnError, destFilename) {
-  let message = err.toString();
-  if (err.stack) {
-    // Drop the node_modules call stack, which begins with '    at'.
-    message = err.stack.replace(/    at[^]*/, '').trim();
-  }
+  const message = err.stack || err.toString();
   log(red('ERROR:'), message, '\n');
   const reasonMessage = `Could not compile ${cyan(destFilename)}`;
   if (continueOnError) {
@@ -347,7 +343,6 @@ async function esbuildCompile(srcDir, srcFilename, destDir, options) {
         entryPoints: [entryPoint],
         bundle: true,
         sourcemap: 'external',
-        sourceRoot: path.dirname(destFile),
         sourcesContent: !!argv.full_sourcemaps,
         outfile: destFile,
         define: experimentDefines,
@@ -375,7 +370,7 @@ async function esbuildCompile(srcDir, srcFilename, destDir, options) {
     const {outputFiles} = result;
 
     let code = outputFiles.find(({path}) => !path.endsWith('.map')).text;
-    let map = JSON.parse(
+    const map = JSON.parse(
       result.outputFiles.find(({path}) => path.endsWith('.map')).text
     );
     const mapChain = [map];
@@ -402,12 +397,13 @@ async function esbuildCompile(srcDir, srcFilename, destDir, options) {
       code = result.code;
       mapChain.unshift(result.map);
     }
-    map = massageSourcemaps(mapChain, options);
 
-    const sourceMapComment = `\n//# sourceMappingURL=${destFilename}.map`;
     await Promise.all([
-      fs.outputFile(destFile, `${code}\n${sourceMapComment}`),
-      fs.outputJson(`${destFile}.map`, map),
+      fs.outputFile(
+        destFile,
+        `${code}\n//# sourceMappingURL=${destFilename}.map`
+      ),
+      fs.outputJson(`${destFile}.map`, massageSourcemaps(mapChain, options)),
     ]);
 
     await finishBundle(destDir, destFilename, options, startTime);
@@ -534,6 +530,11 @@ async function minify(code, options = {}) {
     output: {
       beautify: !!argv.pretty_print,
       keep_quoted_props: true,
+      // The AMP Cache will prepend content on the first line during serving
+      // (for AMP_CONFIG and AMP_EXP). In order for these to not affect
+      // the sourcemap, we must ensure there is no other content on the first
+      // line. If you remove this you will annoy Justin. Don't do it.
+      preamble: ';',
     },
     sourceMap: true,
     toplevel: true,

build-system/tasks/release/index.js

@@ -248,6 +248,9 @@ async function compileDistFlavors_(flavorType, command, tempDir) {
   if (argv.esm) {
     command += ' --esm';
   }
+  if (argv.full_sourcemaps) {
+    command += ' --full_sourcemaps';
+  }
   log('Compiling flavor', green(flavorType), 'using', cyan(command));
 
   execOrDie('amp clean --exclude release');
@@ -483,7 +486,8 @@ async function prependConfig_(outputDir) {
           ...target.config,
         });
 
-        const contents = await fs.readFile(targetPath, 'utf-8');
+        const contents = await fs.readFile(targetPath, 'utf8');
+
         return fs.writeFile(
           targetPath,
           `self.AMP_CONFIG=${channelConfig};/*AMP_CONFIG*/${contents}`
@@ -590,6 +594,7 @@ release.flags = {
   'flavor':
     'Limit this release build to a single flavor (can be used to split the release work across multiple build machines)',
   'esm': 'Compile with --esm if true, without --esm if false or unspecified',
+  'full_sourcemaps': 'Include source code content in sourcemaps',
   'dedup_v0':
     'Removes duplicate copies of the v0/ subdirectory when they are the same files as those in the Stable (01-prefixed) channel',
 };

build-system/tasks/sourcemaps.js

@@ -19,6 +19,13 @@ function massageSourcemaps(mapChain, options) {
   if (map.file) {
     map.file = path.basename(map.file);
   }
+  map.sources = map.sources.map((s) => {
+    if (s?.startsWith('../')) {
+      return s.slice('../'.length);
+    }
+    return s;
+  });
+
   return map;
 }