Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Launch Custom Elements v1 Polyfill #25141

Merged
merged 10 commits into from
Oct 19, 2019
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion build-system/compile/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
6 changes: 1 addition & 5 deletions build-system/compile/shorten-license.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,7 @@ const BSD_SHORT = [

const LICENSES = [[MIT_FULL, MIT_SHORT], [POLYMER_BSD_FULL, BSD_SHORT]];
rsimha marked this conversation as resolved.
Show resolved Hide resolved

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
Expand Down
2 changes: 0 additions & 2 deletions build-system/compile/sources.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
];

/**
Expand Down
9 changes: 1 addition & 8 deletions build-system/global-configs/experiments-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {}
}
4 changes: 0 additions & 4 deletions build-system/tasks/presubmit-checks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
Expand Down
43 changes: 1 addition & 42 deletions build-system/tasks/update-packages.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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 = {
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
12 changes: 1 addition & 11 deletions src/friendly-iframe-embed.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
}

/**
Expand Down
14 changes: 1 addition & 13 deletions src/polyfills.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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);
Expand All @@ -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
Expand Down
22 changes: 18 additions & 4 deletions src/polyfills/custom-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -672,13 +672,26 @@ 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 = 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);
}

/**
Expand Down Expand Up @@ -852,14 +865,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.
Expand Down
10 changes: 1 addition & 9 deletions src/service/custom-element-registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
46 changes: 26 additions & 20 deletions test/unit/test-custom-element-registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ describes.realWin('CustomElement register', {amp: true}, env => {
elements = [];

doc = {
registerElement: sandbox.spy(),
documentElement: {
ownerDocument: doc,
},
Expand Down Expand Up @@ -198,6 +197,9 @@ describes.realWin('CustomElement register', {amp: true}, env => {

win = {
document: doc,
customElements: {
define: sandbox.spy(),
},
Object: {
create: proto => Object.create(proto),
},
Expand All @@ -220,8 +222,8 @@ describes.realWin('CustomElement register', {amp: true}, env => {
expect(win.__AMP_EXTENDED_ELEMENTS).to.exist;
expect(win.__AMP_EXTENDED_ELEMENTS['amp-test1']).to.equal(ElementStub);
expect(win.__AMP_EXTENDED_ELEMENTS['amp-test2']).to.be.undefined;
expect(doc.registerElement).to.be.calledOnce;
expect(doc.registerElement.firstCall.args[0]).to.equal('amp-test1');
expect(win.customElements.define).to.be.calledOnce;
expect(win.customElements.define.firstCall.args[0]).to.equal('amp-test1');
});

it('should repeat stubbing when body is not available', () => {
Expand All @@ -232,8 +234,8 @@ describes.realWin('CustomElement register', {amp: true}, env => {
expect(win.__AMP_EXTENDED_ELEMENTS).to.exist;
expect(win.__AMP_EXTENDED_ELEMENTS['amp-test1']).to.equal(ElementStub);
expect(win.__AMP_EXTENDED_ELEMENTS['amp-test2']).to.be.undefined;
expect(doc.registerElement).to.be.calledOnce;
expect(doc.registerElement.firstCall.args[0]).to.equal('amp-test1');
expect(win.customElements.define).to.be.calledOnce;
expect(win.customElements.define.firstCall.args[0]).to.equal('amp-test1');

// Add more elements
const elem2 = {
Expand All @@ -251,8 +253,10 @@ describes.realWin('CustomElement register', {amp: true}, env => {
stubElementsForDoc(ampdoc);
expect(win.__AMP_EXTENDED_ELEMENTS['amp-test1']).to.equal(ElementStub);
expect(win.__AMP_EXTENDED_ELEMENTS['amp-test2']).to.equal(ElementStub);
expect(doc.registerElement).to.have.callCount(2);
expect(doc.registerElement.getCall(1).args[0]).to.equal('amp-test2');
expect(win.customElements.define).to.have.callCount(2);
expect(win.customElements.define.getCall(1).args[0]).to.equal(
'amp-test2'
);
});

it('should stub element when not stubbed yet', () => {
Expand All @@ -261,38 +265,40 @@ describes.realWin('CustomElement register', {amp: true}, env => {

expect(win.__AMP_EXTENDED_ELEMENTS).to.exist;
expect(win.__AMP_EXTENDED_ELEMENTS['amp-test1']).to.equal(ElementStub);
expect(doc.registerElement).to.be.calledOnce;
expect(doc.registerElement.firstCall.args[0]).to.equal('amp-test1');
expect(win.customElements.define).to.be.calledOnce;
expect(win.customElements.define.firstCall.args[0]).to.equal('amp-test1');

// Second stub is ignored.
stubElementIfNotKnown(win, 'amp-test1');
expect(doc.registerElement).to.be.calledOnce;
expect(win.customElements.define).to.be.calledOnce;
});

it('should copy or stub element definitions in a child window', () => {
stubElementIfNotKnown(win, 'amp-test1');

const registerElement = sandbox.spy();
const childWin = {Object, HTMLElement, document: {registerElement}};
const define = sandbox.spy();
const childWin = {
Object,
HTMLElement,
customElements: {define},
};

copyElementToChildWindow(win, childWin, 'amp-test1');
expect(childWin.__AMP_EXTENDED_ELEMENTS['amp-test1']).to.equal(
ElementStub
);
const firstCallCount = registerElement.callCount;
const firstCallCount = define.callCount;
expect(firstCallCount).to.equal(1);
expect(registerElement.getCall(firstCallCount - 1).args[0]).to.equal(
'amp-test1'
);
expect(define.getCall(firstCallCount - 1).args[0]).to.equal('amp-test1');

copyElementToChildWindow(win, childWin, 'amp-test2');
expect(childWin.__AMP_EXTENDED_ELEMENTS['amp-test1']).to.equal(
ElementStub
);
expect(registerElement.callCount).to.be.above(firstCallCount);
expect(
registerElement.getCall(registerElement.callCount - 1).args[0]
).to.equal('amp-test2');
expect(define.callCount).to.be.above(firstCallCount);
expect(define.getCall(define.callCount - 1).args[0]).to.equal(
'amp-test2'
);
});
});
});
5 changes: 0 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4325,11 +4325,6 @@ doctrine@3.0.0, doctrine@^3.0.0:
dependencies:
esutils "^2.0.2"

document-register-element@1.5.0:
version "1.5.0"
resolved "https://registry.yarnpkg.com/document-register-element/-/document-register-element-1.5.0.tgz#a91bc20afd9340d50cdb6a493afaf10932c12e00"
integrity sha1-qRvCCv2TQNUM22pJOvrxCTLBLgA=

dom-serialize@^2.2.0:
version "2.2.1"
resolved "https://registry.yarnpkg.com/dom-serialize/-/dom-serialize-2.2.1.tgz#562ae8999f44be5ea3076f5419dcd59eb43ac95b"
Expand Down