Merge pull request #1545 from hypothesis/better-js-minification

Improve minification of JS bundles

gulpfile.js

@@ -282,8 +282,9 @@ let isFirstBuild = true;
  * the Hypothesis client.
  *
  * @param {Object} manifest - Manifest mapping asset paths to cache-busted URLs
+ * @param {Object} options - Options for generating the boot script
  */
-function generateBootScript(manifest) {
+function generateBootScript(manifest, { usingDevServer = false } = {}) {
   const { version } = require('./package.json');
 
   const defaultSidebarAppUrl = process.env.SIDEBAR_APP_URL
@@ -292,11 +293,12 @@ function generateBootScript(manifest) {
 
   let defaultAssetRoot;
 
-  if (process.env.NODE_ENV === 'production') {
-    defaultAssetRoot = `https://cdn.hypothes.is/hypothesis/${version}/`;
+  if (process.env.NODE_ENV === 'production' && !usingDevServer) {
+    defaultAssetRoot = 'https://cdn.hypothes.is/hypothesis';
   } else {
-    defaultAssetRoot = `{current_scheme}://{current_host}:3001/hypothesis/${version}/`;
+    defaultAssetRoot = '{current_scheme}://{current_host}:3001/hypothesis';
   }
+  defaultAssetRoot = `${defaultAssetRoot}/${version}/`;
 
   if (isFirstBuild) {
     gulpUtil.log(`Sidebar app URL: ${defaultSidebarAppUrl}`);
@@ -320,7 +322,7 @@ function generateBootScript(manifest) {
  * Generate a JSON manifest mapping file paths to
  * URLs containing cache-busting query string parameters.
  */
-function generateManifest() {
+function generateManifest(opts) {
   return gulp
     .src(MANIFEST_SOURCE_FILES)
     .pipe(manifest({ name: 'manifest.json' }))
@@ -333,7 +335,7 @@ function generateManifest() {
         triggerLiveReload(changed);
 
         // Expand template vars in boot script bundle
-        generateBootScript(newManifest);
+        generateBootScript(newManifest, opts);
 
         this.push(file);
         callback();
@@ -343,7 +345,9 @@ function generateManifest() {
 }
 
 gulp.task('watch-manifest', function() {
-  gulp.watch(MANIFEST_SOURCE_FILES, { delay: 500 }, generateManifest);
+  gulp.watch(MANIFEST_SOURCE_FILES, { delay: 500 }, function updateManifest() {
+    generateManifest({ usingDevServer: true });
+  });
 });
 
 gulp.task('serve-package', function() {

package.json

@@ -105,9 +105,9 @@
     "showdown": "^1.6.4",
     "sinon": "^7.2.3",
     "stringify": "^5.1.0",
+    "terser": "^4.4.0",
     "through2": "^3.0.0",
     "tiny-emitter": "^2.0.2",
-    "uglifyify": "^5.0.1",
     "unorm": "^1.3.3",
     "vinyl": "^2.2.0",
     "watchify": "^3.7.0",

scripts/gulp/create-bundle.js

@@ -13,10 +13,10 @@ const exorcist = require('exorcist');
 const envify = require('loose-envify/custom');
 const gulpUtil = require('gulp-util');
 const mkdirp = require('mkdirp');
-const through = require('through2');
-const uglifyify = require('uglifyify');
 const watchify = require('watchify');
 
+const minifyStream = require('./minify-stream');
+
 const log = gulpUtil.log;
 
 function streamFinished(stream) {
@@ -30,54 +30,6 @@ function waitForever() {
   return new Promise(function() {});
 }
 
-/**
- * Create a transform stream which wraps code from the input with a function
- * which is immediately executed (aka. an "IIFE").
- *
- * @param {string} headerCode - Code added at the start of the wrapper function.
- * @param {string} trailerCode - Code added at the end of the wrapper function.
- * @return {Transform} - A Node `Transform` stream.
- */
-function wrapCodeWithFunction(headerCode, trailerCode = '') {
-  const iifeStart = '(function() {' + headerCode + ';';
-  const iifeEnd = ';' + trailerCode + '})()';
-
-  let isFirstChunk = true;
-  return through(
-    function(data, enc, callback) {
-      if (isFirstChunk) {
-        isFirstChunk = false;
-        this.push(Buffer.from(iifeStart));
-      }
-      this.push(data);
-      callback();
-    },
-    function(callback) {
-      this.push(Buffer.from(iifeEnd));
-      callback();
-    }
-  );
-}
-
-/**
- * Wrap a Browserify bundle's code to change the name of the `require` function
- * which the bundle uses to load modules defined in other bundles.
- *
- * Use this together with Browserify's `externalRequireName` option to define/use
- * a different name for the `require` function. This is useful to avoid conflicts
- * with other code on the page which define/use "require".
- *
- * @param {string} name - Replacement name for the `require` function.
- * @return {Transform} - A node `Transform` stream.
- */
-function useExternalRequireName(name) {
-  // Make the `require` lookup inside the bundle find `name` in the global
-  // scope exported by a previous bundle, instead of `require`.
-  return wrapCodeWithFunction(
-    `var require=("function"==typeof ${name}&&${name})`
-  );
-}
-
 /**
  * type Transform = 'coffee';
  *
@@ -175,6 +127,24 @@ module.exports = function createBundle(config, buildOpts) {
     }
   });
 
+  // Override the "prelude" script which Browserify adds at the start of
+  // generated bundles to look for the custom require name set by previous bundles
+  // on the page instead of the default "require".
+  const defaultPreludePath = require.resolve('browser-pack/prelude');
+  const prelude = fs
+    .readFileSync(defaultPreludePath)
+    .toString()
+    .replace(
+      'var previousRequire = typeof require == "function" && require;',
+      `var previousRequire = typeof ${externalRequireName} == "function" && ${externalRequireName};`
+    );
+  if (!prelude.includes(externalRequireName)) {
+    throw new Error(
+      'Failed to modify prelude to use custom name for "require" function'
+    );
+  }
+  bundleOpts.prelude = prelude;
+
   const name = config.name;
 
   const bundleFileName = name + '.bundle.js';
@@ -223,10 +193,6 @@ module.exports = function createBundle(config, buildOpts) {
     }
   });
 
-  if (config.minify) {
-    bundle.transform({ global: true }, uglifyify);
-  }
-
   // Include or disable debugging checks in our code and dependencies by
   // replacing references to `process.env.NODE_ENV`.
   bundle.transform(
@@ -240,16 +206,22 @@ module.exports = function createBundle(config, buildOpts) {
     }
   );
 
+  if (config.minify) {
+    bundle.transform({ global: true }, minifyStream);
+  }
+
   function build() {
     const output = fs.createWriteStream(bundlePath);
-    const b = bundle.bundle();
-    b.on('error', function(err) {
+    let stream = bundle.bundle();
+    stream.on('error', function(err) {
       log('Build error', err.toString());
     });
-    const stream = b
-      .pipe(useExternalRequireName(externalRequireName))
-      .pipe(exorcist(sourcemapPath))
-      .pipe(output);
+
+    if (config.minify) {
+      stream = stream.pipe(minifyStream());
+    }
+
+    stream = stream.pipe(exorcist(sourcemapPath)).pipe(output);
     return streamFinished(stream);
   }
 

scripts/gulp/minify-stream.js

+'use strict';
+
+const { Transform } = require('stream');
+
+const terser = require('terser');
+
+/**
+ * Return a Node `Transform` stream that minifies JavaScript input.
+ *
+ * This is designed to be used both to process individual modules as a Browserify
+ * transform, and also to be applied to the output of the whole bundle to
+ * compress the module infrastructure that Browserify adds.
+ *
+ * @example
+ *   browserify(['src/app.js'])
+ *     .transform({ global: true }, minifyStream) // Minify individual modules
+ *     .pipe(minifyStream()) // Minify the code added by Browserify
+ *     .pipe(output);
+ */
+function minifyStream() {
+  return new Transform({
+    transform(data, encoding, callback) {
+      if (!this.chunks) {
+        this.chunks = [];
+      }
+      this.chunks.push(data);
+      callback();
+    },
+
+    flush(callback) {
+      const code = Buffer.concat(this.chunks).toString();
+      const minified = terser.minify(code, {
+        // See https://github.com/terser/terser#minify-options-structure
+        sourceMap: {
+          content: 'inline',
+          url: 'inline',
+        },
+      }).code;
+
+      this.push(minified);
+      callback();
+    },
+  });
+}
+
+module.exports = minifyStream;

yarn.lock

@@ -2205,6 +2205,11 @@ commander@^2.19.0, commander@~2.20.0:
   resolved "https://registry.yarnpkg.com/commander/-/commander-2.20.0.tgz#d58bb2b5c1ee8f87b0d340027e9e94e222c5a422"
   integrity sha512-7j2y+40w61zy6YC2iRNpUe/NwhNyoXrYpHMrSunaMG64nRnaf96zO/KMQR4OyN/UnE5KLyEBnKHd4aG3rskjpQ==
 
+commander@^2.20.0:
+  version "2.20.3"
+  resolved "https://registry.yarnpkg.com/commander/-/commander-2.20.3.tgz#fd485e84c03eb4881c20722ba48035e8531aeb33"
+  integrity sha512-GpVkmM8vF2vQUkj2LvZmD35JxeJOLCwJ9cUkugyk2nuhbv3+mJvpLYYt+0+USMxE+oj+ey/lJEnhZw75x/OMcQ==
+
 commander@^4.0.1:
   version "4.0.1"
   resolved "https://registry.yarnpkg.com/commander/-/commander-4.0.1.tgz#b67622721785993182e807f4883633e6401ba53c"
@@ -7632,10 +7637,10 @@ source-map-resolve@^0.5.0, source-map-resolve@^0.5.2:
     source-map-url "^0.4.0"
     urix "^0.1.0"
 
-source-map-support@~0.5.10:
-  version "0.5.12"
-  resolved "https://registry.yarnpkg.com/source-map-support/-/source-map-support-0.5.12.tgz#b4f3b10d51857a5af0138d3ce8003b201613d599"
-  integrity sha512-4h2Pbvyy15EE02G+JOZpUCmqWJuqrs+sEkzewTm++BPi7Hvn/HwcqLAcNxYAyI0x13CpPPn+kMjl+hplXMHITQ==
+source-map-support@~0.5.12:
+  version "0.5.16"
+  resolved "https://registry.yarnpkg.com/source-map-support/-/source-map-support-0.5.16.tgz#0ae069e7fe3ba7538c64c98515e35339eac5a042"
+  integrity sha512-efyLRJDr68D9hBBNIPWFjhpFzURh+KJykQwvMyW5UiZzYwoF6l4YMMDIJJEyFWxWCqfyxLzz6tSfUFR+kXXsVQ==
   dependencies:
     buffer-from "^1.0.0"
     source-map "^0.6.0"
@@ -8038,14 +8043,14 @@ ternary@~1.0.0:
   resolved "https://registry.yarnpkg.com/ternary/-/ternary-1.0.0.tgz#45702725608c9499d46a9610e9b0e49ff26f789e"
   integrity sha1-RXAnJWCMlJnUapYQ6bDkn/JveJ4=
 
-terser@^3.7.5:
-  version "3.17.0"
-  resolved "https://registry.yarnpkg.com/terser/-/terser-3.17.0.tgz#f88ffbeda0deb5637f9d24b0da66f4e15ab10cb2"
-  integrity sha512-/FQzzPJmCpjAH9Xvk2paiWrFq+5M6aVOf+2KRbwhByISDX/EujxsK+BAvrhb6H+2rtrLCHK9N01wO014vrIwVQ==
+terser@^4.4.0:
+  version "4.4.0"
+  resolved "https://registry.yarnpkg.com/terser/-/terser-4.4.0.tgz#22c46b4817cf4c9565434bfe6ad47336af259ac3"
+  integrity sha512-oDG16n2WKm27JO8h4y/w3iqBGAOSCtq7k8dRmrn4Wf9NouL0b2WpMHGChFGZq4nFAQy1FsNJrVQHfurXOSTmOA==
   dependencies:
-    commander "^2.19.0"
+    commander "^2.20.0"
     source-map "~0.6.1"
-    source-map-support "~0.5.10"
+    source-map-support "~0.5.12"
 
 test-exclude@^5.2.3:
   version "5.2.3"
@@ -8324,17 +8329,6 @@ uglify-js@^3.1.4:
     commander "~2.20.0"
     source-map "~0.6.1"
 
-uglifyify@^5.0.1:
-  version "5.0.2"
-  resolved "https://registry.yarnpkg.com/uglifyify/-/uglifyify-5.0.2.tgz#7d0269885e09faa963208a9ec6721afcaf45fc50"
-  integrity sha512-NcSk6pgoC+IgwZZ2tVLVHq+VNKSvLPlLkF5oUiHPVOJI0s/OlSVYEGXG9PCAH0hcyFZLyvt4KBdPAQBRlVDn1Q==
-  dependencies:
-    convert-source-map "~1.1.0"
-    minimatch "^3.0.2"
-    terser "^3.7.5"
-    through "~2.3.4"
-    xtend "^4.0.1"
-
 ultron@~1.1.0:
   version "1.1.1"
   resolved "https://registry.yarnpkg.com/ultron/-/ultron-1.1.1.tgz#9fe1536a10a664a65266a1e3ccf85fd36302bc9c"