🚀 ♻️ build-system: merge unminified and vendor codepath (#35419)

* build-system: merge unminified and vendor codepath * missed a spot * do not modify options object
ampproject · Aug 13, 2021 · 1af084a · 1af084a
1 parent a824298
commit 1af084a
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 101 deletions.
diff --git a/build-system/tasks/3p-vendor-helpers.js b/build-system/tasks/3p-vendor-helpers.js
@@ -16,9 +16,9 @@
 
 const debounce = require('../common/debounce');
 const globby = require('globby');
-const {compileJsWithEsbuild} = require('./helpers');
 const {cyan, red} = require('../common/colors');
 const {endBuildStep} = require('./helpers');
+const {esbuildCompile} = require('./helpers');
 const {VERSION} = require('../compile/internal-version');
 const {watchDebounceDelay} = require('./helpers');
 const {watch} = require('chokidar');
@@ -50,7 +50,7 @@ async function buildVendorConfigs(options) {
 
   await Promise.all(
     Object.values(bundles).map((bundle) =>
-      compileJsWithEsbuild(
+      esbuildCompile(
         bundle.srcDir,
         bundle.srcFilename,
         options.minify ? bundle.minifiedDestDir : bundle.destDir,
@@ -78,7 +78,7 @@ async function buildVendorConfigs(options) {
 async function doBuild3pVendor(jsBundles, name, options) {
   const target = jsBundles[name];
   if (target) {
-    return compileJsWithEsbuild(
+    return esbuildCompile(
       target.srcDir,
       target.srcFilename,
       options.minify ? target.minifiedDestDir : target.destDir,

diff --git a/build-system/tasks/extension-helpers.js b/build-system/tasks/extension-helpers.js
@@ -23,9 +23,9 @@ const path = require('path');
 const wrappers = require('../compile/compile-wrappers');
 const {
   compileJs,
-  compileJsWithEsbuild,
   doBuildJs,
   endBuildStep,
+  esbuildCompile,
   maybeToEsmName,
   maybeToNpmEsmName,
   mkdirSync,
@@ -674,19 +674,15 @@ function buildBinaries(extDir, binaries, options) {
     const {entryPoint, external, outfile, remap} = binary;
     const {name} = pathParse(outfile);
     const esm = argv.esm || argv.sxg || false;
-    return compileJsWithEsbuild(
-      extDir + '/',
-      entryPoint,
-      `${extDir}/dist`,
-      Object.assign(options, {
-        toName: maybeToNpmEsmName(`${name}.max.js`),
-        minifiedName: maybeToNpmEsmName(`${name}.js`),
-        latestName: '',
-        outputFormat: esm ? 'esm' : 'cjs',
-        externalDependencies: external,
-        remapDependencies: remap,
-      })
-    );
+    return esbuildCompile(extDir + '/', entryPoint, `${extDir}/dist`, {
+      ...options,
+      toName: maybeToNpmEsmName(`${name}.max.js`),
+      minifiedName: maybeToNpmEsmName(`${name}.js`),
+      latestName: '',
+      outputFormat: esm ? 'esm' : 'cjs',
+      externalDependencies: external,
+      remapDependencies: remap,
+    });
   });
   return Promise.all(promises);
 }
@@ -720,17 +716,13 @@ async function buildExtensionJs(extDir, name, version, latestVersion, options) {
       ? wrapperOrFn(name, version, latest, argv.esm, options.loadPriority)
       : wrapperOrFn;
 
-  await compileJs(
-    extDir + '/',
-    filename,
-    './dist/v0',
-    Object.assign(options, {
-      toName: `${name}-${version}.max.js`,
-      minifiedName: `${name}-${version}.js`,
-      latestName: latest ? `${name}-latest.js` : '',
-      wrapper,
-    })
-  );
+  await compileJs(extDir + '/', filename, './dist/v0', {
+    ...options,
+    toName: `${name}-${version}.max.js`,
+    minifiedName: `${name}-${version}.js`,
+    latestName: latest ? `${name}-latest.js` : '',
+    wrapper,
+  });
 
   // If an incremental watch build fails, simply return.
   if (options.errored) {

diff --git a/build-system/tasks/helpers.js b/build-system/tasks/helpers.js
@@ -418,16 +418,20 @@ async function finishBundle(
 /**
  * Transforms a given JavaScript file entry point with esbuild and babel, and
  * watches it for changes (if required).
+ *
  * @param {string} srcDir
  * @param {string} srcFilename
  * @param {string} destDir
  * @param {?Object} options
  * @return {!Promise}
  */
-async function compileUnminifiedJs(srcDir, srcFilename, destDir, options) {
+async function esbuildCompile(srcDir, srcFilename, destDir, options) {
   const startTime = Date.now();
   const entryPoint = path.join(srcDir, srcFilename);
-  const destFilename = options.toName || srcFilename;
+  const fileName = options.minify
+    ? options.minifiedName
+    : options.toName ?? srcFilename;
+  const destFilename = options.npm ? fileName : maybeToEsmName(fileName);
   const destFile = path.join(destDir, destFilename);
 
   if (watchedTargets.has(entryPoint)) {
@@ -447,74 +451,7 @@ async function compileUnminifiedJs(srcDir, srcFilename, destDir, options) {
       footer: {js: wrapper.slice(start + sentinel.length)},
     };
   }
-
   const {banner, footer} = splitWrapper();
-  const babelPlugin = getEsbuildBabelPlugin(
-    'unminified',
-    /* enableCache */ true
-  );
-
-  const buildResult = await esbuild
-    .build({
-      entryPoints: [entryPoint],
-      bundle: true,
-      sourcemap: true,
-      define: experimentDefines,
-      outfile: destFile,
-      plugins: [babelPlugin],
-      banner,
-      footer,
-      incremental: !!options.watch,
-      logLevel: 'silent',
-    })
-    .then((result) => {
-      finishBundle(srcFilename, destDir, destFilename, options, startTime);
-      return result;
-    })
-    .catch((err) => handleBundleError(err, !!options.watch, destFilename));
-
-  if (options.watch) {
-    watchedTargets.set(entryPoint, {
-      rebuild: async () => {
-        const time = Date.now();
-        const {rebuild} = /** @type {Required<esbuild.BuildResult>} */ (
-          buildResult
-        );
-
-        const buildPromise = rebuild()
-          .then(() =>
-            finishBundle(srcFilename, destDir, destFilename, options, time)
-          )
-          .catch((err) =>
-            handleBundleError(err, /* continueOnError */ true, destFilename)
-          );
-        options?.onWatchBuild(buildPromise);
-        await buildPromise;
-      },
-    });
-  }
-}
-
-/**
- * Transforms a given JavaScript file entry point with esbuild and babel, and
- * watches it for changes (if required).
- * Used by 3p iframe vendors.
- * @param {string} srcDir
- * @param {string} srcFilename
- * @param {string} destDir
- * @param {?Object} options
- * @return {!Promise}
- */
-async function compileJsWithEsbuild(srcDir, srcFilename, destDir, options) {
-  const startTime = Date.now();
-  const entryPoint = path.join(srcDir, srcFilename);
-  const fileName = options.minify ? options.minifiedName : options.toName;
-  const destFilename = options.npm ? fileName : maybeToEsmName(fileName);
-  const destFile = path.join(destDir, destFilename);
-
-  if (watchedTargets.has(entryPoint)) {
-    return watchedTargets.get(entryPoint).rebuild();
-  }
 
   const babelPlugin = getEsbuildBabelPlugin(
     options.minify ? 'minified' : 'unminified',
@@ -539,8 +476,11 @@ async function compileJsWithEsbuild(srcDir, srcFilename, destDir, options) {
         bundle: true,
         sourcemap: true,
         outfile: destFile,
+        define: experimentDefines,
         plugins,
-        format: options.outputFormat || undefined,
+        format: options.outputFormat,
+        banner,
+        footer,
         // For es5 builds, ensure esbuild-injected code is transpiled.
         target: argv.esm ? 'es6' : 'es5',
         incremental: !!options.watch,
@@ -686,7 +626,7 @@ async function compileJs(srcDir, srcFilename, destDir, options) {
   async function doCompileJs(options) {
     const buildResult = options.minify
       ? compileMinifiedJs(srcDir, srcFilename, destDir, options)
-      : compileUnminifiedJs(srcDir, srcFilename, destDir, options);
+      : esbuildCompile(srcDir, srcFilename, destDir, options);
     if (options.onWatchBuild) {
       options.onWatchBuild(buildResult);
     }
@@ -884,10 +824,9 @@ module.exports = {
   compileAllJs,
   compileCoreRuntime,
   compileJs,
-  compileJsWithEsbuild,
+  esbuildCompile,
   doBuildJs,
   endBuildStep,
-  compileUnminifiedJs,
   maybePrintCoverageMessage,
   maybeToEsmName,
   maybeToNpmEsmName,