From 456a90b9af2528e109eb1357b18e4744b11ed578 Mon Sep 17 00:00:00 2001 From: William Chou Date: Fri, 2 Feb 2018 12:29:35 -0500 Subject: [PATCH] change amp-bind verification to user warning and clarify message --- extensions/amp-bind/0.1/bind-impl.js | 15 ++++++++------- .../0.1/test/integration/test-bind-impl.js | 14 +++++++------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/extensions/amp-bind/0.1/bind-impl.js b/extensions/amp-bind/0.1/bind-impl.js index 55ad5f068c1f0..a688e645a2551 100644 --- a/extensions/amp-bind/0.1/bind-impl.js +++ b/extensions/amp-bind/0.1/bind-impl.js @@ -962,11 +962,12 @@ export class Bind { * @private */ verifyBinding_(boundProperty, element, expectedValue) { - const property = boundProperty.property; + const {property, expressionString} = boundProperty; + const tagName = element.tagName; // Don't show a warning for bind-only attributes, // like 'slide' on amp-carousel. - const bindOnlyAttrs = BIND_ONLY_ATTRIBUTES[element.tagName]; + const bindOnlyAttrs = BIND_ONLY_ATTRIBUTES[tagName]; if (bindOnlyAttrs && bindOnlyAttrs.includes(property)) { return; } @@ -1023,11 +1024,11 @@ export class Bind { } if (!match) { - const err = user().createError(`${TAG}: ` + - `Default value for [${property}] does not match first expression ` + - `result (${expectedValue}). This can result in unexpected behavior ` + - 'after the next state change.'); - reportError(err, element); + user().warn(`${TAG}: ` + + `Default value for <${tagName} [${property}]="${expressionString}"> ` + + `does not match first result (${expectedValue}). We recommend ` + + 'writing expressions with matching default values, but this can be ' + + 'safely ignored if intentional.'); } } 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 c44cb2cee3ae6..78f5e871e837a 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 @@ -287,10 +287,10 @@ describe.configure().ifNewChrome().run('Bind', function() { createElement(env, container, '[class]="\'foo\'" class=" foo "'); createElement(env, container, '[class]="\'\'"'); createElement(env, container, '[class]="\'bar\'" class="qux"'); // Error - const errorSpy = env.sandbox.spy(user(), 'createError'); + const warnSpy = env.sandbox.spy(user(), 'warn'); return onBindReady(env, bind).then(() => { - expect(errorSpy).to.be.calledOnce; - expect(errorSpy).calledWithMatch(/bar/); + expect(warnSpy).to.be.calledOnce; + expect(warnSpy).calledWithMatch(/bar/); }); }); @@ -298,9 +298,9 @@ describe.configure().ifNewChrome().run('Bind', function() { window.AMP_MODE = {development: true, test: true}; // Only the initial value for [a] binding does not match. createElement(env, container, '[text]="\'a\'" [class]="\'b\'" class="b"'); - const errorSpy = env.sandbox.spy(user(), 'createError'); + const warnSpy = env.sandbox.spy(user(), 'warn'); return onBindReady(env, bind).then(() => { - expect(errorSpy).to.be.calledOnce; + expect(warnSpy).to.be.calledOnce; }); }); @@ -309,9 +309,9 @@ describe.configure().ifNewChrome().run('Bind', function() { createElement(env, container, '[disabled]="true" disabled', 'button'); createElement(env, container, '[disabled]="false"', 'button'); createElement(env, container, '[disabled]="true"', 'button'); // Mismatch. - const errorSpy = env.sandbox.spy(user(), 'createError'); + const warnSpy = env.sandbox.spy(user(), 'warn'); return onBindReady(env, bind).then(() => { - expect(errorSpy).to.be.calledOnce; + expect(warnSpy).to.be.calledOnce; }); });