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
41 changes: 18 additions & 23 deletions src/polyfills/custom-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

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

/**
Expand Down Expand Up @@ -115,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 @@ -200,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 @@ -308,9 +309,9 @@ class Registry {
};

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

/**
Expand All @@ -330,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 @@ -479,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 @@ -518,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_(candidate);
}
}
}
Expand Down Expand Up @@ -648,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 @@ -872,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 @@ -937,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
8 changes: 5 additions & 3 deletions src/polyfills/stubs/intersection-observer-stub.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ export class IntersectionObserverStub {
...options,
};

/** @private {?Array<!Element>} */
/** @private {!Array<!Element>} */
this.elements_ = [];

/** @private {?IntersectionObserver} */
Expand Down Expand Up @@ -268,8 +268,10 @@ export class IntersectionObserverStub {
upgrade_(Ctor) {
const inst = new Ctor(this.callback_, this.options_);
this.inst_ = inst;
this.elements_.forEach((e) => inst.observe(e));
this.elements_ = null;
for (const e of this.elements_) {
inst.observe(e);
}
this.elements_.length = 0;
}
}

Expand Down
12 changes: 8 additions & 4 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 All @@ -95,7 +97,7 @@ export class ResizeObserverStub {
/** @private @const {!ResizeObserverCallback} */
this.callback_ = callback;

/** @private {?Array<!Element>} */
/** @private {!Array<!Element>} */
this.elements_ = [];

/** @private {?ResizeObserver} */
Expand Down Expand Up @@ -144,8 +146,10 @@ export class ResizeObserverStub {
upgrade_(Ctor) {
const inst = new Ctor(this.callback_);
this.inst_ = inst;
this.elements_.forEach((e) => inst.observe(e));
this.elements_ = null;
for (const e of this.elements_) {
inst.observe(e);
}
this.elements_.length = 0;
}
}

Expand Down
4 changes: 2 additions & 2 deletions test/unit/polyfills/test-intersection-observer.js
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ describes.fakeWin('IntersectionObserverStub', {}, (env) => {
io.unobserve(element1);
const native = upgrade(io);
expect(native.observe).to.not.be.called;
expect(io.elements_).to.be.null;
expect(io.elements_.length).to.equal(0);
});

it('should re-observe previously observed elements', () => {
Expand All @@ -573,7 +573,7 @@ describes.fakeWin('IntersectionObserverStub', {}, (env) => {
expect(native.observe).to.be.calledTwice;
expect(native.observe).to.be.calledWith(element1);
expect(native.observe).to.be.calledWith(element2);
expect(io.elements_).to.be.null;
expect(io.elements_.length).to.equal(0);
});

it('should observe new elements only on native', () => {
Expand Down
4 changes: 2 additions & 2 deletions test/unit/polyfills/test-resize-observer.js
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ describes.fakeWin('ResizeObserverStub', {}, (env) => {
ro.unobserve(element1);
const native = upgrade(ro);
expect(native.observe).to.not.be.called;
expect(ro.elements_).to.be.null;
expect(ro.elements_.length).to.equal(0);
});

it('should re-observe previously observed elements', () => {
Expand All @@ -303,7 +303,7 @@ describes.fakeWin('ResizeObserverStub', {}, (env) => {
expect(native.observe).to.be.calledTwice;
expect(native.observe).to.be.calledWith(element1);
expect(native.observe).to.be.calledWith(element2);
expect(ro.elements_).to.be.null;
expect(ro.elements_.length).to.equal(0);
});

it('should observe new elements only on native', () => {
Expand Down