🏗Prefer destructuring objects (#15204)

* Use destructuring

* Cleanup

* Force renaming if it'll help

* Lint

* Updates after rebase

* Fixes

* Fix lint

* comment

* Updates

* tmp

* Lint

* tmp

* Only consider identifiers and member expressions idempotent

* Only force idempotent combined destructures

.eslintrc

@@ -61,13 +61,15 @@
  "amphtml-internal/enforce-private-props": 2,
  "amphtml-internal/html-template": 2,
  "amphtml-internal/no-array-destructuring": 2,
+ "amphtml-internal/no-deep-destructuring": 2,
  "amphtml-internal/no-es2015-number-props": 2,
  "amphtml-internal/no-export-side-effect": 2,
  "amphtml-internal/no-for-of-statement": 2,
  "amphtml-internal/no-global": 0,
  "amphtml-internal/no-spread": 2,
  "amphtml-internal/no-style-property-setting": 2,
  "amphtml-internal/prefer-deferred-promise": 1,
+ "amphtml-internal/prefer-destructuring": 2,
  "amphtml-internal/query-selector": 2,
  "amphtml-internal/todo-format": 0,
  "amphtml-internal/unused-private-field": 1,

3p/3p.js

@@ -185,7 +185,7 @@ export function validateSrcContains(string, src) {
  * done. The first argument is the result.
  */
 export function computeInMasterFrame(global, taskId, work, cb) {
- const master = global.context.master;
+ const {master} = global.context;
  let tasks = master.__ampMasterTasks;
  if (!tasks) {
  tasks = master.__ampMasterTasks = {};

3p/beopinion.js

@@ -33,7 +33,7 @@ function getBeOpinion(global) {
  * @description Make canonicalUrl available from iframe
  */
 function addCanonicalLinkTag(global) {
- const canonicalUrl = global.context.canonicalUrl;
+ const {canonicalUrl} = global.context;
  if (canonicalUrl) {
  const link = global.document.createElement('link');
  link.setAttribute('rel', 'canonical');
@@ -82,7 +82,7 @@ function createContainer(global, data) {
 }
 
 function getBeOpinionAsyncInit(global, accountId) {
- const context = global.context;
+ const {context} = global;
  return function() {
  global.BeOpinionSDK.init({
  account: accountId,

3p/environment.js

@@ -73,7 +73,7 @@ function manageWin_(win) {
  */
 function instrumentDocWrite(parent, win) {
  const doc = win.document;
- const close = doc.close;
+ const {close} = doc;
  doc.close = function() {
  parent.ampManageWin = function(win) {
  manageWin(win);
@@ -178,7 +178,7 @@ function installObserver(win) {
  */
 function instrumentEntryPoints(win) {
  // Change setTimeout to respect a minimum timeout.
- const setTimeout = win.setTimeout;
+ const {setTimeout} = win;
  win.setTimeout = function(fn, time) {
  time = minTime(time);
  arguments[1] = time;
@@ -205,7 +205,7 @@ function instrumentEntryPoints(win) {
  next();
  return id;
  };
- const clearInterval = win.clearInterval;
+ const {clearInterval} = win;
  win.clearInterval = function(id) {
  clearInterval(id);
  win.clearTimeout(intervals[id]);

3p/integration.js

@@ -607,7 +607,7 @@ export function validateAllowedEmbeddingOrigins(window, allowedHostnames) {
  // We prefer the unforgable ancestorOrigins, but referrer is better than
  // nothing.
  const ancestor = ancestors ? ancestors[0] : window.document.referrer;
- let hostname = parseUrl(ancestor).hostname;
+ let {hostname} = parseUrl(ancestor);
  if (isProxyOrigin(ancestor)) {
  // If we are on the cache domain, parse the source hostname from
  // the referrer. The referrer is used because it should be

3p/messaging.js

@@ -95,7 +95,7 @@ function startListening(win) {
  if (listeners[i].type != data['type']) {
  continue;
  }
- const cb = listeners[i].cb;
+ const {cb} = listeners[i];
  try {
  cb(data);
  } catch (e) {

ads/24smi.js

@@ -24,7 +24,7 @@ const jsnPrefix = 'https://jsn.24smi.net/';
  */
 export function _24smi(global, data) {
  validateData(data, ['src']);
- const src = data.src;
+ const {src} = data;
  validateSrcPrefix(jsnPrefix, src);
 
  createContainer(global, getBlockId(src));

ads/adform.js

@@ -33,9 +33,7 @@ const hosts = {
 export function adform(global, data) {
  validateData(data, [['src', 'bn', 'mid']]);
  global.Adform = {ampData: data};
- const src = data.src;
- const bn = data.bn;
- const mid = data.mid;
+ const {src, bn, mid} = data;
  let url;
 
  // Custom ad url using "data-src" attribute

ads/adplugg.js

@@ -46,11 +46,11 @@ export function adplugg(global,data) {
 
  // Get a handle on the AdPlugg SDK.
  global.AdPlugg = (global.AdPlugg || []);
- const AdPlugg = global.AdPlugg;
+ const {AdPlugg} = global;
 
  // Register event listeners (via async wrapper).
  AdPlugg.push(function() {
- const AdPlugg = global.AdPlugg;
+ const {AdPlugg} = global;
  // Register the renderStart event listener.
  AdPlugg.on(
  adTag,

ads/adventive.js

@@ -116,8 +116,8 @@ function callback(id, ns) { ns.addInstance(id); }
  * provided, otherwise false
  */
 function getUrl(context, data, ns) {
- const location = context.location,
- mode = ns.mode,
+ const {location} = context,
+ {mode} = ns,
  isDev = hasOwn(data, 'isDev'),
  sld_ = sld[!mode.dev],
  thld_ = thld[isDev && !mode.live],

ads/chargeads.js

@@ -21,7 +21,7 @@ import {validateSrcPrefix, writeScript} from '../3p/3p';
  * @param {!Object} data
  */
 export function chargeads(global, data) {
- const src = data.src;
+ const {src} = data;
  validateSrcPrefix('https://www.chargeplatform.com/', src);
  writeScript(global, src);
 }

ads/contentad.js

@@ -34,7 +34,7 @@ export function contentad(global, data) {
  window.document.body.appendChild(cadDiv);
 
  /* Pass Source URL */
- let sourceUrl = window.context.sourceUrl;
+ let {sourceUrl} = window.context;
  if (data.url) {
  const domain = data.url || window.atob(data.d);
  sourceUrl = sourceUrl.replace(parseUrl(sourceUrl).host, domain);

ads/flite.js

@@ -23,7 +23,7 @@ import {loadScript, validateData} from '../3p/3p';
 export function flite(global, data) {
  // TODO: check mandatory fields
  validateData(data, [], ['guid','mixins']);
- const guid = data.guid, o = global, e = encodeURIComponent, x = 0;
+ const {guid} = data, o = global, e = encodeURIComponent, x = 0;
  let r = '', dep = '';
  o.FLITE = o.FLITE || {};
  o.FLITE.config = o.FLITE.config || {};

ads/google/a4a/google-data-reporter.js

@@ -62,7 +62,7 @@ export const PROFILING_BRANCHES = {
  * @visibleForTesting
  */
 export function getLifecycleReporter(ampElement, slotId) {
- const win = ampElement.win;
+ const {win} = ampElement;
  randomlySelectUnsetExperiments(win, PROFILING_BRANCHES);
  if (isReportingEnabled(ampElement) &&
  (!!getExperimentBranch(win, DOUBLECLICK_A4A_EXPERIMENT_NAME) ||
@@ -132,7 +132,7 @@ export function setGoogleLifecycleVarsFromHeaders(headers, reporter) {
 }
 
 function setupPageLoadMetricsReporter_(ampElement) {
- const win = ampElement.win;
+ const {win} = ampElement;
  const correlator = getCorrelator(win);
  win.ampAnalyticsPageLoadMetricsConfig =
  win.ampAnalyticsPageLoadMetricsConfig ||

ads/google/a4a/test/test-performance.js

@@ -78,8 +78,7 @@ describe('GoogleAdLifecycleReporter', () => {
  sandbox = sinon.sandbox.create();
  emitPingSpy = sandbox.spy(GoogleAdLifecycleReporter.prototype, 'emitPing_');
  iframe = createIframePromise(false).then(iframeFixture => {
- const win = iframeFixture.win;
- const doc = iframeFixture.doc;
+ const {win, doc} = iframeFixture;
  const viewer = Services.viewerForDoc(doc);
  const elem = doc.createElement('div');
  doc.body.appendChild(elem);

ads/google/a4a/test/test-utils.js

@@ -48,7 +48,7 @@ import {installXhrService} from '../../../../src/service/xhr-impl';
 function setupForAdTesting(fixture) {
  installDocService(fixture.win, /* isSingleDoc */ true);
  installExtensionsService(fixture.win);
- const doc = fixture.doc;
+ const {doc} = fixture;
  // TODO(a4a-cam@): This is necessary in the short term, until A4A is
  // smarter about host document styling. The issue is that it needs to
  // inherit the AMP runtime style element in order for shadow DOM-enclosed
@@ -247,7 +247,7 @@ describe('Google A4A utils', () => {
  this.timeout(5000);
  return createIframePromise().then(fixture => {
  setupForAdTesting(fixture);
- const doc = fixture.doc;
+ const {doc} = fixture;
  doc.win = window;
  const elem = createElementWithAttributes(doc, 'amp-a4a', {
  'type': 'adsense',
@@ -268,7 +268,7 @@ describe('Google A4A utils', () => {
  it('should specify that this is canary', () => {
  return createIframePromise().then(fixture => {
  setupForAdTesting(fixture);
- const doc = fixture.doc;
+ const {doc} = fixture;
  doc.win = window;
  const elem = createElementWithAttributes(doc, 'amp-a4a', {
  'type': 'adsense',
@@ -288,7 +288,7 @@ describe('Google A4A utils', () => {
  it('should specify that this is control', () => {
  return createIframePromise().then(fixture => {
  setupForAdTesting(fixture);
- const doc = fixture.doc;
+ const {doc} = fixture;
  doc.win = window;
  const elem = createElementWithAttributes(doc, 'amp-a4a', {
  'type': 'adsense',
@@ -308,7 +308,7 @@ describe('Google A4A utils', () => {
  it('should not have `art` parameter when AMP_CONFIG is undefined', () => {
  return createIframePromise().then(fixture => {
  setupForAdTesting(fixture);
- const doc = fixture.doc;
+ const {doc} = fixture;
  doc.win = window;
  const elem = createElementWithAttributes(doc, 'amp-a4a', {
  'type': 'adsense',
@@ -327,7 +327,7 @@ describe('Google A4A utils', () => {
  it('should not have `art` parameter when binary type is production', () => {
  return createIframePromise().then(fixture => {
  setupForAdTesting(fixture);
- const doc = fixture.doc;
+ const {doc} = fixture;
  doc.win = window;
  const elem = createElementWithAttributes(doc, 'amp-a4a', {
  'type': 'adsense',
@@ -348,7 +348,7 @@ describe('Google A4A utils', () => {
  it('should include scroll position', function() {
  return createIframePromise().then(fixture => {
  setupForAdTesting(fixture);
- const doc = fixture.doc;
+ const {doc} = fixture;
  doc.win = window;
  const elem = createElementWithAttributes(doc, 'amp-a4a', {
  'type': 'adsense',
@@ -376,7 +376,7 @@ describe('Google A4A utils', () => {
  this.timeout(5000);
  return createIframePromise().then(fixture => {
  setupForAdTesting(fixture);
- const doc = fixture.doc;
+ const {doc} = fixture;
  doc.win = window;
  const elem = createElementWithAttributes(doc, 'amp-a4a', {
  'type': 'adsense',
@@ -397,7 +397,7 @@ describe('Google A4A utils', () => {
  it('should include debug_experiment_id if local mode w/ deid hash', () => {
  return createIframePromise().then(fixture => {
  setupForAdTesting(fixture);
- const doc = fixture.doc;
+ const {doc} = fixture;
  doc.win = fixture.win;
  const elem = createElementWithAttributes(doc, 'amp-a4a', {
  'type': 'adsense',
@@ -419,7 +419,7 @@ describe('Google A4A utils', () => {
  it('should include GA cid/hid', () => {
  return createIframePromise().then(fixture => {
  setupForAdTesting(fixture);
- const doc = fixture.doc;
+ const {doc} = fixture;
  doc.win = fixture.win;
  const elem = createElementWithAttributes(doc, 'amp-a4a', {
  'type': 'adsense',
@@ -441,7 +441,7 @@ describe('Google A4A utils', () => {
  it('should have correct bc value when everything supported', () => {
  return createIframePromise().then(fixture => {
  setupForAdTesting(fixture);
- const doc = fixture.doc;
+ const {doc} = fixture;
  doc.win = fixture.win;
  const elem = createElementWithAttributes(doc, 'amp-a4a', {
  'type': 'adsense',
@@ -467,7 +467,7 @@ describe('Google A4A utils', () => {
  it('should have correct bc value when sandbox not supported', () => {
  return createIframePromise().then(fixture => {
  setupForAdTesting(fixture);
- const doc = fixture.doc;
+ const {doc} = fixture;
  doc.win = fixture.win;
  const elem = createElementWithAttributes(doc, 'amp-a4a', {
  'type': 'adsense',
@@ -491,7 +491,7 @@ describe('Google A4A utils', () => {
  it('should not include bc when nothing supported', () => {
  return createIframePromise().then(fixture => {
  setupForAdTesting(fixture);
- const doc = fixture.doc;
+ const {doc} = fixture;
  doc.win = fixture.win;
  const elem = createElementWithAttributes(doc, 'amp-a4a', {
  'type': 'adsense',

ads/google/a4a/utils.js

@@ -148,7 +148,7 @@ export function isReportingEnabled(ampElement) {
  // If any of those fail, we use the `BaseLifecycleReporter`, which is a
  // a no-op (sends no pings).
  const type = ampElement.element.getAttribute('type');
- const win = ampElement.win;
+ const {win} = ampElement;
  const experimentName = 'a4aProfilingRate';
  // In local dev mode, neither the canary nor prod config files is available,
  // so manually set the profiling rate, for testing/dev.
@@ -168,8 +168,7 @@ export function isReportingEnabled(ampElement) {
  * @return {!Object<string,null|number|string>} block level parameters
  */
 export function googleBlockParameters(a4a, opt_experimentIds) {
- const adElement = a4a.element;
- const win = a4a.win;
+ const {element: adElement, win} = a4a;
  const slotRect = a4a.getPageLayoutBox();
  const iframeDepth = iframeNestingDepth(win);
  const enclosingContainers = getEnclosingContainerTypes(adElement);
@@ -249,7 +248,7 @@ export function googlePageParameters(win, nodeOrDoc, startTime) {
  // Read by GPT for GA/GPT integration.
  win.gaGlobal = win.gaGlobal ||
  {cid: clientId, hid: documentInfo.pageViewId};
- const screen = win.screen;
+ const {screen} = win;
  const viewport = Services.viewportForDoc(nodeOrDoc);
  const viewportRect = viewport.getRect();
  const viewportSize = viewport.getSize();
@@ -357,9 +356,9 @@ function getHistoryLength(win) {
  * @return {?string}
  */
 function topWindowUrlOrDomain(win) {
- const ancestorOrigins = win.location.ancestorOrigins;
+ const {ancestorOrigins} = win.location;
  if (ancestorOrigins) {
- const origin = win.location.origin;
+ const {origin} = win.location;
  const topOrigin = ancestorOrigins[ancestorOrigins.length - 1];
  if (origin == topOrigin) {
  return win.top.location.hostname;

ads/google/ima-player-data.js

@@ -40,8 +40,8 @@ export class ImaPlayerData {
  this.duration = videoPlayer.duration;
 
  // Adapt videoPlayer.played for the playedRanges format AMP wants.
- const played = videoPlayer.played;
- const length = played.length;
+ const {played} = videoPlayer;
+ const {length} = played;
  this.playedRanges = [];
  for (let i = 0; i < length; i++) {
  this.playedRanges.push([played.start(i), played.end(i)]);

ads/gumgum.js

@@ -33,7 +33,7 @@ export function gumgum(global, data) {
  ggevents = global.ggevents || [];
 
  const
- max = Math.max,
+ {max} = Math,
  slotId = parseInt(data.slot, 10),
  onLoad = function(type) {
  return function(evt) {

ads/ibillboard.js

@@ -31,7 +31,7 @@ const validHosts = [
 export function ibillboard(global, data) {
 
  validateData(data, ['src']);
- const src = data.src;
+ const {src} = data;
  validateSrcPrefix(validHosts, src);
 
  writeScript(global, src);

ads/imonomy.js

@@ -87,8 +87,7 @@ function reportStats(data, code) {
  if (typeof window.context.location.href !== 'undefined') {
  pageLocation = encodeURIComponent(window.context.location.href);
  }
- const subId = data.subId,
- pid = data.pid,
+ const {subId, pid} = data,
  trackId = 'AMP',
  notFirst = true,
  cid = '',

ads/inabox/position-observer.js

@@ -96,8 +96,8 @@ export class PositionObserver {
  * A method to get viewport rect
  */
  getViewportRect() {
- const scrollingElement = this.scrollingElement_;
- const win = this.win_;
+ const {scrollingElement_: scrollingElement, win_: win} = this;
+
  const scrollLeft = scrollingElement./*OK*/scrollLeft ||
  win./*OK*/pageXOffset;
  const scrollTop = scrollingElement./*OK*/scrollTop ||