Merge pull request #5625 from Polymer/legacyNoObservedAttributes-tele…

…metry Ensure telemetry system works with `legacyNoObservedAttributes` setting
Polymer · Jan 30, 2020 · c5c7eec · c5c7eec
2 parents 3e6ba3e + 5383f5f
commit c5c7eec
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 6 deletions.
diff --git a/lib/legacy/legacy-element-mixin.js b/lib/legacy/legacy-element-mixin.js
@@ -23,6 +23,7 @@ import { wrap } from '../utils/wrap.js';
 import { scopeSubtree } from '../utils/scope-subtree.js';
 import { legacyOptimizations, legacyNoObservedAttributes } from '../utils/settings.js';
 import { findObservedAttributesGetter } from '../mixins/disable-upgrade-mixin.js';
+import { register } from '../utils/telemetry.js';
 
 const DISABLED_ATTR = 'disable-upgrade';
 
@@ -103,6 +104,8 @@ export const LegacyElementMixin = dedupingMixin((base) => {
       this.__isUpgradeDisabled;
       /** @type {boolean|undefined} */
       this.__needsAttributesAtConnected;
+      /** @type {boolean|undefined} */
+      this._legacyForceObservedAttributes;
     }
 
     /**
@@ -142,7 +145,7 @@ export const LegacyElementMixin = dedupingMixin((base) => {
 
     /** @override */
     setAttribute(name, value) {
-      if (legacyNoObservedAttributes) {
+      if (legacyNoObservedAttributes && !this._legacyForceObservedAttributes) {
         const oldValue = this.getAttribute(name);
         super.setAttribute(name, value);
         // value coerced to String for closure's benefit
@@ -154,7 +157,7 @@ export const LegacyElementMixin = dedupingMixin((base) => {
 
     /** @override */
     removeAttribute(name) {
-      if (legacyNoObservedAttributes) {
+      if (legacyNoObservedAttributes && !this._legacyForceObservedAttributes) {
         const oldValue = this.getAttribute(name);
         super.removeAttribute(name);
         this.__attributeReaction(name, oldValue, null);
@@ -165,8 +168,16 @@ export const LegacyElementMixin = dedupingMixin((base) => {
 
     // NOTE: Inlined for perf from version of DisableUpgradeMixin.
     static get observedAttributes() {
-      return legacyNoObservedAttributes ? [] :
-        observedAttributesGetter.call(this).concat(DISABLED_ATTR);
+      if (legacyNoObservedAttributes && !this.prototype._legacyForceObservedAttributes) {
+        // Ensure this element is property registered with the telemetry system.
+        if (!this.hasOwnProperty(JSCompiler_renameProperty('__observedAttributes', this))) {
+          this.__observedAttributes = [];
+          register(this.prototype);
+        }
+        return this.__observedAttributes;
+      } else {
+        return observedAttributesGetter.call(this).concat(DISABLED_ATTR);
+      }
     }
 
     // NOTE: Inlined for perf from version of DisableUpgradeMixin.
@@ -306,7 +317,7 @@ export const LegacyElementMixin = dedupingMixin((base) => {
         this.root = /** @type {HTMLElement} */(this);
         this.created();
         // Pull all attribute values 1x if `legacyNoObservedAttributes` is set.
-        if (legacyNoObservedAttributes) {
+        if (legacyNoObservedAttributes && !this._legacyForceObservedAttributes) {
           if (this.hasAttributes()) {
             this._takeAttributes();
           // Element created from scratch or parser generated

diff --git a/lib/legacy/polymer-fn.js b/lib/legacy/polymer-fn.js
@@ -39,6 +39,10 @@ const Polymer = function(info) {
   } else {
     klass = Polymer.Class(info);
   }
+  // Copy opt out for `legacyNoObservedAttributes` from info object to class.
+  if (info._legacyForceObservedAttributes) {
+    klass.prototype._legacyForceObservedAttributes = info._legacyForceObservedAttributes;
+  }
   customElements.define(klass.is, /** @type {!HTMLElement} */(klass));
   return klass;
 };

diff --git a/test/unit/legacy-noattributes.html b/test/unit/legacy-noattributes.html
@@ -64,7 +64,7 @@
           zot: Boolean,
           shouldIf: Boolean,
           camelCase: String,
-          disabled: {type: Boolean, value: 'true'}
+          disabled: {type: Boolean, value: 'true'},
         },
         created() {
           this.wasCreated = true;
@@ -74,6 +74,17 @@
           this.attrInfo = {name, old, value};
         }
       });
+
+      Polymer({
+        is: 'x-native-attrs-force',
+        _legacyForceObservedAttributes: true,
+        properties: {
+          tabindex: {reflectToAttribute: true, type: Number}
+        },
+        attributeChanged() {
+          this.attributeChangedCalled = true;
+        }
+      });
     </script>
   </dom-module>
 
@@ -92,6 +103,7 @@
   <script type="module">
     import {flush} from '../../lib/utils/flush.js';
     import {wrap} from '../../lib/utils/wrap.js';
+    import {registrations} from '../../lib/utils/telemetry.js';
 
     suite('legacyNoObservedAttributes', () => {
 
@@ -100,6 +112,24 @@
         el = fixture('declarative');
       });
 
+      test('telemetry collected as expected', () => {
+        const defs = registrations.filter((def) => def.is === el.localName);
+        assert.ok(defs[0]);
+      });
+
+      test('native properties observeable when `_legacyForceObservedAttributes` set', function() {
+        // Unsupported when polyfill is in use since it uses `setAttribute`.
+        if (customElements.polyfillWrapFlushCallback) {
+          this.skip();
+        }
+        el = document.createElement('x-native-attrs-force');
+        document.body.appendChild(el);
+        el.tabIndex = 5;
+        assert.ok(el.attributeChangedCalled);
+        assert.equal(el.tabindex, 5);
+        document.body.removeChild(el);
+      });
+
       test('static attributes', () => {
         assert.equal(el.foo, 'foo');
         assert.equal(el.$.child1.getAttribute('foo'), 'x-attrs.foo');