Skip to content

Commit

Permalink
amp-bind: Support data-amp-bind-* attributes (#15408)
Browse files Browse the repository at this point in the history 
* Support data-amp-bind-*, fix random lint and type errors.

* Ignore data-amp-bind-foo if [foo] already exists, add tests.
  • Loading branch information
William Chou committed May 19, 2018
1 parent c014eb2 commit e7c4353
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 60 deletions.
116 changes: 56 additions & 60 deletions extensions/amp-bind/0.1/bind-impl.js
View file
Expand Up @@ -20,6 +20,7 @@ import {BindExpressionResultDef} from './bind-expression';
import {BindValidator} from './bind-validator';
import {BindingDef} from './bind-evaluator';
import {ChunkPriority, chunk} from '../../../src/chunk';
import {Deferred} from '../../../src/utils/promise';
import {RAW_OBJECT_ARGS_KEY} from '../../../src/action-constants';
import {Services} from '../../../src/services';
import {debounce} from '../../../src/utils/rate-limit';
Expand All @@ -36,6 +37,7 @@ import {map} from '../../../src/utils/object';
import {parseJson, recursiveEquals} from '../../../src/json';
import {reportError} from '../../../src/error';
import {rewriteAttributesForElement} from '../../../src/sanitizer';
import {startsWith} from '../../../src/string';

const TAG = 'amp-bind';

Expand All @@ -55,23 +57,21 @@ const MAX_MERGE_DEPTH = 10;
/**
* A bound property, e.g. [property]="expression".
* `previousResult` is the result of this expression during the last evaluation.
* @typedef {{
* property: string,
* expressionString: string,
* previousResult: (./bind-expression.BindExpressionResultDef|undefined),
* }}
* @typedef {{property: string, expressionString: string, previousResult: (./bind-expression.BindExpressionResultDef|undefined)}}
*/
let BoundPropertyDef;

/**
* A tuple containing a single element and all of its bound properties.
* @typedef {{
* boundProperties: !Array<BoundPropertyDef>,
* element: !Element,
* }}
* @typedef {{boundProperties: !Array<BoundPropertyDef>, element: !Element}}
*/
let BoundElementDef;

/**
* @typedef {{boundElements: !Array<BoundElementDef>, bindings: !Array<./bind-evaluator.BindingDef>, expressionToElements: !Object<string, !Array<!Element>>, limitExceeded: boolean}}
*/
let NodeScanDef;

/**
* A map of tag names to arrays of attributes that do not have non-bind
* counterparts. For instance, amp-carousel allows a `[slide]` attribute,
Expand Down Expand Up @@ -547,7 +547,6 @@ export class Bind {
* @param {!Array<!Node>} nodes
* @return {!Promise}
* @private
* @visibleForTesting
*/
removeBindingsForNodes_(nodes) {
const before = (getMode().development) ? this.numberOfBindings() : 0;
Expand Down Expand Up @@ -600,14 +599,7 @@ export class Bind {
* a tuple containing bound elements and binding data for the evaluator.
* @param {!Node} node
* @param {number} limit
* @return {
* !Promise<{
* boundElements: !Array<BoundElementDef>,
* bindings: !Array<./bind-evaluator.BindingDef>,
* expressionToElements: !Object<string, !Array<!Element>>,
* limitExceeded: boolean,
* }>
* }
* @return {!Promise<NodeScanDef>}
* @private
*/
scanNode_(node, limit) {
Expand Down Expand Up @@ -657,34 +649,34 @@ export class Bind {
return !walker.nextNode() || limitExceeded;
};

return new Promise(resolve => {
const chunktion = idleDeadline => {
let completed = false;
// If `requestIdleCallback` is available, scan elements until
// idle time runs out.
if (idleDeadline && !idleDeadline.didTimeout) {
while (idleDeadline.timeRemaining() > 1 && !completed) {
completed = scanNextNode_();
}
} else {
// If `requestIdleCallback` isn't available, scan elements in buckets.
// Bucket size is a magic number that fits within a single frame.
const bucketSize = 250;
for (let i = 0; i < bucketSize && !completed; i++) {
completed = scanNextNode_();
}
const {promise, resolve} = new Deferred();
const chunktion = idleDeadline => {
let completed = false;
// If `requestIdleCallback` is available, scan elements until
// idle time runs out.
if (idleDeadline && !idleDeadline.didTimeout) {
while (idleDeadline.timeRemaining() > 1 && !completed) {
completed = scanNextNode_();
}
// If we scanned all elements, resolve. Otherwise, continue chunking.
if (completed) {
resolve({
boundElements, bindings, expressionToElements, limitExceeded,
});
} else {
chunk(this.ampdoc, chunktion, ChunkPriority.LOW);
} else {
// If `requestIdleCallback` isn't available, scan elements in buckets.
// Bucket size is a magic number that fits within a single frame.
const bucketSize = 250;
for (let i = 0; i < bucketSize && !completed; i++) {
completed = scanNextNode_();
}
};
chunk(this.ampdoc, chunktion, ChunkPriority.LOW);
});
}
// If we scanned all elements, resolve. Otherwise, continue chunking.
if (completed) {
resolve({
boundElements, bindings, expressionToElements, limitExceeded,
});
} else {
chunk(this.ampdoc, chunktion, ChunkPriority.LOW);
}
};
chunk(this.ampdoc, chunktion, ChunkPriority.LOW);
return promise;
}

/**
Expand Down Expand Up @@ -715,15 +707,26 @@ export class Bind {
* @private
*/
scanAttribute_(attribute, element) {
const {tagName} = element;
const {name} = attribute;
if (name.length > 2 && name[0] === '[' && name[name.length - 1] === ']') {
const property = name.substr(1, name.length - 2);
if (this.validator_.canBind(tagName, property)) {
const tag = element.tagName;
const attr = attribute.name;

let property;
if (attr.length > 2 && attr[0] === '[' && attr[attr.length - 1] === ']') {
property = attr.substr(1, attr.length - 2);
} else if (startsWith(attr, 'data-amp-bind-')) {
property = attr.substr(14);
// Ignore `data-amp-bind-foo` if `[foo]` already exists.
if (element.hasAttribute(`[${property}]`)) {
property = null;
}
}

if (property) {
if (this.validator_.canBind(tag, property)) {
return {property, expressionString: attribute.value};
} else {
const err = user().createError(
`${TAG}: Binding to [${property}] on <${tagName}> is not allowed.`);
`${TAG}: Binding to [${property}] on <${tag}> is not allowed.`);
reportError(err, element);
}
}
Expand Down Expand Up @@ -754,9 +757,7 @@ export class Bind {

/**
* Reevaluates all expressions and returns a map of expressions to results.
* @return {!Promise<
* !Object<string, ./bind-expression.BindExpressionResultDef>
* >}
* @return {!Promise<!Object<string, ./bind-expression.BindExpressionResultDef>>}
* @private
*/
evaluate_() {
Expand Down Expand Up @@ -805,12 +806,7 @@ export class Bind {
* new value.
* @param {!Array<!BoundPropertyDef>} boundProperties
* @param {Object<string, ./bind-expression.BindExpressionResultDef>} results
* @return {
* !Array<{
* boundProperty: !BoundPropertyDef,
* newValue: !./bind-expression.BindExpressionResultDef,
* }>
* }
* @return {!Array<{boundProperty: !BoundPropertyDef, newValue: !./bind-expression.BindExpressionResultDef}>}
* @private
*/
calculateUpdates_(boundProperties, results) {
Expand Down Expand Up @@ -928,7 +924,7 @@ export class Bind {
* @param {!BoundPropertyDef} boundProperty
* @param {!Element} element
* @param {./bind-expression.BindExpressionResultDef} newValue
* @return (?{name: string, value:./bind-expression.BindExpressionResultDef})
* @return {?{name: string, value:./bind-expression.BindExpressionResultDef}}
* @private
*/
applyBinding_(boundProperty, element, newValue) {
Expand Down
21 changes: 21 additions & 0 deletions extensions/amp-bind/0.1/test/integration/test-bind-impl.js
View file
Expand Up @@ -275,6 +275,27 @@ describe.configure().ifNewChrome().run('Bind', function() {
});
});

it('should support data-amp-bind-* syntax', () => {
const element = createElement(env, container, 'data-amp-bind-text="1+1"');
expect(bind.numberOfBindings()).to.equal(0);
expect(element.textContent).to.equal('');
return onBindReadyAndSetState(env, bind, {}).then(() => {
expect(bind.numberOfBindings()).to.equal(1);
expect(element.textContent).to.equal('2');
});
});

it('should prefer [foo] over data-amp-bind-foo', () => {
const element = createElement(
env, container, '[text]="1+1" data-amp-bind-text="2+2"');
expect(bind.numberOfBindings()).to.equal(0);
expect(element.textContent).to.equal('');
return onBindReadyAndSetState(env, bind, {}).then(() => {
expect(bind.numberOfBindings()).to.equal(1);
expect(element.textContent).to.equal('2');
});
});

it('should call createTreeWalker() with all params', () => {
const spy = env.sandbox.spy(env.win.document, 'createTreeWalker');
createElement(env, container, '[text]="1+1"');
Expand Down

0 comments on commit e7c4353

Please sign in to comment.