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

♻️ Use tryResolve instead of Promise.resolve with call #15143

Merged
merged 4 commits into from
May 8, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions build-system/eslint-rules/resolve-inside-promise-resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ module.exports = function(context) {
}

context.report(node, 'Use the Promise constructor, ' +
'or tryResolve in the src/promise.js module.');
'or tryResolve in the src/utils/promise.js module.');
},

// new Promise(...)
Expand Down Expand Up @@ -107,7 +107,7 @@ module.exports = function(context) {
const message = [
'Must call the resolve param.',
'If you are creating a pending promise to extract a resolve function',
'please use the Deferred class in the src/promise.js module instead.',
'please use Deferred in the src/utils/promise.js module instead.',
].join('\n\t');
context.report(resolver, message);
},
Expand Down
2 changes: 1 addition & 1 deletion build-system/tasks/bundle-size.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const gulp = require('gulp-help')(require('gulp'));
const log = require('fancy-log');

const runtimeFile = './dist/v0.js';
const maxSize = '77.26KB';
const maxSize = '77.27KB';

const green = colors.green;
const red = colors.red;
Expand Down
3 changes: 2 additions & 1 deletion extensions/amp-a4a/0.1/amp-a4a.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import {getBinaryType} from '../../../src/experiments';
import {getBinaryTypeNumericalCode} from '../../../ads/google/a4a/utils';
import {getContextMetadata} from '../../../src/iframe-attributes';
import {getMode} from '../../../src/mode';
import {tryResolve} from '../../../src/utils/promise';
// TODO(tdrl): Temporary. Remove when we migrate to using amp-analytics.
import {getTimingDataAsync} from '../../../src/service/variable-source';
import {insertAnalyticsElement} from '../../../src/extension-analytics';
Expand Down Expand Up @@ -1548,7 +1549,7 @@ export class AmpA4A extends AMP.BaseElement {
'releaseType': this.releaseType_,
});
const checkStillCurrent = this.verifyStillCurrent();
return Promise.resolve(utf8Decode(creativeBody)).then(creative => {
return tryResolve(() => utf8Decode(creativeBody)).then(creative => {
checkStillCurrent();
let srcPath;
let name = '';
Expand Down
3 changes: 2 additions & 1 deletion extensions/amp-access/0.1/amp-access-other.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ export class AccessOtherAdapter {
dev().fine(TAG, 'Use the authorization fallback for type=other');
// Disallow authorization for proxy origin (`cdn.ampproject.org`).
dev().assert(!this.isProxyOrigin_, 'Cannot authorize for proxy origin');
return Promise.resolve(dev().assert(this.authorizationResponse_));
const response = dev().assert(this.authorizationResponse_);
return Promise.resolve(response);
}

/** @override */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {AmpAdTemplates} from '../../amp-a4a/0.1/amp-ad-templates';
import {dev} from '../../../src/log';
import {getMode} from '../../../src/mode';
import {tryParseJson} from '../../../src/json';
import {tryResolve} from '../../../src/utils/promise';
import {utf8Decode, utf8Encode} from '../../../src/utils/bytes';

/** @type {string} */
Expand Down Expand Up @@ -109,7 +110,7 @@ export class AmpAdNetworkAdzerkImpl extends AmpA4A {
}
// Shorthand for: reject promise if current promise chain is out of date.
const checkStillCurrent = this.verifyStillCurrent();
return Promise.resolve(utf8Decode(bytes)).then(body => {
return tryResolve(() => utf8Decode(bytes)).then(body => {
checkStillCurrent();
this.ampCreativeJson_ = /** @type {!AmpTemplateCreativeDef} */
(tryParseJson(body) || {});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ import {
import {setStyles} from '../../../src/style';
import {stringHash32} from '../../../src/string';
import {tryParseJson} from '../../../src/json';
import {tryResolve} from '../../../src/utils/promise';
import {utf8Encode} from '../../../src/utils/bytes';

/** @type {string} */
Expand Down Expand Up @@ -1230,7 +1231,7 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
/** @type {?../../../src/service/xhr-impl.FetchResponse} */
({
headers,
arrayBuffer: () => Promise.resolve(utf8Encode(creative)),
arrayBuffer: () => tryResolve(() => utf8Encode(creative)),
});
// Pop head off of the array of resolvers as the response
// should match the order of blocks declared in the ad url.
Expand Down
3 changes: 2 additions & 1 deletion extensions/amp-analytics/0.1/analytics-root.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {dev, user} from '../../../src/log';
import {getMode} from '../../../src/mode';
import {layoutRectLtwh} from '../../../src/layout-rect';
import {map} from '../../../src/utils/object';
import {tryResolve} from '../../../src/utils/promise';
import {whenContentIniLoad} from '../../../src/friendly-iframe-embed';

const TAG = 'amp-analytics';
Expand Down Expand Up @@ -196,7 +197,7 @@ export class AnalyticsRoot {
// Special case selectors. The selection method is irrelavant.
// And no need to wait for document ready.
if (selector == ':root') {
return Promise.resolve(this.getRootElement());
return tryResolve(() => this.getRootElement());
}
if (selector == ':host') {
return new Promise(resolve => {
Expand Down
3 changes: 2 additions & 1 deletion extensions/amp-auto-ads/0.1/ad-strategy.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import {DataAttributeDef, PlacementState} from './placement';
import {tryResolve} from '../../../src/utils/promise';
import {user} from '../../../src/log';

/** @const */
Expand Down Expand Up @@ -55,7 +56,7 @@ export class AdStrategy {
*/
run() {
if (this.adTracker_.isMaxAdCountReached()) {
return Promise.resolve(this.getStrategyResult_());
return tryResolve(() => this.getStrategyResult_());
}
return this.placeNextAd_().then(success => {
if (success) {
Expand Down
3 changes: 2 additions & 1 deletion extensions/amp-pinterest/0.1/follow-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import {Util} from './util';
import {assertHttpsUrl} from '../../../src/url';
import {openWindowDialog} from '../../../src/dom';
import {tryResolve} from '../../../src/utils/promise';

import {user} from '../../../src/log';

Expand Down Expand Up @@ -81,6 +82,6 @@ export class FollowButton {
}
this.href += `pins/follow/?guid=${Util.guid}`;

return Promise.resolve(this.renderTemplate());
return tryResolve(() => this.renderTemplate());
}
}
3 changes: 2 additions & 1 deletion extensions/amp-share-tracking/0.1/amp-share-tracking.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {dict} from '../../../src/utils/object';
import {getCryptoRandomBytesArray} from '../../../src/utils/bytes';
import {isExperimentOn} from '../../../src/experiments';
import {registerServiceBuilder} from '../../../src/service';
import {tryResolve} from '../../../src/utils/promise';

/** @private @const {string} */
const TAG = 'amp-share-tracking';
Expand Down Expand Up @@ -125,7 +126,7 @@ export class AmpShareTracking extends AMP.BaseElement {
if (this.vendorHref_) {
return this.getOutgoingFragmentFromVendor_(this.vendorHref_);
}
return Promise.resolve(base64UrlEncodeFromBytes(
return tryResolve(() => base64UrlEncodeFromBytes(
this.getShareTrackingRandomBytes_()));
}

Expand Down
3 changes: 2 additions & 1 deletion extensions/amp-story/0.1/logging.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
import {LogLevel, dev} from '../../../src/log';
import {scopedQuerySelectorAll} from '../../../src/dom';
import {tryResolve} from '../../../src/utils/promise';


/** @typedef {function(!Element): (boolean|!Promise<boolean>)} */
Expand Down Expand Up @@ -173,7 +174,7 @@ function getLogType(logTypeKey) {
function getLogEntry(rootElement, logType, element) {
const predicate = logType.predicate || (unusedEl => false);

return Promise.resolve(predicate(element))
return tryResolve(() => predicate(element))
.then(conforms => {
return new Promise(resolve => {
resolve({
Expand Down
3 changes: 2 additions & 1 deletion extensions/amp-story/0.1/media-tasks.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import {Sources} from './sources';
import {isConnectedNode} from '../../../src/dom';
import {tryResolve} from '../../../src/utils/promise';



Expand Down Expand Up @@ -231,7 +232,7 @@ export class PlayTask extends MediaTask {
// The play() invocation is wrapped in a Promise.resolve(...) due to the
// fact that some browsers return a promise from media elements' play()
// function, while others return a boolean.
return Promise.resolve(mediaEl.play());
return tryResolve(() => mediaEl.play());
}
}

Expand Down
3 changes: 2 additions & 1 deletion extensions/amp-story/0.1/page-scaling.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {isExperimentOn} from '../../../src/experiments';
import {px, setImportantStyles, setStyle} from '../../../src/style';
import {throttle} from '../../../src/utils/rate-limit';
import {toArray, toWin} from '../../../src/types';
import {tryResolve} from '../../../src/utils/promise';
import {unscaledClientRect} from './utils';


Expand Down Expand Up @@ -234,7 +235,7 @@ export class PageScalingService {
* @return {!Promise}
*/
scale(page) {
return Promise.resolve(this.scale_(page));
return tryResolve(() => this.scale_(page));
}

/**
Expand Down
3 changes: 2 additions & 1 deletion extensions/amp-story/1.0/logging.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
import {LogLevel, dev} from '../../../src/log';
import {scopedQuerySelectorAll} from '../../../src/dom';
import {tryResolve} from '../../../src/utils/promise';


/** @typedef {function(!Element): (boolean|!Promise<boolean>)} */
Expand Down Expand Up @@ -173,7 +174,7 @@ function getLogType(logTypeKey) {
function getLogEntry(rootElement, logType, element) {
const predicate = logType.predicate || (unusedEl => false);

return Promise.resolve(predicate(element))
return tryResolve(() => predicate(element))
.then(conforms => {
return new Promise(resolve => {
resolve({
Expand Down
3 changes: 2 additions & 1 deletion extensions/amp-story/1.0/media-tasks.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import {Sources} from './sources';
import {isConnectedNode} from '../../../src/dom';
import {tryResolve} from '../../../src/utils/promise';



Expand Down Expand Up @@ -231,7 +232,7 @@ export class PlayTask extends MediaTask {
// The play() invocation is wrapped in a Promise.resolve(...) due to the
// fact that some browsers return a promise from media elements' play()
// function, while others return a boolean.
return Promise.resolve(mediaEl.play());
return tryResolve(() => mediaEl.play());
}
}

Expand Down
3 changes: 2 additions & 1 deletion extensions/amp-story/1.0/page-scaling.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {isExperimentOn} from '../../../src/experiments';
import {px, setImportantStyles, setStyle} from '../../../src/style';
import {throttle} from '../../../src/utils/rate-limit';
import {toArray, toWin} from '../../../src/types';
import {tryResolve} from '../../../src/utils/promise';
import {unscaledClientRect} from './utils';


Expand Down Expand Up @@ -234,7 +235,7 @@ export class PageScalingService {
* @return {!Promise}
*/
scale(page) {
return Promise.resolve(this.scale_(page));
return tryResolve(() => this.scale_(page));
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/animation.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import {Deferred} from './promise';
import {Deferred} from './utils/promise';
import {Services} from './services';
import {dev} from './log';
import {getCurve} from './curve';
Expand Down
59 changes: 0 additions & 59 deletions src/promise.js

This file was deleted.

4 changes: 3 additions & 1 deletion src/service/cid-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
} from '../url';
import {isIframed} from '../dom';
import {parseJson, tryParseJson} from '../json';
import {tryResolve} from '../utils/promise';

const ONE_DAY_MILLIS = 24 * 3600 * 1000;

Expand Down Expand Up @@ -609,7 +610,8 @@ function getNewCidForCookie(win) {
} else {
// If our entropy is a pure random number, we can just directly turn it
// into base 64
return Promise.resolve(base64UrlEncodeFromBytes(entropy)
const cast = /** @type {!Uint8Array} */(entropy);
return tryResolve(() => base64UrlEncodeFromBytes(cast)
// Remove trailing padding
.replace(/\.+$/, ''));
}
Expand Down
3 changes: 2 additions & 1 deletion src/service/resources-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {isBlockedByConsent, reportError} from '../error';
import {isExperimentOn} from '../experiments';
import {loadPromise} from '../event-helper';
import {registerServiceBuilderForDoc} from '../service';
import {tryResolve} from '../utils/promise';

const TAG_ = 'Resources';
const READY_SCAN_SIGNAL_ = 'ready-scan';
Expand Down Expand Up @@ -465,7 +466,7 @@ export class Resources {
*/
ensuredMeasured_(resource) {
if (resource.hasBeenMeasured()) {
return Promise.resolve(resource.getPageLayoutBox());
return tryResolve(() => resource.getPageLayoutBox());
}
return this.vsync_.measurePromise(() => {
resource.measure();
Expand Down
2 changes: 1 addition & 1 deletion src/service/url-expander/expander.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import {rethrowAsync, user} from '../../log';
import {tryResolve} from '../../promise';
import {tryResolve} from '../../utils/promise';

export const PARSER_IGNORE_FLAG = '`';

Expand Down
3 changes: 2 additions & 1 deletion src/service/viewer-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
import {isIframed} from '../dom';
import {registerServiceBuilderForDoc} from '../service';
import {reportError} from '../error';
import {tryResolve} from '../utils/promise';

const TAG_ = 'Viewer';
const SENTINEL_ = '__AMP__';
Expand Down Expand Up @@ -1040,7 +1041,7 @@ export class Viewer {
// "Thenables". Convert from these values into trusted Promise instances,
// assimilating with the resolved (or rejected) internal value.
return /** @type {!Promise<?JsonObject|string|undefined>} */ (
Promise.resolve(this.messageDeliverer_(
tryResolve(() => this.messageDeliverer_(
eventType,
/** @type {?JsonObject|string|undefined} */ (data),
awaitResponse)));
Expand Down
4 changes: 2 additions & 2 deletions src/service/xhr-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -761,8 +761,8 @@ export class FetchResponse {
user().assert(this.xhr_.responseXML,
'responseXML should exist. Make sure to return ' +
'Content-Type: text/html header.');
return /** @type {!Promise<!Document>} */ (
Promise.resolve(dev().assert(this.xhr_.responseXML)));
const doc = /** @type {!Document} */(dev().assert(this.xhr_.responseXML));
return Promise.resolve(doc);
}

/**
Expand Down
Loading