Bento: Generate npm binaries during build/dist (#32742)

* Move toChildArray to preact compat

* Compile npm binaries with esbuild

* Cleanup

* Fix merge conflicts

* Fix fit-text's package.json exports

* Use the new minified + compileJsWithEsbuild to compile npm modules

* Build npm into extension's dist dir

* Create a preact/dom library

* Ignore the extension's dist dir

* Implement remapping of npm dependencies

* Fix dist compilation

* Implement react remapping

* Peer depend on React

* Remove last traces of extensions/**/build

* Apply suggestions from code review

Co-authored-by: Jake Fried <samouri@users.noreply.github.com>

Co-authored-by: Jake Fried <samouri@users.noreply.github.com>

.eslintignore

@@ -1,5 +1,6 @@
 # Output directories
 build/**
+extensions/**/dist/**
 dist/**
 dist.3p/**
 dist.tools/**

.gitignore

@@ -4,6 +4,7 @@
 .babel-cache/
 build/
 .amp-dep-check
+extensions/**/dist/**
 c
 /dist
 dist.3p

.prettierignore

@@ -6,3 +6,4 @@
 **/dist/**
 **/dist.3p/**
 **/dist.tools/**
+extensions/**/dist/**

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

@@ -25,11 +25,21 @@ const {getReplacePlugin} = require('./helpers');
  */
 function getMinifiedConfig() {
   const replacePlugin = getReplacePlugin();
+  const reactJsxPlugin = [
+    '@babel/plugin-transform-react-jsx',
+    {
+      pragma: 'Preact.createElement',
+      pragmaFrag: 'Preact.Fragment',
+      useSpread: true,
+    },
+  ];
+
   const plugins = [
     'optimize-objstr',
     './build-system/babel-plugins/babel-plugin-transform-fix-leading-comments',
     './build-system/babel-plugins/babel-plugin-transform-promise-resolve',
     '@babel/plugin-transform-react-constant-elements',
+    reactJsxPlugin,
     argv.esm
       ? './build-system/babel-plugins/babel-plugin-transform-dev-methods'
       : null,

build-system/common/update-packages.js

@@ -201,6 +201,12 @@ function patchShadowDom() {
   writeIfUpdated(patchedName, file);
 }
 
+function patchPreact() {
+  fs.ensureDirSync('node_modules/preact/dom');
+  const file = `export { render, hydrate } from 'preact';`;
+  writeIfUpdated('node_modules/preact/dom/index.js', file);
+}
+
 /**
  * Deletes the map file for rrule, which breaks closure compiler.
  * TODO(rsimha): Remove this workaround after a fix is merged for
@@ -242,6 +248,7 @@ async function updatePackages() {
   patchIntersectionObserver();
   patchResizeObserver();
   patchShadowDom();
+  patchPreact();
   removeRruleSourcemap();
 }
 

build-system/compile/bundles.config.extensions.json

@@ -157,10 +157,37 @@
   {"name": "amp-facebook-page", "version": "0.1", "latestVersion": "0.1"},
   {
     "name": "amp-fit-text",
-    "version": ["0.1", "1.0"],
+    "version": "0.1",
     "latestVersion": "0.1",
     "options": {"hasCss": true}
   },
+  {
+    "name": "amp-fit-text",
+    "version": "1.0",
+    "latestVersion": "0.1",
+    "options": {
+      "hasCss": true,
+      "binaries": [
+        {
+          "entryPoint": "component.js",
+          "outfile": "component-preact.js",
+          "external": ["preact", "preact/dom", "preact/compat", "preact/hooks"],
+          "remap": {"preact/dom": "preact"}
+        },
+        {
+          "entryPoint": "component.js",
+          "outfile": "component-react.js",
+          "external": ["react", "react-dom"],
+          "remap": {
+            "preact": "react",
+            "preact/compat": "react",
+            "preact/hooks": "react",
+            "preact/dom": "react-dom"
+          }
+        }
+      ]
+    }
+  },
   {"name": "amp-font", "version": "0.1", "latestVersion": "0.1"},
   {
     "name": "amp-form",

build-system/compile/sources.js

@@ -65,6 +65,7 @@ const COMMON_GLOBS = [
   'node_modules/@ampproject/worker-dom/dist/amp-production/main.mjs',
   'node_modules/preact/package.json',
   'node_modules/preact/dist/*.js',
+  'node_modules/preact/dom/*.js',
   'node_modules/preact/hooks/package.json',
   'node_modules/preact/hooks/dist/*.js',
   'node_modules/preact/compat/package.json',
@@ -157,6 +158,7 @@ const CLOSURE_SRC_GLOBS = [
   // Not sure what these files are, but they seem to duplicate code
   // one level below and confuse the compiler.
   '!node_modules/core-js/modules/library/**.js',
+  '!extensions/**/dist/**/*.js',
 ].concat(COMMON_GLOBS);
 
 module.exports = {

build-system/tasks/clean.js

@@ -31,6 +31,7 @@ async function clean() {
     '.amp-dep-check',
     '.babel-cache',
     'build',
+    'extensions/**/dist',
     'build-system/server/new-server/transforms/dist',
     'deps.txt',
     'dist',

build-system/tasks/extension-helpers.js

@@ -25,14 +25,20 @@ const {
   verifyExtensionBundles,
   jsBundles,
 } = require('../compile/bundles.config');
+const {
+  maybeToEsmName,
+  compileJs,
+  compileJsWithEsbuild,
+  mkdirSync,
+} = require('./helpers');
 const {analyticsVendorConfigs} = require('./analytics-vendor-configs');
 const {compileJison} = require('./compile-jison');
 const {endBuildStep, watchDebounceDelay, doBuildJs} = require('./helpers');
 const {green, red, cyan} = require('kleur/colors');
 const {isCiBuild} = require('../common/ci');
 const {jsifyCssAsync} = require('./css/jsify-css');
 const {log} = require('../common/logging');
-const {maybeToEsmName, compileJs, mkdirSync} = require('./helpers');
+const {parse: pathParse} = require('path');
 const {removeFromClosureBabelCache} = require('../compile/pre-closure-babel');
 const {watch} = require('chokidar');
 
@@ -80,10 +86,21 @@ const DEFAULT_EXTENSION_SET = ['amp-loader', 'amp-auto-lightbox'];
  *   loadPriority?: string,
  *   cssBinaries?: Array<string>,
  *   extraGlobs?: Array<string>,
+ *   binaries?: Array<ExtensionBinary>,
  * }}
  */
 const ExtensionOption = {}; // eslint-disable-line no-unused-vars
 
+/**
+ * @typedef {{
+ *   entryPoint: string,
+ *   outfile: string,
+ *   external?: Array<string>
+ *   remap?: Map<string, string>
+ * }}
+ */
+const ExtensionBinary = {}; // eslint-disable-line no-unused-vars
+
 // All declared extensions.
 const extensions = {};
 
@@ -109,7 +126,7 @@ function declareExtension(
   extensionsObject,
   includeLatest
 ) {
-  const defaultOptions = {hasCss: false};
+  const defaultOptions = {hasCss: false, npm: undefined};
   const versions = Array.isArray(version) ? version : [version];
   versions.forEach((v) => {
     extensionsObject[`${name}-${v}`] = {
@@ -463,6 +480,9 @@ async function buildExtension(
   if (name === 'amp-bind') {
     await doBuildJs(jsBundles, 'ww.max.js', options);
   }
+  if (options.binaries) {
+    await buildBinaries(extDir, options);
+  }
   if (name === 'amp-analytics') {
     await analyticsVendorConfigs(options);
   }
@@ -520,6 +540,37 @@ function buildExtensionCss(extDir, name, version, options) {
   return Promise.all(promises);
 }
 
+/**
+ * @param {string} extDir
+ * @param {!Object} options
+ * @return {!Promise}
+ */
+function buildBinaries(extDir, options) {
+  const {binaries} = options;
+  mkdirSync(`${extDir}/dist`);
+
+  const promises = binaries.map((binary) => {
+    const {entryPoint, outfile, external, remap} = binary;
+    const {name} = pathParse(outfile);
+    const esm = argv.esm || argv.sxg || false;
+    return compileJsWithEsbuild(
+      extDir + '/',
+      entryPoint,
+      `${extDir}/dist`,
+      Object.assign(options, {
+        toName: maybeToEsmName(`${name}.max.js`),
+        minifiedName: maybeToEsmName(`${name}.js`),
+        latestName: '',
+        outputFormat: esm ? 'esm' : 'cjs',
+        wrapper: '',
+        externalDependencies: external,
+        remapDependencies: remap,
+      })
+    );
+  });
+  return Promise.all(promises);
+}
+
 /**
  * Build the JavaScript for the extension specified
  *

build-system/tasks/helpers.js

@@ -490,15 +490,18 @@ async function compileUnminifiedJs(srcDir, srcFilename, destDir, options) {
   }
 
   const {banner, footer} = splitWrapper();
-  const plugin = getEsbuildBabelPlugin('unminified', /* enableCache */ true);
+  const babelPlugin = getEsbuildBabelPlugin(
+    'unminified',
+    /* enableCache */ true
+  );
   const buildResult = await esbuild
     .build({
       entryPoints: [entryPoint],
       bundle: true,
       sourcemap: true,
       define: experimentDefines,
       outfile: destFile,
-      plugins: [plugin],
+      plugins: [babelPlugin],
       banner,
       footer,
       incremental: !!options.watch,
@@ -551,10 +554,15 @@ async function compileJsWithEsbuild(srcDir, srcFilename, destDir, options) {
     return watchedTargets.get(entryPoint).rebuild();
   }
 
-  const plugin = getEsbuildBabelPlugin(
+  const babelPlugin = getEsbuildBabelPlugin(
     options.minify ? 'minified' : 'unminified',
     /* enableCache */ true
   );
+  const plugins = [babelPlugin];
+
+  if (options.remapDependencies) {
+    plugins.unshift(remapDependenciesPlugin());
+  }
 
   let result = null;
 
@@ -568,11 +576,13 @@ async function compileJsWithEsbuild(srcDir, srcFilename, destDir, options) {
         bundle: true,
         sourcemap: true,
         outfile: destFile,
-        plugins: [plugin],
+        plugins,
         minify: options.minify,
+        format: options.outputFormat || undefined,
         target: argv.esm ? 'es6' : 'es5',
         incremental: !!options.watch,
         logLevel: 'silent',
+        external: options.externalDependencies,
       });
     } else {
       result = await result.rebuild();
@@ -581,6 +591,27 @@ async function compileJsWithEsbuild(srcDir, srcFilename, destDir, options) {
     await finishBundle(srcFilename, destDir, destFilename, options, time);
   }
 
+  function remapDependenciesPlugin() {
+    const remapDependencies = {__proto__: null, ...options.remapDependencies};
+    const external = options.externalDependencies;
+    return {
+      name: 'remap-dependencies',
+      setup(build) {
+        build.onResolve({filter: /.*/}, (args) => {
+          const dep = args.path;
+          const remap = remapDependencies[dep];
+          if (remap) {
+            const isExternal = external.includes(remap);
+            return {
+              path: isExternal ? remap : require.resolve(remap),
+              external: isExternal,
+            };
+          }
+        });
+      },
+    };
+  }
+
   await build(startTime).catch((err) =>
     handleBundleError(err, !!options.watch, destFilename)
   );
@@ -614,7 +645,7 @@ async function minifyWithTerser(destDir, destFilename, options) {
     return;
   }
 
-  const filename = destDir + destFilename;
+  const filename = path.join(destDir, destFilename);
   const terserOptions = {
     mangle: true,
     compress: true,
@@ -653,7 +684,7 @@ async function compileJs(srcDir, srcFilename, destDir, options) {
   if (options.minify) {
     return compileMinifiedJs(srcDir, srcFilename, destDir, options);
   } else {
-    return await compileUnminifiedJs(srcDir, srcFilename, destDir, options);
+    return compileUnminifiedJs(srcDir, srcFilename, destDir, options);
   }
 }
 
@@ -826,6 +857,7 @@ module.exports = {
   compileJsWithEsbuild,
   doBuildJs,
   endBuildStep,
+  compileUnminifiedJs,
   maybePrintCoverageMessage,
   maybeToEsmName,
   mkdirSync,

build-system/test-configs/config.js

@@ -141,6 +141,7 @@ const presubmitGlobs = [
   '!build-system/runner/build/**/*.*',
   '!third_party/**/*.*',
   '!**/node_modules/**/*.*',
+  '!extensions/**/dist/*',
   '!examples/**/*',
   '!examples/visual-tests/**/*',
   '!test/coverage/**/*.*',

extensions/amp-base-carousel/1.0/base-carousel.js

@@ -28,11 +28,11 @@ import {ContainWrapper} from '../../../src/preact/component';
 import {Scroller} from './scroller';
 import {WithAmpContext} from '../../../src/preact/context';
 import {WithLightbox} from '../../amp-lightbox-gallery/1.0/component';
-import {forwardRef} from '../../../src/preact/compat';
+import {forwardRef, toChildArray} from '../../../src/preact/compat';
 import {isRTL} from '../../../src/dom';
 import {mod} from '../../../src/utils/math';
+import {toWin} from '../../../src/types';
 import {
-  toChildArray,
   useCallback,
   useContext,
   useEffect,
@@ -42,7 +42,6 @@ import {
   useRef,
   useState,
 } from '../../../src/preact';
-import {toWin} from '../../../src/types';
 import {useStyles} from './base-carousel.jss';
 
 /**
@@ -363,7 +362,7 @@ function BaseCarouselWithRef(
 
           Note: We naively display all slides for mixedLength as multiple
           can be visible within the carousel viewport - eventually these can also
-          be optimized to only display the minimum necessary for the current 
+          be optimized to only display the minimum necessary for the current
           and next viewport.
         */}
         {childrenArray.map((child, index) =>