Skip to content

Commit

Permalink
set the names of known mixins (emberjs#1055)
Browse files Browse the repository at this point in the history 
remove properties set by mixins unless different value
caused them to hide the class with only merged mixins
from Object.extend(mix1, mix2)
  • Loading branch information
@patricklx @nummi
patricklx authored and nummi committed Apr 1, 2020
1 parent dbee05f commit 83c6e4d
Show file tree
Hide file tree
Showing 2 changed files with 140 additions and 38 deletions.
107 changes: 82 additions & 25 deletions ember_debug/object-inspector.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
Expand Down Expand Up @@ -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)';
}
}
Expand Down Expand Up @@ -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 };
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 }));
Expand Down
71 changes: 58 additions & 13 deletions tests/ember_debug/object-inspector-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
});

Expand All @@ -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'
];

Expand All @@ -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() {
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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');
Expand Down

0 comments on commit 83c6e4d

Please sign in to comment.