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

♻️ Modernize polyfills #34342

Merged
merged 14 commits into from
May 13, 2021
55 changes: 19 additions & 36 deletions src/polyfills/custom-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/

import {Deferred} from '../core/data-structures/promise';
import {map} from '../core/types/object';
import {rethrowAsync} from '../core/error';

/**
* For type anotations where Element is a local variable.
Expand Down Expand Up @@ -98,19 +100,6 @@ function isPatched(win) {
return tag.indexOf('[native code]') === -1;
}

/**
* Throws the error outside the current event loop.
* TODO(rcebulko): Condense with core/error#rethrowAsync
*
* @param {!Error} error
*/
function rethrowAsync(error) {
setTimeout(() => {
self.__AMP_REPORT_ERROR(error);
throw error;
});
}

/**
* The public Custom Elements API.
*/
Expand All @@ -127,7 +116,7 @@ class CustomElementRegistry {
this.registry_ = registry;

/** @private @const @type {!Object<string, !Deferred>} */
this.pendingDefines_ = Object.create(null);
this.pendingDefines_ = map();
}

/**
Expand Down Expand Up @@ -212,7 +201,7 @@ class Registry {
this.win_ = win;

/** @private @const @type {!Object<string, !CustomElementDef>} */
this.definitions_ = Object.create(null);
this.definitions_ = map();

/**
* A up-to-date DOM selector for all custom elements.
Expand Down Expand Up @@ -320,9 +309,9 @@ class Registry {
};

this.observe_(name);
this.roots_.forEach((tree) => {
for (const tree of this.roots_) {
this.upgrade(tree, name);
});
}
}

/**
Expand All @@ -342,8 +331,7 @@ class Registry {
const query = opt_query || this.query_;
const upgradeCandidates = this.queryAll_(root, query);

for (let i = 0; i < upgradeCandidates.length; i++) {
const candidate = upgradeCandidates[i];
for (const candidate of upgradeCandidates) {
if (newlyDefined) {
this.connectedCallback_(candidate);
} else {
Expand Down Expand Up @@ -491,9 +479,9 @@ class Registry {
// I would love to not have to hold onto all of the roots, since it's a
// memory leak. Unfortunately, there's no way to iterate a list and hold
// onto its contents weakly.
this.roots_.forEach((tree) => {
for (const tree of this.roots_) {
rcebulko marked this conversation as resolved.
Show resolved Hide resolved
mo.observe(tree, TRACK_SUBTREE);
});
}

installPatches(this.win_, this);
}
Expand Down Expand Up @@ -530,28 +518,25 @@ class Registry {
* @param {!Array<!MutationRecord>} records
*/
handleRecords_(records) {
for (let i = 0; i < records.length; i++) {
const record = records[i];
for (const record of records) {
if (!record) {
continue;
}

const {addedNodes, removedNodes} = record;
for (let i = 0; i < addedNodes.length; i++) {
const node = addedNodes[i];
for (const node of addedNodes) {
const connectedCandidates = this.queryAll_(node, this.query_);
this.connectedCallback_(node);
for (let i = 0; i < connectedCandidates.length; i++) {
this.connectedCallback_(connectedCandidates[i]);
for (const candidate of connectedCandidates) {
this.connectedCallback_(candidate);
}
}

for (let i = 0; i < removedNodes.length; i++) {
const node = removedNodes[i];
for (const node of removedNodes) {
const disconnectedCandidates = this.queryAll_(node, this.query_);
this.disconnectedCallback_(node);
for (let i = 0; i < disconnectedCandidates.length; i++) {
this.disconnectedCallback_(disconnectedCandidates[i]);
for (const candidate of disconnectedCandidates) {
this.disconnectedCallback_(candidates);
}
}
}
Expand Down Expand Up @@ -660,7 +645,7 @@ function installPatches(win, registry) {
'innerHTML'
);
}
if (innerHTMLDesc && innerHTMLDesc.configurable) {
if (innerHTMLDesc?.configurable) {
const innerHTMLSetter = innerHTMLDesc.set;
innerHTMLDesc.set = function (html) {
innerHTMLSetter.call(this, html);
Expand Down Expand Up @@ -884,9 +869,7 @@ export function copyProperties(obj, prototype) {
break;
}

const props = Object.getOwnPropertyNames(current);
for (let i = 0; i < props.length; i++) {
const prop = props[i];
for (const prop of Object.getOwnPropertyNames(current)) {
if (Object.hasOwnProperty.call(obj, prop)) {
continue;
}
Expand Down Expand Up @@ -949,7 +932,7 @@ export function install(win, ctor) {

// If that didn't throw, we're transpiled.
// Let's find out if we can wrap HTMLElement and avoid a full patch.
installWrapper = !!(Reflect && Reflect.construct);
installWrapper = !!Reflect?.construct;
} catch (e) {
// The ctor threw when we constructed it via ES5, so it's a real class.
// We're ok to not install the polyfill.
Expand Down
10 changes: 5 additions & 5 deletions src/polyfills/fetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ let XMLHttpRequestDef;
* @return {!Promise<!FetchResponse>}
*/
export function fetchPolyfill(input, init = {}) {
return new Promise(function (resolve, reject) {
return new Promise((resolve, reject) => {
const requestMethod = normalizeMethod(init.method || 'GET');
const xhr = createXhrRequest(requestMethod, input);

Expand All @@ -68,9 +68,9 @@ export function fetchPolyfill(input, init = {}) {
}

if (init.headers) {
Object.keys(init.headers).forEach(function (header) {
for (const header of Object.keys(init.headers)) {
rcebulko marked this conversation as resolved.
Show resolved Hide resolved
xhr.setRequestHeader(header, init.headers[header]);
});
}
}

xhr.onreadystatechange = () => {
Expand Down Expand Up @@ -280,12 +280,12 @@ export class Response extends FetchResponse {
data.status = init.status === undefined ? 200 : parseInt(init.status, 10);

if (isArray(init.headers)) {
/** @type {!Array} */ (init.headers).forEach((entry) => {
for (const entry of /** @type {!Array} */ (init.headers)) {
rcebulko marked this conversation as resolved.
Show resolved Hide resolved
const headerName = entry[0];
const headerValue = entry[1];
lowercasedHeaders[String(headerName).toLowerCase()] =
String(headerValue);
});
}
} else if (isObject(init.headers)) {
for (const key in init.headers) {
lowercasedHeaders[String(key).toLowerCase()] = String(
Expand Down
3 changes: 1 addition & 2 deletions src/polyfills/object-assign.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ export function assign(target, var_args) {
}

const output = Object(target);
for (let i = 1; i < arguments.length; i++) {
const source = arguments[i];
for (const source of arguments) {
rcebulko marked this conversation as resolved.
Show resolved Hide resolved
if (source != null) {
for (const key in source) {
if (hasOwnProperty.call(source, key)) {
Expand Down
4 changes: 3 additions & 1 deletion src/polyfills/stubs/intersection-observer-stub.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,9 @@ export class IntersectionObserverStub {
upgrade_(Ctor) {
const inst = new Ctor(this.callback_, this.options_);
this.inst_ = inst;
this.elements_.forEach((e) => inst.observe(e));
for (const e of this.elements_) {
inst.observe(e);
}
this.elements_ = null;
}
}
Expand Down
8 changes: 6 additions & 2 deletions src/polyfills/stubs/resize-observer-stub.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ export function upgradePolyfill(win, installer) {
const upgrade = (upgrader) => {
microtask.then(() => upgrader(Polyfill));
};
upgraders.forEach(upgrade);
for (const upgrader of upgraders) {
upgrade(upgrader);
}
Stub[UPGRADERS] = {'push': upgrade};
} else {
// Even if this is not the stub, we still may need to install the polyfill.
Expand Down Expand Up @@ -144,7 +146,9 @@ export class ResizeObserverStub {
upgrade_(Ctor) {
const inst = new Ctor(this.callback_);
this.inst_ = inst;
this.elements_.forEach((e) => inst.observe(e));
for (const e of this.elements_) {
inst.observe(e);
}
this.elements_ = null;
}
}
Expand Down