✨ Launch Custom Elements v1 Polyfill (ampproject#25141)

* Launch Custom Elements v1 Polyfill There don't seem to be any errors. Let's launch! * Always use customElements registry * Fix isPatched check IE11 doesn't have `"[native code]"`, instead it returns `"[object HTMLElement]"`. Well, if there's no CE, then we obviously need to polyfill anyways. * Fix IE11 innerHTML overwrite IE11 puts the `innerHTML` accessor on `HTMLElement.prototype`, not `Element.prototype`. And we've already replaced `HTMLElement` with our polyfill class. That polyfill class directly inherits from the native `HTMLElement`, so we can access it via the `__proto__` field.
jeffjose · Oct 19, 2019 · c51ceb6 · c51ceb6
1 parent f2744a4
commit c51ceb6
Show file tree

Hide file tree

Showing 13 changed files with 56 additions and 152 deletions.
diff --git a/build-system/compile/compile.js b/build-system/compile/compile.js
@@ -103,7 +103,6 @@ function cleanupBuildDir() {
   del.sync('build/fake-module');
   del.sync('build/patched-module');
   del.sync('build/parsers');
-  fs.mkdirsSync('build/patched-module/document-register-element/build');
   fs.mkdirsSync('build/fake-module/third_party/babel');
   fs.mkdirsSync('build/fake-module/src/polyfills/');
   fs.mkdirsSync('build/fake-polyfills/src/polyfills');

diff --git a/build-system/compile/shorten-license.js b/build-system/compile/shorten-license.js
@@ -20,32 +20,6 @@ const pumpify = require('pumpify');
 const replace = require('gulp-regexp-sourcemaps');
 
 /* eslint-disable */
-const MIT_FULL = [
-'Permission is hereby granted, free of charge, to any person obtaining a copy',
-'of this software and associated documentation files (the "Software"), to deal',
-'in the Software without restriction, including without limitation the rights',
-'to use, copy, modify, merge, publish, distribute, sublicense, and/or sell',
-'copies of the Software, and to permit persons to whom the Software is',
-'furnished to do so, subject to the following conditions:',
-'',
-'The above copyright notice and this permission notice shall be included in',
-'all copies or substantial portions of the Software.',
-'',
-'THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR',
-'IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,',
-'FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE',
-'AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER',
-'LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,',
-'OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN',
-'THE SOFTWARE.'
-].join('\n');
-
-const MIT_SHORT = [
-'Use of this source code is governed by a MIT-style',
-'license that can be found in the LICENSE file or at',
-'https://opensource.org/licenses/MIT.'
-].join('\n');
-
 const POLYMER_BSD_FULL = [
 'This code may only be used under the BSD style license found at http://polymer.github.io/LICENSE.txt',
 'The complete set of authors may be found at http://polymer.github.io/AUTHORS.txt',
@@ -62,13 +36,9 @@ const BSD_SHORT = [
 
 /* eslint-enable */
 
-const LICENSES = [[MIT_FULL, MIT_SHORT], [POLYMER_BSD_FULL, BSD_SHORT]];
+const LICENSES = [[POLYMER_BSD_FULL, BSD_SHORT]];
 
-const PATHS = [
-  'third_party/webcomponentsjs/ShadowCSS.js',
-  'node_modules/document-register-element/build/' +
-    'document-register-element.patched.js',
-];
+const PATHS = ['third_party/webcomponentsjs/ShadowCSS.js'];
 
 /**
  * We can replace full-text of standard licenses with a pre-approved shorten
@@ -81,6 +51,10 @@ exports.shortenLicense = function() {
     return replace(regex, tuple[1], 'shorten-license');
   });
 
+  // Pumpify requires at least 2 streams
+  if (streams.length === 1) {
+    return streams[0];
+  }
   return pumpify.obj(streams);
 };
 

diff --git a/build-system/compile/sources.js b/build-system/compile/sources.js
@@ -46,8 +46,6 @@ const COMMON_GLOBS = [
   'node_modules/web-activities/activity-ports.js',
   'node_modules/@ampproject/animations/dist/animations.mjs',
   'node_modules/@ampproject/worker-dom/dist/amp/main.mjs',
-  'node_modules/document-register-element/build/' +
-    'document-register-element.patched.js',
 ];
 
 /**

diff --git a/build-system/global-configs/experiments-config.json b/build-system/global-configs/experiments-config.json
@@ -15,12 +15,5 @@
     "expirationDateUTC": "2019-11-10",
     "defineExperimentConstant": "_RTVEXP_INABOX_LITE"
   },
-  "experimentC": {
-    "name": "Custom Elements V1",
-    "environment": "AMP",
-    "command": "gulp dist --defineExperimentConstant=CUSTOM_ELEMENTS_V1",
-    "issue": "https://github.com/ampproject/amphtml/issues/17243",
-    "expirationDateUTC": "2020-01-01",
-    "defineExperimentConstant": "CUSTOM_ELEMENTS_V1"
-  }
+  "experimentC": {}
 }
diff --git a/build-system/tasks/presubmit-checks.js b/build-system/tasks/presubmit-checks.js
@@ -85,10 +85,6 @@ const forbiddenTerms = {
       'https://medium.com/gulpjs/gulp-util-ca3b1f9f9ac5 ' +
       'for a list of alternatives.',
   },
-  'document-register-element.node': {
-    message: 'Use `document-register-element.patched` instead',
-    whitelist: ['build-system/tasks/update-packages.js'],
-  },
   'sinon\\.(spy|stub|mock)\\(': {
     message: 'Use a sandbox instead to avoid repeated `#restore` calls',
   },

diff --git a/build-system/tasks/update-packages.js b/build-system/tasks/update-packages.js
@@ -37,24 +37,6 @@ function writeIfUpdated(patchedName, file) {
   }
 }
 
-/**
- * @param {string} filePath
- * @param {string} newFilePath
- * @param  {...any} args Search and replace string pairs.
- */
-function replaceInFile(filePath, newFilePath, ...args) {
-  let file = fs.readFileSync(filePath, 'utf8');
-  for (let i = 0; i < args.length; i += 2) {
-    const searchValue = args[i];
-    const replaceValue = args[i + 1];
-    if (!file.includes(searchValue)) {
-      throw new Error(`Expected "${searchValue}" to appear in ${filePath}.`);
-    }
-    file = file.replace(searchValue, replaceValue);
-  }
-  writeIfUpdated(newFilePath, file);
-}
-
 /**
  * Patches Web Animations API by wrapping its body into `install` function.
  * This gives us an option to call polyfill directly on the main window
@@ -94,28 +76,6 @@ function patchWebAnimations() {
   writeIfUpdated(patchedName, file);
 }
 
-/**
- * Creates a version of document-register-element that can be installed
- * without side effects.
- */
-function patchRegisterElement() {
-  // Copies document-register-element into a new file that has an export.
-  // This works around a bug in closure compiler, where without the
-  // export this module does not generate a goog.provide which fails
-  // compilation: https://github.com/google/closure-compiler/issues/1831
-  const dir = 'node_modules/document-register-element/build/';
-  replaceInFile(
-    dir + 'document-register-element.node.js',
-    dir + 'document-register-element.patched.js',
-    // Elimate the immediate side effect.
-    'installCustomElements(global);',
-    '',
-    // Replace CJS export with ES6 export.
-    'module.exports = installCustomElements;',
-    'export {installCustomElements};'
-  );
-}
-
 /**
  * Does a yarn check on node_modules, and if it is outdated, runs yarn.
  */
@@ -158,14 +118,13 @@ function maybeUpdatePackages() {
 
 /**
  * Installs custom lint rules, updates node_modules (for local dev), and patches
- * web-animations-js and document-register-element if necessary.
+ * web-animations-js if necessary.
  */
 async function updatePackages() {
   if (!isTravisBuild()) {
     runYarnCheck();
   }
   patchWebAnimations();
-  patchRegisterElement();
 }
 
 module.exports = {

diff --git a/package.json b/package.json
@@ -20,7 +20,6 @@
   "dependencies": {
     "@ampproject/animations": "0.2.0",
     "@ampproject/worker-dom": "0.21.0",
-    "document-register-element": "1.5.0",
     "dompurify": "2.0.2",
     "moment": "2.24.0",
     "preact": "8.4.2",

diff --git a/src/friendly-iframe-embed.js b/src/friendly-iframe-embed.js
@@ -37,12 +37,10 @@ import {
 } from './service';
 import {escapeHtml} from './dom';
 import {getExperimentBranch, isExperimentOn} from './experiments';
-import {getMode} from './mode';
 import {installAmpdocServices} from './service/core-services';
 import {install as installCustomElements} from './polyfills/custom-elements';
 import {install as installDOMTokenList} from './polyfills/domtokenlist';
 import {install as installDocContains} from './polyfills/document-contains';
-import {installCustomElements as installRegisterElement} from 'document-register-element/build/document-register-element.patched';
 import {installStylesForDoc, installStylesLegacy} from './style-installer';
 import {installTimerInEmbedWindow} from './service/timer-impl';
 import {isDocumentReady} from './document-ready';
@@ -857,15 +855,7 @@ export class FriendlyIframeEmbed {
 function installPolyfillsInChildWindow(parentWin, childWin) {
   installDocContains(childWin);
   installDOMTokenList(childWin);
-  if (
-    // eslint-disable-next-line no-undef
-    CUSTOM_ELEMENTS_V1 ||
-    getMode().test
-  ) {
-    installCustomElements(childWin);
-  } else {
-    installRegisterElement(childWin, 'auto');
-  }
+  installCustomElements(childWin);
 }
 
 /**

diff --git a/src/polyfills.js b/src/polyfills.js
@@ -16,7 +16,6 @@
 
 /** @fileoverview */
 
-import {getMode} from './mode';
 import {install as installArrayIncludes} from './polyfills/array-includes';
 import {install as installCustomElements} from './polyfills/custom-elements';
 import {install as installDOMTokenList} from './polyfills/domtokenlist';
@@ -27,7 +26,6 @@ import {install as installMathSign} from './polyfills/math-sign';
 import {install as installObjectAssign} from './polyfills/object-assign';
 import {install as installObjectValues} from './polyfills/object-values';
 import {install as installPromise} from './polyfills/promise';
-import {installCustomElements as installRegisterElement} from 'document-register-element/build/document-register-element.patched';
 
 installFetch(self);
 installMathSign(self);
@@ -41,17 +39,7 @@ if (self.document) {
   installDOMTokenList(self);
   installDocContains(self);
   installGetBoundingClientRect(self);
-
-  // TODO(jridgewell, estherkim): Find out why CE isn't being polyfilled for IE.
-  if (
-    // eslint-disable-next-line no-undef
-    CUSTOM_ELEMENTS_V1 ||
-    (getMode().test && !getMode().testIe)
-  ) {
-    installCustomElements(self);
-  } else {
-    installRegisterElement(self, 'auto');
-  }
+  installCustomElements(self);
 }
 
 // TODO(#18268, erwinm): For whatever reason imports to modules that have no

diff --git a/src/polyfills/custom-elements.js b/src/polyfills/custom-elements.js
@@ -672,13 +672,27 @@ function installPatches(win, registry) {
   // Patch the innerHTML setter to immediately upgrade custom elements.
   // Note, this could technically fire connectedCallbacks if this node was
   // connected, but we leave that to the Mutation Observer.
-  const innerHTMLDesc = Object.getOwnPropertyDescriptor(elProto, 'innerHTML');
+  let innerHTMLProto = elProto;
+  let innerHTMLDesc = Object.getOwnPropertyDescriptor(
+    innerHTMLProto,
+    'innerHTML'
+  );
+  if (!innerHTMLDesc) {
+    // Sigh... IE11 puts innerHTML desciptor on HTMLElement. But, we've
+    // replaced HTMLElement with a polyfill wrapper, so have to get its proto.
+    innerHTMLProto =
+      /** @type {!Object} */ (win.HTMLElement.prototype.__proto__);
+    innerHTMLDesc = Object.getOwnPropertyDescriptor(
+      innerHTMLProto,
+      'innerHTML'
+    );
+  }
   const innerHTMLSetter = innerHTMLDesc.set;
   innerHTMLDesc.set = function(html) {
     innerHTMLSetter.call(this, html);
     registry.upgrade(this);
   };
-  Object.defineProperty(elProto, 'innerHTML', innerHTMLDesc);
+  Object.defineProperty(innerHTMLProto, 'innerHTML', innerHTMLDesc);
 }
 
 /**
@@ -852,14 +866,15 @@ function subClass(Object, superClass, subClass) {
 export function install(win, opt_ctor) {
   // Don't install in no-DOM environments e.g. worker.
   const shouldInstall = win.document;
-  if (!shouldInstall || isPatched(win)) {
+  const hasCE = hasCustomElements(win);
+  if (!shouldInstall || (hasCE && isPatched(win))) {
     return;
   }
 
   let install = true;
   let installWrapper = false;
 
-  if (opt_ctor && hasCustomElements(win)) {
+  if (opt_ctor && hasCE) {
     // If ctor is constructable without new, it's a function. That means it was
     // compiled down, and we need to do the minimal polyfill because all you
     // cannot extend HTMLElement without native classes.

diff --git a/src/service/custom-element-registry.js b/src/service/custom-element-registry.js
@@ -141,15 +141,7 @@ export function registerElement(win, name, implementationClass) {
   const knownElements = getExtendedElements(win);
   knownElements[name] = implementationClass;
   const klass = createCustomElementClass(win, name);
-
-  const supportsCustomElementsV1 = 'customElements' in win;
-  if (supportsCustomElementsV1) {
-    win['customElements'].define(name, klass);
-  } else {
-    win.document.registerElement(name, {
-      prototype: klass.prototype,
-    });
-  }
+  win['customElements'].define(name, klass);
 }
 
 /**