From 3f24f7175c0e90d2092714b0b8c6bee85dbad0e7 Mon Sep 17 00:00:00 2001 From: Josh <34038781+josh313@users.noreply.github.com> Date: Wed, 24 Jan 2018 12:43:16 -0800 Subject: [PATCH] Implement support for the amp-bind premutate message (#12906) * Implement support for the amp-bind premutate message * fix unit tests * Fix lint * Revert changes to onMessage and instead add a new onMessageRespond method * remove eventType and awaitResponse from RequestResponder parameters --- extensions/amp-bind/0.1/amp-state.js | 6 +++ extensions/amp-bind/0.1/bind-impl.js | 37 +++++++++++++++++++ .../0.1/test/integration/test-bind-impl.js | 30 +++++++++++++++ .../amp-bind/validator-amp-bind.protoascii | 3 ++ src/service/viewer-impl.js | 29 +++++++++++++++ 5 files changed, 105 insertions(+) diff --git a/extensions/amp-bind/0.1/amp-state.js b/extensions/amp-bind/0.1/amp-state.js index 32617f74eb67..0c2553547cd0 100644 --- a/extensions/amp-bind/0.1/amp-state.js +++ b/extensions/amp-bind/0.1/amp-state.js @@ -80,6 +80,12 @@ export class AmpState extends AMP.BaseElement { /** @private */ initialize_() { + if (this.element.hasAttribute('overridable')) { + Services.bindForDocOrNull(this.element).then(bind => { + dev().assert(bind, 'Bind service can not be found.'); + bind.makeStateKeyOverridable(this.element.getAttribute('id')); + }); + } // Parse child script tag and/or fetch JSON from endpoint at `src` // attribute, with the latter taking priority. const children = this.element.children; diff --git a/extensions/amp-bind/0.1/bind-impl.js b/extensions/amp-bind/0.1/bind-impl.js index 87f91f77439e..55ad5f068c1f 100644 --- a/extensions/amp-bind/0.1/bind-impl.js +++ b/extensions/amp-bind/0.1/bind-impl.js @@ -124,6 +124,9 @@ export class Bind { /** @private {!../../../src/service/history-impl.History} */ this.history_ = Services.historyForDoc(ampdoc); + /** @private {!Array} */ + this.overridableKeys_ = []; + /** * Upper limit on number of bindings for performance. * @private {number} @@ -147,6 +150,7 @@ export class Bind { /** @const @private {!../../../src/service/viewer-impl.Viewer} */ this.viewer_ = Services.viewerForDoc(this.ampdoc); + this.viewer_.onMessageRespond('premutate', this.premutate_.bind(this)); const bodyPromise = (opt_win) ? waitForBodyPromise(opt_win.document) @@ -333,6 +337,39 @@ export class Bind { return this.history_; } + /** + * Calls setState(s), where s is data.state with the non-overridable keys removed. + * @param {*} data + * @return {!Promise} + * @private + */ + premutate_(data) { + const ignoredKeys = []; + return this.initializePromise_.then(() => { + Object.keys(data.state).forEach(key => { + if (!this.overridableKeys_.includes(key)) { + delete data.state[key]; + ignoredKeys.push(key); + } + }); + if (ignoredKeys.length > 0) { + user().warn(TAG, 'Some state keys could not be premutated ' + + 'because they are missing the overridable attribute: ' + + ignoredKeys.join(', ')); + } + return this.setState(data.state); + }); + } + + /** + * Marks the given key as overridable so that it can be overriden by + * a premutate message from the viewer. + * @param {string} key + */ + makeStateKeyOverridable(key) { + this.overridableKeys_.push(key); + } + /** * Scans the document for elements, and adds them to the * bind-evaluator. diff --git a/extensions/amp-bind/0.1/test/integration/test-bind-impl.js b/extensions/amp-bind/0.1/test/integration/test-bind-impl.js index 87f3c194d610..c44cb2cee3ae 100644 --- a/extensions/amp-bind/0.1/test/integration/test-bind-impl.js +++ b/extensions/amp-bind/0.1/test/integration/test-bind-impl.js @@ -23,6 +23,7 @@ import * as sinon from 'sinon'; import {AmpEvents} from '../../../../../src/amp-events'; import {Bind} from '../../bind-impl'; import {BindEvents} from '../../bind-events'; +import {Services} from '../../../../../src/services'; import {chunkInstanceForTesting} from '../../../../../src/chunk'; import {toArray} from '../../../../../src/types'; import {user} from '../../../../../src/log'; @@ -210,11 +211,13 @@ describe.configure().ifNewChrome().run('Bind', function() { }, env => { let bind; let container; + let viewer; beforeEach(() => { // Make sure we have a chunk instance for testing. chunkInstanceForTesting(env.ampdoc); + viewer = Services.viewerForDoc(env.ampdoc); bind = new Bind(env.ampdoc); // Connected
element created by describes.js. container = env.win.document.getElementById('parent'); @@ -583,5 +586,32 @@ describe.configure().ifNewChrome().run('Bind', function() { sinon.match(/Maximum number of bindings reached/)); }); }); + + it('should update premutate keys that are overridable', () => { + bind.makeStateKeyOverridable('foo'); + bind.makeStateKeyOverridable('bar'); + const foo = createElement(env, container, '[text]="foo"'); + const bar = createElement(env, container, '[text]="bar"'); + const baz = createElement(env, container, '[text]="baz"'); + const qux = createElement(env, container, '[text]="qux"'); + + return onBindReadyAndSetState(env, bind, { + foo: 1, bar: 2, baz: 3, qux: 4, + }).then(() => { + return viewer.receiveMessage('premutate', { + state: { + foo: 'foo', + bar: 'bar', + baz: 'baz', + qux: 'qux', + }, + }).then(() => { + expect(foo.textContent).to.equal('foo'); + expect(bar.textContent).to.equal('bar'); + expect(baz.textContent).to.be.equal('3'); + expect(qux.textContent).to.be.equal('4'); + }); + }); + }); }); // in single ampdoc }); diff --git a/extensions/amp-bind/validator-amp-bind.protoascii b/extensions/amp-bind/validator-amp-bind.protoascii index 849e6b6eb53d..5b3f1d77e766 100644 --- a/extensions/amp-bind/validator-amp-bind.protoascii +++ b/extensions/amp-bind/validator-amp-bind.protoascii @@ -63,6 +63,9 @@ tags: { # name: "id" mandatory: true } + attrs: { + name: "overridable" + } attrs: { name: "src" value_url: { diff --git a/src/service/viewer-impl.js b/src/service/viewer-impl.js index 809d521fa7e2..ac44fefb8c80 100644 --- a/src/service/viewer-impl.js +++ b/src/service/viewer-impl.js @@ -87,6 +87,12 @@ const TRUSTED_REFERRER_HOSTS = [ /^t.co$/, ]; +/** + * @typedef {function(*):(!Promise<*>|undefined)} + */ +export let RequestResponder; + + /** * An AMP representation of the Viewer. This class doesn't do any work itself * but instead delegates everything to the actual viewer. This class and the @@ -131,6 +137,9 @@ export class Viewer { /** @private {!Object>} */ this.messageObservables_ = map(); + /** @private {!Object} */ + this.messageResponders_ = map(); + /** @private {!Observable} */ this.runtimeOnObservable_ = new Observable(); @@ -803,6 +812,21 @@ export class Viewer { return observable.add(handler); } + /** + * Adds a eventType listener for viewer events. + * @param {string} eventType + * @param {!RequestResponder} responder + * @return {!UnlistenDef} + */ + onMessageRespond(eventType, responder) { + this.messageResponders_[eventType] = responder; + return () => { + if (this.messageResponders_[eventType] === responder) { + delete this.messageResponders_[eventType]; + } + }; + } + /** * Requests AMP document to receive a message from Viewer. * @param {string} eventType @@ -828,6 +852,11 @@ export class Viewer { const observable = this.messageObservables_[eventType]; if (observable) { observable.fire(data); + } + const responder = this.messageResponders_[eventType]; + if (responder) { + return responder(data); + } else if (observable) { return Promise.resolve(); } dev().fine(TAG_, 'unknown message:', eventType);