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

🏗Prefer destructuring objects #15204

Merged
merged 16 commits into from
May 17, 2018
2 changes: 2 additions & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,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,
Expand Down
2 changes: 1 addition & 1 deletion 3p/3p.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};
Expand Down
4 changes: 2 additions & 2 deletions 3p/beopinion.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions 3p/environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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]);
Expand Down
2 changes: 1 addition & 1 deletion 3p/integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion 3p/messaging.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion ads/24smi.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
4 changes: 1 addition & 3 deletions ads/adform.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions ads/adplugg.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions ads/adventive.js
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
2 changes: 1 addition & 1 deletion ads/chargeads.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
2 changes: 1 addition & 1 deletion ads/contentad.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion ads/flite.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 || {};
Expand Down
4 changes: 2 additions & 2 deletions ads/google/a4a/google-data-reporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) ||
Expand Down Expand Up @@ -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 ||
Expand Down
3 changes: 1 addition & 2 deletions ads/google/a4a/test/test-performance.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
26 changes: 13 additions & 13 deletions ads/google/a4a/test/test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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',
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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',
Expand Down Expand Up @@ -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',
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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',
Expand Down
11 changes: 5 additions & 6 deletions ads/google/a4a/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions ads/google/ima-player-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)]);
Expand Down
2 changes: 1 addition & 1 deletion ads/gumgum.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion ads/ibillboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions ads/imonomy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '',
Expand Down
4 changes: 2 additions & 2 deletions ads/inabox/position-observer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand Down
Loading