set the names of known mixins (#1055)

remove properties set by mixins unless different value
caused them to hide the class with only merged mixins
from Object.extend(mix1, mix2)

Author: patricklx

ember_debug/object-inspector.js

@@ -17,11 +17,11 @@ let HAS_GLIMMER_TRACKING = false;
 try {
   glimmer = Ember.__loader.require('@glimmer/reference');
   metal = Ember.__loader.require('@ember/-internals/metal');
-  HAS_GLIMMER_TRACKING = glimmer && 
-                         glimmer.value && 
-                         glimmer.validate && 
-                         metal && 
-                         metal.track && 
+  HAS_GLIMMER_TRACKING = glimmer &&
+                         glimmer.value &&
+                         glimmer.validate &&
+                         metal &&
+                         metal.track &&
                          metal.tagForProperty;
 } catch (e) {
   glimmer = null;
@@ -30,6 +30,39 @@ try {
 
 const keys = Object.keys || Ember.keys;
 
+/**
+ * Add Known Ember Mixins and Classes so we can label them correctly in the inspector
+ */
+const emberNames = new Map([
+  [Ember.Evented, 'Evented Mixin'],
+  [Ember.PromiseProxyMixin, 'PromiseProxy Mixin'],
+  [Ember.MutableArray, 'MutableArray Mixin'],
+  [Ember.MutableEnumerable, 'MutableEnumerable Mixin'],
+  [Ember.NativeArray, 'NativeArray Mixin'],
+  [Ember.Observable, 'Observable Mixin'],
+  [Ember.ControllerMixin, 'Controller Mixin'],
+  [Ember.TargetActionSupport, 'TargetActionSupport Mixin'],
+  [Ember.ActionHandler, 'ActionHandler Mixin'],
+  [Ember.CoreObject, 'CoreObject'],
+  [Ember.Object, 'EmberObject'],
+  [Ember.Component, 'Component'],
+]);
+
+try {
+  const Views = Ember.__loader.require('@ember/-internals/views');
+  emberNames.set(Views.ViewStateSupport, 'ViewStateSupport Mixin');
+  emberNames.set(Views.ViewMixin, 'View Mixin');
+  emberNames.set(Views.ActionSupport, 'ActionSupport Mixin');
+  emberNames.set(Views.ClassNamesSupport, 'ClassNamesSupport Mixin');
+  emberNames.set(Views.ChildViewsSupport, 'ChildViewsSupport Mixin');
+  emberNames.set(Views.ViewStateSupport , 'ViewStateSupport  Mixin');
+  // this one is not a Mixin, but an .extend({}), which results in a class
+  emberNames.set(Views.CoreView, 'CoreView');
+} catch (e) {
+  // do nothing
+}
+
+
 /**
  * Determine the type and get the value of the passed property
  * @param {*} object The parent object we will look for `key` on
@@ -132,7 +165,7 @@ function isMandatorySetter(descriptor) {
   return false;
 }
 
-function getTagTrackedProps(tag, ownTag, level=0) {
+function getTagTrackedProps(tag, ownTag, level = 0) {
   const props = [];
   // do not include tracked properties from dependencies
   if (!tag || level > 1) {
@@ -179,7 +212,7 @@ export default EmberObject.extend(PortMixin, {
 
   updateCurrentObject() {
     if (this.currentObject) {
-      const {object, mixinDetails, objectId} = this.currentObject;
+      const { object, mixinDetails, objectId } = this.currentObject;
       mixinDetails.forEach((mixin, mixinIndex) => {
         mixin.properties.forEach(item => {
           if (item.overridden) {
@@ -193,7 +226,7 @@ export default EmberObject.extend(PortMixin, {
             let value = null;
             let changed = false;
             const values = this.objectPropertyValues[objectId] = this.objectPropertyValues[objectId] || {};
-            const tracked = this.trackedTags[objectId] = this.trackedTags[objectId]  || {};
+            const tracked = this.trackedTags[objectId] = this.trackedTags[objectId] || {};
 
             const desc = Object.getOwnPropertyDescriptor(object, item.name);
             const isSetter = desc && isMandatorySetter(desc);
@@ -525,26 +558,28 @@ export default EmberObject.extend(PortMixin, {
   mixinDetailsForObject(object) {
     const mixins = [];
 
-    const mixin = {
+    const own = ownMixins(object);
+
+    const objectMixin = {
       id: guidFor(object),
       name: getClassName(object),
-      properties: ownProperties(object)
+      properties: ownProperties(object, own)
     };
 
-    mixins.push(mixin);
+    mixins.push(objectMixin);
 
     // insert ember mixins
-    for (let mixin of ownMixins(object)) {
-      let name = (mixin[Ember.NAME_KEY] || mixin.ownerConstructor || '').toString();
+    for (let mixin of own) {
+      let name = (mixin[Ember.NAME_KEY] || mixin.ownerConstructor || emberNames.get(mixin) || '').toString();
 
-      if (typeof mixin.toString === 'function') {
+      if (!name && (typeof mixin.toString === 'function')) {
         try {
           name = mixin.toString();
 
           if (name === '(unknown)') {
             name = '(unknown mixin)';
           }
-        } catch(e) {
+        } catch (e) {
           name = '(Unable to convert Object to string)';
         }
       }
@@ -596,7 +631,7 @@ export default EmberObject.extend(PortMixin, {
     let objectId = this.retainObject(object);
 
     let errorsForObject = this.get('_errorsFor')[objectId] = {};
-    const tracked = this.trackedTags[objectId] = this.trackedTags[objectId]  || {};
+    const tracked = this.trackedTags[objectId] = this.trackedTags[objectId] || {};
     calculateCPs(object, mixinDetails, errorsForObject, expensiveProperties, tracked);
 
     this.currentObject = { object, mixinDetails, objectId };
@@ -629,7 +664,7 @@ export default EmberObject.extend(PortMixin, {
 
 function getClassName(object) {
   let name = '';
-  let className = object.constructor.name;
+  let className = emberNames.get(object.constructor) || object.constructor.name;
 
   if ('toString' in object && object.toString !== Function.prototype.toString) {
     name = object.toString();
@@ -677,10 +712,9 @@ function ownMixins(object) {
   return mixins;
 }
 
-function ownProperties(object) {
+function ownProperties(object, ownMixins) {
   let meta = Ember.meta(object);
-  let parentMeta = meta.parent;
-  
+
   if (Array.isArray(object)) {
     // slice to max 101, for performance and so that the object inspector will show a `more items` indicator above 100
     object = object.slice(0, 101);
@@ -689,13 +723,36 @@ function ownProperties(object) {
   let props = Object.getOwnPropertyDescriptors(object);
   delete props.constructor;
 
+  // meta has the correct descriptors for CPs
   meta.forEachDescriptors((name, desc) => {
-    // TODO: Need to add a way to get own descriptors only from meta
-    if (!parentMeta || !parentMeta.peekDescriptors(name)) {
+    // only for own properties
+    if (props[name]) {
       props[name] = desc;
     }
   });
 
+  // remove properties set by mixins
+  // especially for Object.extend(mixin1, mixin2), where a new class is created which holds the merged properties
+  // if all properties are removed, it will be marked as useless mixin and will not be shown
+  ownMixins.forEach((m) => {
+    if (m.mixins) {
+      m.mixins.forEach((mix) => {
+        Object.keys(mix.properties || {}).forEach((k) => {
+          const pDesc = Object.getOwnPropertyDescriptor(mix.properties, k);
+          if (pDesc && props[k] && pDesc.get && pDesc.get === props[k].get) {
+            delete props[k];
+          }
+          if (pDesc && props[k] && 'value' in pDesc && pDesc.value === props[k].value) {
+            delete props[k];
+          }
+          if (pDesc && props[k] && pDesc._getter === props[k]._getter) {
+            delete props[k];
+          }
+        })
+      })
+    }
+  });
+
   Object.keys(props).forEach(k => {
     if (typeof props[k].value === 'function') {
       return;
@@ -910,7 +967,7 @@ function calculateCPs(object, mixinDetails, errorsForObject, expensiveProperties
             tagInfo.revision = glimmer.value(object, item.name);
             item.dependentKeys = getTrackedDependencies(object, item.name, tagInfo.tag);
           } else {
-           value = calculateCP(object, item.name, errorsForObject);
+            value = calculateCP(object, item.name, errorsForObject);
           }
           if (!value || !(value instanceof CalculateCPError)) {
             item.value = inspectValue(object, item.name, value);
@@ -1080,13 +1137,13 @@ function calculateCP(object, property, errorsForObject) {
       return object.objectAt(property);
     }
     return get(object, property);
-  } catch(error) {
+  } catch (error) {
     errorsForObject[property] = { property, error };
     return new CalculateCPError();
   }
 }
 
-function CalculateCPError() {}
+function CalculateCPError() { }
 
 function errorsToSend(errors) {
   return toArray(errors).map(error => ({ property: error.property }));

tests/ember_debug/object-inspector-test.js

@@ -220,13 +220,21 @@ module('Ember Debug - Object Inspector', function(hooks) {
 
   test('Correct mixin order with es6 class', function (assert) {
 
-    // eslint-disable-next-line ember/no-new-mixins
-    const MyMixin = Mixin.create({
+    class MyMixinClass extends Mixin {
+      toString() {
+        return 'MyMixin';
+      }
+    }
+    const MyMixin = MyMixinClass.create({
       a: 'custom',
     });
 
-    // eslint-disable-next-line ember/no-new-mixins
-    const MyMixin2 = Mixin.create({
+    class MyMixin2Class extends Mixin {
+      toString() {
+        return 'MyMixin2';
+      }
+    }
+    const MyMixin2 = MyMixin2Class.create({
       b: 'custom2',
     });
 
@@ -249,13 +257,12 @@ module('Ember Debug - Object Inspector', function(hooks) {
       'Own Properties',
       'Foo',
       'FooBar',
-      '(unknown)',
-      '(unknown mixin)',
-      '(unknown mixin)',
+      'MyMixin',
+      'MyMixin2',
       'Bar',
       'Baz',
       'EmberObject',
-      '(unknown mixin)',
+      'Observable Mixin',
       'CoreObject'
     ];
 
@@ -264,6 +271,47 @@ module('Ember Debug - Object Inspector', function(hooks) {
     });
   });
 
+  test('Correct mixin properties', function (assert) {
+
+    // eslint-disable-next-line ember/no-new-mixins
+    class MyMixin extends Mixin {
+      toString() {
+        return 'MyMixin1';
+      }
+    }
+    // eslint-disable-next-line ember/no-new-mixins
+    class MyMixin2 extends Mixin {
+      toString() {
+        return 'MyMixin2';
+      }
+    }
+
+    const mix1 = MyMixin.create({a: 'custom1'});
+    const mix2 = MyMixin2.create({b: 'custom2'});
+
+    class Foo extends EmberObject.extend(mix1, mix2) {}
+
+    const instance = Foo.create({
+      ownProp: 'b'
+    });
+
+    objectInspector.sendObject(instance);
+
+    const details = message.details;
+
+    assert.equal(details[0].properties.length, 1, 'should not show mixin properties');
+    assert.equal(details[0].properties[0].name, 'ownProp');
+
+    assert.equal(details[2].name, 'MyMixin1');
+    assert.equal(details[2].properties.length, 1, 'should only show own mixin properties');
+    assert.equal(details[2].properties[0].value.inspect, inspect('custom1'));
+
+    assert.equal(details[3].name, 'MyMixin2');
+    assert.equal(details[3].properties.length, 1, 'should only show own mixin properties');
+    assert.equal(details[3].properties[0].value.inspect, inspect('custom2'));
+  });
+
+
   test('Computed properties are correctly calculated', function(assert) {
     let inspected = EmberObject.extend({
       hi: computed(function() {
@@ -459,11 +507,7 @@ module('Ember Debug - Object Inspector', function(hooks) {
 
   test('Property grouping can be customized using _debugInfo', function(assert) {
     // eslint-disable-next-line ember/no-new-mixins
-    let mixinToSkip = Mixin.create({
-      toString() {
-        return 'MixinToSkip';
-      }
-    });
+    let mixinToSkip = Mixin.create({});
 
     let Inspected = EmberObject.extend(mixinToSkip, {
       name: 'Teddy',
@@ -518,6 +562,7 @@ module('Ember Debug - Object Inspector', function(hooks) {
 
     assert.equal(message.details[3].name, 'TestObject');
     assert.equal(message.details[3].properties.length, 3, 'Correctly merges properties');
+    assert.equal(message.details[3].properties[0].name, 'toString');
     assert.equal(message.details[3].properties[1].name, 'hasChildren');
     assert.equal(message.details[3].properties[2].name, 'expensiveProperty', 'property name is correct');
     assert.equal(message.details[3].properties[2].value.isCalculated, undefined, 'Does not calculate expensive properties');