Merge pull request #980 from hypothesis/polyfill-refactor

Load polyfills only in browsers that need them

gulpfile.js

@@ -117,9 +117,28 @@ const appBundles = [{
   transforms: ['babel', 'coffee'],
 }];
 
-const appBundleConfigs = appBundles.map(function (config) {
-  return Object.assign({}, appBundleBaseConfig, config);
-});
+// Polyfill bundles. Polyfills are grouped into "sets" (one bundle per set)
+// based on major ECMAScript version or DOM API. Some large polyfills
+// (eg. for String.prototype.normalize) are additionally separated out into
+// their own bundles.
+const polyfillBundles = [
+  'document.evaluate',
+  'es2015',
+  'es2016',
+  'es2017',
+  'string.prototype.normalize',
+  'url',
+].map(set => ({
+  name: `polyfills-${set}`,
+  entry: `./src/shared/polyfills/${set}`,
+  transforms: ['babel'],
+}));
+
+const appBundleConfigs = appBundles
+  .concat(polyfillBundles)
+  .map(config => {
+    return Object.assign({}, appBundleBaseConfig, config);
+  });
 
 gulp.task('build-js', gulp.parallel('build-vendor-js', function () {
   return Promise.all(appBundleConfigs.map(function (config) {

scripts/gulp/vendor-bundles.js

@@ -9,7 +9,6 @@
 module.exports = {
   bundles: {
     jquery: ['jquery'],
-    polyfills: [require.resolve('../../src/shared/polyfills')],
     angular: [
       'angular',
       'angular-route',
@@ -21,7 +20,6 @@ module.exports = {
     ],
     katex: ['katex'],
     showdown: ['showdown'],
-    unorm: ['unorm'],
     raven: ['raven-js'],
   },
 

src/annotator/index.js

 'use strict';
 
 const configFrom = require('./config/index');
-require('../shared/polyfills');
-
-// Polyfills
-
-// `document.evaluate` polyfill for IE 11.
-if (!window.document.evaluate) {
-  const wgxpath = require('wicked-good-xpath');
-  wgxpath.install();
-}
 
 const $ = require('jquery');
 

src/boot/boot.js

 'use strict';
 
+const { requiredPolyfillSets } = require('../shared/polyfills');
+
 function injectStylesheet(doc, href) {
   const link = doc.createElement('link');
   link.rel = 'stylesheet';
@@ -30,6 +32,12 @@ function injectAssets(doc, config, assets) {
   });
 }
 
+function polyfillBundles(needed) {
+  return requiredPolyfillSets(needed).map(
+    set => `scripts/polyfills-${set}.bundle.js`
+  );
+}
+
 /**
  * Bootstrap the Hypothesis client.
  *
@@ -61,9 +69,17 @@ function bootHypothesisClient(doc, config) {
   clientUrl.type = 'application/annotator+javascript';
   doc.head.appendChild(clientUrl);
 
+  const polyfills = polyfillBundles([
+    'document.evaluate',
+    'es2015',
+    'es2016',
+    'es2017',
+    'url',
+  ]);
+
   injectAssets(doc, config, [
     // Vendor code and polyfills
-    'scripts/polyfills.bundle.js',
+    ...polyfills,
     'scripts/jquery.bundle.js',
 
     // Main entry point for the client
@@ -79,14 +95,22 @@ function bootHypothesisClient(doc, config) {
  * Bootstrap the sidebar application which displays annotations.
  */
 function bootSidebarApp(doc, config) {
+  const polyfills = polyfillBundles([
+    'es2015',
+    'es2016',
+    'es2017',
+    'string.prototype.normalize',
+    'url',
+  ]);
+
   injectAssets(doc, config, [
-    // Vendor code and polyfills required by app.bundle.js
+    ...polyfills,
+
+    // Vendor code required by sidebar.bundle.js
     'scripts/raven.bundle.js',
     'scripts/angular.bundle.js',
     'scripts/katex.bundle.js',
     'scripts/showdown.bundle.js',
-    'scripts/polyfills.bundle.js',
-    'scripts/unorm.bundle.js',
 
     // The sidebar app
     'scripts/sidebar.bundle.js',

src/boot/test/boot-test.js

 'use strict';
 
-const boot = require('../boot');
+const proxyquire = require('proxyquire');
+
+function assetUrl(url) {
+  return `https://marginal.ly/client/build/${url}`;
+}
 
 describe('bootstrap', function() {
+  let boot;
+  let fakePolyfills;
   let iframe;
 
   beforeEach(function() {
     iframe = document.createElement('iframe');
     document.body.appendChild(iframe);
+
+    fakePolyfills = {
+      requiredPolyfillSets: sinon.stub().returns([]),
+    };
+
+    boot = proxyquire('../boot', {
+      '../shared/polyfills': fakePolyfills,
+    });
   });
 
   afterEach(function() {
@@ -16,8 +30,10 @@ describe('bootstrap', function() {
 
   function runBoot() {
     const assetNames = [
+      // Polyfills
+      'scripts/polyfills-es2015.bundle.js',
+
       // Annotation layer
-      'scripts/polyfills.bundle.js',
       'scripts/jquery.bundle.js',
       'scripts/annotator.bundle.js',
       'styles/annotator.css',
@@ -29,8 +45,6 @@ describe('bootstrap', function() {
       'scripts/angular.bundle.js',
       'scripts/katex.bundle.js',
       'scripts/showdown.bundle.js',
-      'scripts/polyfills.bundle.js',
-      'scripts/unorm.bundle.js',
       'scripts/sidebar.bundle.js',
 
       'styles/angular-csp.css',
@@ -75,13 +89,10 @@ describe('bootstrap', function() {
       const expectedAssets = [
         'scripts/annotator.bundle.1234.js',
         'scripts/jquery.bundle.1234.js',
-        'scripts/polyfills.bundle.1234.js',
         'styles/annotator.1234.css',
         'styles/icomoon.1234.css',
         'styles/pdfjs-overrides.1234.css',
-      ].map(function(url) {
-        return 'https://marginal.ly/client/build/' + url;
-      });
+      ].map(assetUrl);
 
       assert.deepEqual(findAssets(iframe.contentDocument), expectedAssets);
     });
@@ -105,6 +116,22 @@ describe('bootstrap', function() {
 
       assert.deepEqual(findAssets(iframe.contentDocument), []);
     });
+
+    it('loads polyfills if required', () => {
+      fakePolyfills.requiredPolyfillSets.callsFake(sets =>
+        sets.filter(s => s.match(/es2015/))
+      );
+
+      runBoot();
+
+      const polyfillsLoaded = findAssets(iframe.contentDocument).filter(a =>
+        a.match(/polyfills/)
+      );
+      assert.called(fakePolyfills.requiredPolyfillSets);
+      assert.deepEqual(polyfillsLoaded, [
+        assetUrl('scripts/polyfills-es2015.bundle.1234.js'),
+      ]);
+    });
   });
 
   context('in the sidebar application', function() {
@@ -124,21 +151,33 @@ describe('bootstrap', function() {
       const expectedAssets = [
         'scripts/angular.bundle.1234.js',
         'scripts/katex.bundle.1234.js',
-        'scripts/polyfills.bundle.1234.js',
         'scripts/raven.bundle.1234.js',
         'scripts/showdown.bundle.1234.js',
         'scripts/sidebar.bundle.1234.js',
-        'scripts/unorm.bundle.1234.js',
         'styles/angular-csp.1234.css',
         'styles/angular-toastr.1234.css',
         'styles/icomoon.1234.css',
         'styles/katex.min.1234.css',
         'styles/sidebar.1234.css',
-      ].map(function(url) {
-        return 'https://marginal.ly/client/build/' + url;
-      });
+      ].map(assetUrl);
 
       assert.deepEqual(findAssets(iframe.contentDocument), expectedAssets);
     });
+
+    it('loads polyfills if required', () => {
+      fakePolyfills.requiredPolyfillSets.callsFake(sets =>
+        sets.filter(s => s.match(/es2015/))
+      );
+
+      runBoot();
+
+      const polyfillsLoaded = findAssets(iframe.contentDocument).filter(a =>
+        a.match(/polyfills/)
+      );
+      assert.called(fakePolyfills.requiredPolyfillSets);
+      assert.deepEqual(polyfillsLoaded, [
+        assetUrl('scripts/polyfills-es2015.bundle.1234.js'),
+      ]);
+    });
   });
 });

src/karma.config.js

@@ -16,7 +16,11 @@ module.exports = function(config) {
     // list of files / patterns to load in the browser
     files: [
       // Polyfills for PhantomJS
-      './shared/polyfills.js',
+      './shared/polyfills/es2015.js',
+      './shared/polyfills/es2016.js',
+      './shared/polyfills/es2017.js',
+      './shared/polyfills/string.prototype.normalize.js',
+      './shared/polyfills/url.js',
 
       // Test setup
       './sidebar/test/bootstrap.js',
@@ -57,7 +61,7 @@ module.exports = function(config) {
     // preprocess matching files before serving them to the browser
     // available preprocessors: https://npmjs.org/browse/keyword/karma-preprocessor
     preprocessors: {
-      './shared/polyfills.js': ['browserify'],
+      './shared/polyfills/*.js': ['browserify'],
       './sidebar/test/bootstrap.js': ['browserify'],
       '**/*-test.js': ['browserify'],
       '**/*-test.coffee': ['browserify'],

src/shared/polyfills/document.evaluate.js

+'use strict';
+
+const wgxpath = require('wicked-good-xpath');
+wgxpath.install();

src/shared/polyfills.js → src/shared/polyfills/es2015.js

@@ -9,25 +9,6 @@ require('core-js/fn/array/fill');
 require('core-js/fn/array/find');
 require('core-js/fn/array/find-index');
 require('core-js/fn/array/from');
-require('core-js/fn/array/includes');
 require('core-js/fn/object/assign');
 require('core-js/fn/string/ends-with');
 require('core-js/fn/string/starts-with');
-
-// ES2017
-require('core-js/fn/object/entries');
-require('core-js/fn/object/values');
-
-// URL constructor, required by IE 10/11,
-// early versions of Microsoft Edge.
-try {
-  const url = new window.URL('https://hypothes.is');
-
-  // Some browsers (eg. PhantomJS 2.x) include a `URL` constructor which works
-  // but is broken.
-  if (url.hostname !== 'hypothes.is') {
-    throw new Error('Broken URL constructor');
-  }
-} catch (err) {
-  require('js-polyfills/url');
-}

src/shared/polyfills/es2016.js

+'use strict';
+
+require('core-js/fn/array/includes');

src/shared/polyfills/es2017.js

+'use strict';
+
+require('core-js/fn/object/entries');
+require('core-js/fn/object/values');

src/shared/polyfills/index.js

+'use strict';
+
+/**
+ * Checkers to test which polyfills are required by the current browser.
+ *
+ * This module executes in an environment without any polyfills loaded so it
+ * needs to run in old browsers, down to IE 11.
+ */
+
+/**
+ * Return true if `obj` has all of the methods in `methods`.
+ */
+function hasMethods(obj, ...methods) {
+  return methods.every(method => typeof obj[method] === 'function');
+}
+
+/**
+ * Map of polyfill set name to function to test whether the current browser
+ * needs that polyfill set.
+ *
+ * Each checker function returns `true` if the polyfill is required or `false`
+ * if the browser has the functionality natively available.
+ */
+const needsPolyfill = {
+  es2015: () => {
+    // Check for new objects in ES2015.
+    if (
+      typeof Promise !== 'function' ||
+      typeof Map !== 'function' ||
+      typeof Set !== 'function' ||
+      typeof Symbol !== 'function'
+    ) {
+      return true;
+    }
+
+    // Check for new methods on existing objects in ES2015.
+    const objMethods = [
+      [Array, 'from'],
+      [Array.prototype, 'fill', 'find', 'findIndex'],
+      [Object, 'assign'],
+      [String.prototype, 'startsWith', 'endsWith'],
+    ];
+    for (let [obj, ...methods] of objMethods) {
+      if (!hasMethods(obj, ...methods)) {
+        return true;
+      }
+    }
+
+    return false;
+  },
+
+  es2016: () => {
+    return !hasMethods(Array.prototype, 'includes');
+  },
+
+  es2017: () => {
+    return !hasMethods(Object, 'entries', 'values');
+  },
+
+  // Test for a fully-working URL constructor.
+  url: () => {
+    try {
+      // Some browsers do not have a URL constructor at all.
+      const url = new window.URL('https://hypothes.is');
+
+      // Other browsers have a broken URL constructor.
+      if (url.hostname !== 'hypothes.is') {
+        throw new Error('Broken URL constructor');
+      }
+      return false;
+    } catch (e) {
+      return true;
+    }
+  },
+
+  // Test for XPath evaluation.
+  'document.evaluate': () => {
+    // Depending on the browser the `evaluate` property may be on the prototype
+    // or just the object itself.
+    return typeof document.evaluate !== 'function';
+  },
+
+  // Test for Unicode normalization. This depends on a large polyfill so it
+  // is separated out into its own bundle.
+  'string.prototype.normalize': () => {
+    return !hasMethods(String.prototype, 'normalize');
+  },
+};
+
+/**
+ * Return the subset of polyfill sets from `needed`  which are needed by the
+ * current browser.
+ */
+function requiredPolyfillSets(needed) {
+  return needed.filter(set => {
+    const checker = needsPolyfill[set];
+    if (!checker) {
+      throw new Error(`Unknown polyfill set "${set}"`);
+    }
+    return checker();
+  });
+}
+
+module.exports = {
+  requiredPolyfillSets,
+};

src/shared/polyfills/string.prototype.normalize.js

+'use strict';
+
+require('unorm');

src/shared/polyfills/test/index-test.js

+'use strict';
+
+const { requiredPolyfillSets } = require('../');
+
+function stubOut(obj, property, replacement = undefined) {
+  const saved = obj[property];
+
+  // We don't use `delete obj[property]` here because that isn't allowed for
+  // some native APIs in some browsers.
+  obj[property] = replacement;
+
+  return () => {
+    obj[property] = saved;
+  };
+}
+
+describe('shared/polyfills/index', () => {
+  describe('requiredPolyfillSets', () => {
+    let undoStub;
+
+    afterEach(() => {
+      if (undoStub) {
+        undoStub();
+        undoStub = null;
+      }
+    });
+
+    [
+      {
+        set: 'es2015',
+        providesMethod: [Object, 'assign'],
+      },
+      {
+        set: 'es2016',
+        providesMethod: [Array.prototype, 'includes'],
+      },
+      {
+        set: 'es2017',
+        providesMethod: [Object, 'entries'],
+      },
+      {
+        set: 'string.prototype.normalize',
+        providesMethod: [String.prototype, 'normalize'],
+      },
+      {
+        set: 'document.evaluate',
+        providesMethod: [Document.prototype, 'evaluate'],
+      },
+      {
+        // Missing URL constructor.
+        set: 'url',
+        providesMethod: [window, 'URL'],
+      },
+      {
+        // Broken URL constructor.
+        set: 'url',
+        providesMethod: [window, 'URL', () => {}],
+      },
+    ].forEach(({ set, providesMethod }) => {
+      it(`includes "${set}" if required`, () => {
+        const [obj, method, replacement] = providesMethod;
+        undoStub = stubOut(obj, method, replacement);
+        const sets = requiredPolyfillSets([set]);
+        assert.deepEqual(sets, [set]);
+      });
+
+      it(`does not include "${set}" if not required`, () => {
+        assert.deepEqual(requiredPolyfillSets([set]), []);
+      });
+    });
+  });
+});

src/shared/polyfills/url.js

+'use strict';
+
+require('js-polyfills/url');

src/sidebar/index.js

@@ -5,7 +5,6 @@ const disableOpenerForExternalLinks = require('./util/disable-opener-for-externa
 const { fetchConfig } = require('./util/fetch-config');
 const serviceConfig = require('./service-config');
 const crossOriginRPC = require('./cross-origin-rpc.js');
-require('../shared/polyfills');
 
 let raven;
 

src/sidebar/services/unicode.js

 'use strict';
 
-const unorm = require('unorm');
-
 /**
  * Unicode combining characters
  * from http://xregexp.com/addons/unicode/unicode-categories.js line:30
@@ -11,7 +9,7 @@ const COMBINING_MARKS = /[\u0300-\u036F\u0483-\u0489\u0591-\u05BD\u05BF\u05C1\u0
 // @ngInject
 function unicode() {
   return {
-    normalize: str => unorm.nfkd(str),
+    normalize: str => str.normalize('NFKD'),
     fold: str => str.replace(COMBINING_MARKS, ''),
   };
 }