Skip to content

Commit

Permalink
♻️ Use NodeList.forEach (ampproject#38004)
Browse files Browse the repository at this point in the history
Since we no longer support IE, we can take advantage of [`NodeList.forEach`](https://developer.mozilla.org/en-US/docs/Web/API/NodeList/forEach). This replaces niche techniques:

- `Array.prototype.forEach.call`
- `iterateCursor`
  • Loading branch information
alanorozco authored Apr 4, 2022
1 parent 993fd47 commit f24834d
Show file tree
Hide file tree
Showing 24 changed files with 73 additions and 110 deletions.
3 changes: 1 addition & 2 deletions extensions/amp-bind/0.1/bind-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import {AmpEvents_Enum} from '#core/constants/amp-events';
import {Deferred} from '#core/data-structures/promise';
import {Signals} from '#core/data-structures/signals';
import {isAmp4Email} from '#core/document/format';
import {iterateCursor} from '#core/dom';
import {whenUpgradedToCustomElement} from '#core/dom/amp-element-helpers';
import {escapeCssSelectorIdent} from '#core/dom/css-selectors';
import {closestAncestorElementBySelector} from '#core/dom/query';
Expand Down Expand Up @@ -705,7 +704,7 @@ export class Bind {
// created elements. Should do what <amp-state> does.
const elements = this.ampdoc.getBody().querySelectorAll('AMP-BIND-MACRO');
const macros = /** @type {!Array<!BindMacroDef>} */ ([]);
iterateCursor(elements, (element) => {
elements.forEach((element) => {
const argumentNames = (element.getAttribute('arguments') || '')
.split(',')
.map((s) => s.trim());
Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-date-picker/0.1/amp-date-picker.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {AmpEvents_Enum} from '#core/constants/amp-events';
import {Keys_Enum} from '#core/constants/key-codes';
import {FiniteStateMachine} from '#core/data-structures/finite-state-machine';
import {Deferred} from '#core/data-structures/promise';
import {isRTL, iterateCursor, tryFocus} from '#core/dom';
import {isRTL, tryFocus} from '#core/dom';
import {escapeCssSelectorIdent} from '#core/dom/css-selectors';
import {Layout_Enum, isLayoutSizeDefined} from '#core/dom/layout';
import {
Expand Down Expand Up @@ -1310,7 +1310,7 @@ export class AmpDatePicker extends AMP.BaseElement {
*/
parseElementTemplates_(templates) {
const parsed = [];
iterateCursor(templates, (template) =>
templates.forEach((template) =>
parsed.push(this.parseElementTemplate_(template))
);
return parsed;
Expand Down
6 changes: 3 additions & 3 deletions extensions/amp-form/0.1/amp-form-textarea.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {AmpEvents_Enum} from '#core/constants/amp-events';
import {iterateCursor, removeElement} from '#core/dom';
import {removeElement} from '#core/dom';
import {computedStyle, px, setStyle} from '#core/dom/style';
import {toArray} from '#core/types/array';
import {throttle} from '#core/types/function';
Expand Down Expand Up @@ -170,10 +170,10 @@ export function getHasOverflow(element) {

/**
* Attempt to resize all textarea elements
* @param {!IArrayLike<!Element>} elements
* @param {!NodeList} elements
*/
function resizeTextareaElements(elements) {
iterateCursor(elements, (element) => {
elements.forEach((element) => {
if (
element.tagName != 'TEXTAREA' ||
!element.hasAttribute(AMP_FORM_TEXTAREA_EXPAND_ATTR)
Expand Down
15 changes: 7 additions & 8 deletions extensions/amp-form/0.1/amp-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -523,15 +523,15 @@ export class AmpForm {
const validityElements = this.form_.querySelectorAll(
'.user-valid, .user-invalid'
);
iterateCursor(validityElements, (element) => {
validityElements.forEach((element) => {
element.classList.remove('user-valid');
element.classList.remove('user-invalid');
});

const messageElements = this.form_.querySelectorAll(
'.visible[validation-for]'
);
iterateCursor(messageElements, (element) => {
messageElements.forEach((element) => {
element.classList.remove('visible');
});

Expand Down Expand Up @@ -1401,7 +1401,7 @@ export class AmpForm {
maybeFillField(field, key);
} else if (formControls.length) {
const fields = /** @type {!NodeList} */ (formControls);
iterateCursor(fields, (field) => maybeFillField(field, key));
fields.forEach((field) => maybeFillField(field, key));
}
});
}
Expand Down Expand Up @@ -1434,7 +1434,7 @@ export class AmpForm {
*/
function checkUserValidityOnSubmission(form) {
const elements = form.querySelectorAll('input,select,textarea,fieldset');
iterateCursor(elements, (element) => checkUserValidity(element));
elements.forEach((element) => checkUserValidity(element));
return checkUserValidity(form);
}

Expand Down Expand Up @@ -1477,7 +1477,7 @@ function removeValidityStateClasses(form) {
const elements = form.querySelectorAll(
`.${escapeCssSelectorIdent(validityState)}`
);
iterateCursor(elements, (element) => {
elements.forEach((element) => {
dev().assertElement(element).classList.remove(validityState);
});
}
Expand Down Expand Up @@ -1620,16 +1620,15 @@ export class AmpFormService {

/**
* Install submission handler on all forms in the document.
* @param {?IArrayLike<T>} forms
* @template T
* @param {NodeList} forms
* @private
*/
installSubmissionHandlers_(forms) {
if (!forms) {
return;
}

iterateCursor(forms, (form, index) => {
forms.forEach((form, index) => {
const existingAmpForm = formOrNullForElement(form);
if (!existingAmpForm) {
new AmpForm(form, `amp-form-${index}`);
Expand Down
3 changes: 1 addition & 2 deletions extensions/amp-fx-collection/0.1/amp-fx-collection.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {AmpEvents_Enum} from '#core/constants/amp-events';
import {iterateCursor} from '#core/dom';
import {tryCallback} from '#core/error';

import {listen} from '#utils/event-helper';
Expand Down Expand Up @@ -49,7 +48,7 @@ export class AmpFxCollection {
*/
scan_() {
const elements = this.ampdoc_.getRootNode().querySelectorAll('[amp-fx]');
iterateCursor(elements, (element) => {
elements.forEach((element) => {
if (this.seen_.includes(element)) {
return;
}
Expand Down
27 changes: 10 additions & 17 deletions extensions/amp-inline-gallery/0.1/amp-inline-gallery-slide.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {Layout_Enum} from '#core/dom/layout';
import {htmlFor} from '#core/dom/static-template';
import {toArray} from '#core/types/array';

import {isExperimentOn} from '#experiments';

Expand Down Expand Up @@ -85,23 +84,17 @@ export class AmpInlineGallerySlide extends AMP.BaseElement {
const attributionSlot = dom.querySelector(
'.i-amphtml-inline-gallery-slide-persistent-slot'
);
const childNodesArray = toArray(this.element.childNodes);

childNodesArray
.filter((n) => {
return n.hasAttribute && n.getAttribute('slot') === 'caption';
})
.forEach((node) => captionSlot.appendChild(node));
childNodesArray
.filter((n) => {
return !n.hasAttribute || !n.hasAttribute('slot');
})
.forEach((node) => contentSlot.appendChild(node));
childNodesArray
.filter((n) => {
return n.hasAttribute && n.getAttribute('slot') === 'attribution';
})
.forEach((node) => attributionSlot.appendChild(node));
this.element.childNodes.forEach((node) => {
const slot = node.getAttribute?.('slot');
if (slot === 'caption') {
captionSlot.appendChild(node);
} else if (slot === 'attribution') {
attributionSlot.appendChild(node);
} else if (slot == null) {
contentSlot.appendChild(node);
}
});

this.element.appendChild(dom);
}
Expand Down
30 changes: 13 additions & 17 deletions extensions/amp-inline-gallery/0.1/amp-inline-gallery.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import {iterateCursor} from '#core/dom';
import {Layout_Enum} from '#core/dom/layout';
import {scopedQuerySelector, scopedQuerySelectorAll} from '#core/dom/query';
import {toArray} from '#core/types/array';
Expand Down Expand Up @@ -92,14 +91,14 @@ class AmpInlineGallery extends AMP.BaseElement {
* @private
*/
updateProgress_(total, index, offset, slides) {
iterateCursor(
scopedQuerySelectorAll(this.element, CHILDREN_FOR_PROGRESS_SELECTOR),
(el) => {
el.getImpl().then((pagination) => {
pagination.updateProgress(total, index, offset, slides);
});
}
);
scopedQuerySelectorAll(
this.element,
CHILDREN_FOR_PROGRESS_SELECTOR
).forEach((el) => {
el.getImpl().then((pagination) => {
pagination.updateProgress(total, index, offset, slides);
});
});
}

/**
Expand Down Expand Up @@ -137,14 +136,11 @@ class AmpInlineGallery extends AMP.BaseElement {
const detail = getDetail(event);
const index = detail['index'];

iterateCursor(
scopedQuerySelectorAll(this.element, CAROUSEL_SELECTOR),
(el) => {
el.getImpl().then((carousel) => {
carousel.goToSlide(index, {smoothScroll: true});
});
}
);
scopedQuerySelectorAll(this.element, CAROUSEL_SELECTOR).forEach((el) => {
el.getImpl().then((carousel) => {
carousel.goToSlide(index, {smoothScroll: true});
});
});
}
}

Expand Down
3 changes: 1 addition & 2 deletions extensions/amp-inputmask/0.1/amp-inputmask.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {AmpEvents_Enum} from '#core/constants/amp-events';
import {iterateCursor} from '#core/dom';

import {listen} from '#utils/event-helper';

Expand Down Expand Up @@ -33,7 +32,7 @@ export class AmpInputmaskService {
const maskElements = this.ampdoc
.getRootNode()
.querySelectorAll('input[mask]');
iterateCursor(maskElements, (element) => {
maskElements.forEach((element) => {
if (TextMask.isMasked(element)) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {AmpEvents_Enum} from '#core/constants/amp-events';
import {CommonSignals_Enum} from '#core/constants/common-signals';
import {iterateCursor} from '#core/dom';
import {
childElement,
childElementByAttr,
Expand Down Expand Up @@ -153,7 +152,7 @@ export class LightboxManager {
.getRootNode()
.querySelectorAll('[lightbox],[data-lightbox]');
const processLightboxElement = this.processLightboxElement_.bind(this);
iterateCursor(matches, processLightboxElement);
matches.forEach(processLightboxElement);
});
}

Expand Down
3 changes: 1 addition & 2 deletions extensions/amp-link-rewriter/0.1/scope.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import {iterateCursor} from '#core/dom';
import {closestAncestorElementBySelector} from '#core/dom/query';

/**
Expand All @@ -18,7 +17,7 @@ export function getScopeElements(ampDoc, configOpts) {
selection = doc.querySelectorAll(cssSelector);
}

iterateCursor(selection, (element) => {
selection.forEach((element) => {
if (hasAttributeValues(element, configOpts)) {
filteredSelection.push(element);
}
Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-mustache/0.1/amp-mustache.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {iterateCursor, templateContentClone} from '#core/dom';
import {templateContentClone} from '#core/dom';

import {user} from '#utils/log';

Expand Down Expand Up @@ -85,7 +85,7 @@ export class AmpMustache extends BaseTemplate {
*/
processNestedTemplates_(content) {
const templates = content.querySelectorAll('template');
iterateCursor(templates, (nestedTemplate, index) => {
templates.forEach((nestedTemplate, index) => {
const nestedTemplateKey = `__AMP_NESTED_TEMPLATE_${index}`;
this.nestedTemplates_[nestedTemplateKey] =
nestedTemplate./*OK*/ outerHTML;
Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-mustache/0.2/amp-mustache.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {iterateCursor, templateContentClone} from '#core/dom';
import {templateContentClone} from '#core/dom';

import {Purifier} from '#purifier';

Expand Down Expand Up @@ -88,7 +88,7 @@ export class AmpMustache extends BaseTemplate {
*/
processNestedTemplates_(content) {
const templates = content.querySelectorAll('template');
iterateCursor(templates, (template, index) => {
templates.forEach((template, index) => {
const key = `__AMP_NESTED_TEMPLATE_${index}`;

// Store the nested template markup, keyed by index.
Expand Down
6 changes: 3 additions & 3 deletions extensions/amp-story-auto-ads/0.1/story-ad-ui.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {createElementWithAttributes, iterateCursor} from '#core/dom';
import {createElementWithAttributes} from '#core/dom';
import {map} from '#core/types/object';
import {getWin} from '#core/window';

Expand Down Expand Up @@ -77,7 +77,7 @@ export function getStoryAdMetaTags(doc) {
*/
export function getStoryAdMetadataFromDoc(metaTags) {
const vars = map();
iterateCursor(metaTags, (tag) => {
metaTags.forEach((tag) => {
const {content, name} = tag;
if (name.startsWith(CTA_META_PREFIX)) {
const key = name.split('amp-')[1];
Expand All @@ -98,7 +98,7 @@ export function getStoryAdMetadataFromDoc(metaTags) {
*/
export function getStoryAdMacroTags(metaTags) {
const result = map();
iterateCursor(metaTags, (tag) => {
metaTags.forEach((tag) => {
const {content, name} = tag;
// If the meta tag name is not alphanumerical, we would ignore it.
if (/^[a-zA-Z0-9\-_]+$/.test(name)) {
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-story/1.0/amp-story-grid-layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export class AmpStoryGridLayer extends AmpStoryBaseLayer {
SUPPORTED_CSS_GRID_ATTRIBUTES_SELECTOR
);

Array.prototype.forEach.call(elementsToUpgradeStyles, (element) => {
elementsToUpgradeStyles.forEach((element) => {
this.setCssGridStyles_(element);
});
}
Expand Down
20 changes: 8 additions & 12 deletions extensions/amp-story/1.0/amp-story-page.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
*/
import {CommonSignals_Enum} from '#core/constants/common-signals';
import {Deferred} from '#core/data-structures/promise';
import {iterateCursor} from '#core/dom';
import {whenUpgradedToCustomElement} from '#core/dom/amp-element-helpers';
import * as Preact from '#core/dom/jsx';
import {Layout_Enum} from '#core/dom/layout';
Expand Down Expand Up @@ -389,7 +388,7 @@ export class AmpStoryPage extends AMP.BaseElement {
* play videos from an inactive page.
*/
delegateVideoAutoplay() {
iterateCursor(this.element.querySelectorAll('amp-video'), delegateAutoplay);
this.element.querySelectorAll('amp-video').forEach(delegateAutoplay);
}

/** @private */
Expand All @@ -413,7 +412,7 @@ export class AmpStoryPage extends AMP.BaseElement {
*/
markMediaElementsWithPreload_() {
const mediaSet = this.element.querySelectorAll('amp-audio, amp-video');
Array.prototype.forEach.call(mediaSet, (mediaItem) => {
mediaSet.forEach((mediaItem) => {
mediaItem.setAttribute('preload', 'auto');
});
}
Expand Down Expand Up @@ -738,18 +737,15 @@ export class AmpStoryPage extends AMP.BaseElement {
);
const mediaSet = [];

iterateCursor(scopedQuerySelectorAll(this.element, selector), (el) =>
scopedQuerySelectorAll(this.element, selector).forEach((el) =>
mediaSet.push(el)
);

if (fie) {
iterateCursor(
scopedQuerySelectorAll(
fie.win.document.body,
selector.replace(/amp-story-grid-layer/g, '')
),
(el) => mediaSet.push(el)
);
scopedQuerySelectorAll(
fie.win.document.body,
selector.replace(/amp-story-grid-layer/g, '')
).forEach((el) => mediaSet.push(el));
}

return mediaSet;
Expand Down Expand Up @@ -1420,7 +1416,7 @@ export class AmpStoryPage extends AMP.BaseElement {
}
}

Array.prototype.forEach.call(videoEls, (videoEl) => {
videoEls.forEach((videoEl) => {
this.unlisteners_.push(
listen(videoEl, 'playing', () =>
this.debounceToggleLoadingSpinner_(false)
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-story/1.0/amp-story.js
Original file line number Diff line number Diff line change
Expand Up @@ -2439,7 +2439,7 @@ export class AmpStory extends AMP.BaseElement {
'amp-story-cta-layer a'
);

Array.prototype.forEach.call(ctaAnchorEls, (ctaAnchorEl) => {
ctaAnchorEls.forEach((ctaAnchorEl) => {
ctaAnchorEl.setAttribute('data-vars-story-page-id', pageId);
ctaAnchorEl.setAttribute('data-vars-story-page-index', pageIndex);
});
Expand Down
Loading

0 comments on commit f24834d

Please sign in to comment.