Skip to content

Commit

Permalink
[BUGFIX beta] Make better errors for meta updates after object destru…
Browse files Browse the repository at this point in the history
…ction.

Prior to this change the errors being thrown by `Meta` were
extremely hard to track down since no "real" information was
given.

After this change, all errors include the `object.toString` and
property name in question (if applicable).
  • Loading branch information
rwjblue committed Jun 10, 2017
1 parent 4591eec commit 0392461
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 13 deletions.
25 changes: 13 additions & 12 deletions packages/ember-metal/lib/meta.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import {
HAS_NATIVE_WEAKMAP,
lookupDescriptor,
symbol
symbol,
toString
} from 'ember-utils';
import { protoMethods as listenerMethods } from './meta_listeners';
import { assert } from 'ember-debug';
Expand Down Expand Up @@ -211,7 +212,7 @@ export class Meta {
// Implements a member that provides a lazily created map of maps,
// with inheritance at both levels.
writeDeps(subkey, itemkey, value) {
assert(`Cannot call writeDeps after the object is destroyed.`, !this.isMetaDestroyed());
assert(`Cannot modify dependent keys for \`${itemkey}\` on \`${toString(this.source)}\` after it has been destroyed.`, !this.isMetaDestroyed());

let outerMap = this._getOrCreateOwnMap('_deps');
let innerMap = outerMap[subkey];
Expand Down Expand Up @@ -331,7 +332,7 @@ export class Meta {
readableTags() { return this._tags; }

writableTag(create) {
assert(`Cannot call writableTag after the object is destroyed.`, !this.isMetaDestroyed());
assert(`Cannot create a new tag for \`${toString(this.source)}\` after it has been destroyed.`, !this.isMetaDestroyed());
let ret = this._tag;
if (ret === undefined) {
ret = this._tag = create(this.source);
Expand All @@ -344,7 +345,7 @@ export class Meta {
}

writableChainWatchers(create) {
assert(`Cannot call writableChainWatchers after the object is destroyed.`, !this.isMetaDestroyed());
assert(`Cannot create a new chain watcher for \`${toString(this.source)}\` after it has been destroyed.`, !this.isMetaDestroyed());
let ret = this._chainWatchers;
if (ret === undefined) {
ret = this._chainWatchers = create(this.source);
Expand All @@ -357,7 +358,7 @@ export class Meta {
}

writableChains(create) {
assert(`Cannot call writableChains after the object is destroyed.`, !this.isMetaDestroyed());
assert(`Cannot create a new chains for \`${toString(this.source)}\` after it has been destroyed.`, !this.isMetaDestroyed());
let ret = this._chains;
if (ret === undefined) {
if (this.parent) {
Expand All @@ -374,7 +375,7 @@ export class Meta {
}

writeWatching(subkey, value) {
assert(`Cannot call writeWatching after the object is destroyed.`, !this.isMetaDestroyed());
assert(`Cannot update watchers for \`hello\` on \`${toString(this.source)}\` after it has been destroyed.`, !this.isMetaDestroyed());
let map = this._getOrCreateOwnMap('_watching');
map[subkey] = value;
}
Expand Down Expand Up @@ -402,7 +403,7 @@ export class Meta {
}

clearWatching() {
assert(`Cannot call clearWatching after the object is destroyed.`, !this.isMetaDestroyed());
assert(`Cannot clear watchers on \`${toString(this.source)}\` after it has been destroyed.`, !this.isMetaDestroyed());

this._watching = undefined;
}
Expand All @@ -416,7 +417,7 @@ export class Meta {
}

writeMixins(subkey, value) {
assert(`Cannot call writeMixins after the object is destroyed.`, !this.isMetaDestroyed());
assert(`Cannot add mixins for \`${subkey}\` on \`${toString(this.source)}\` call writeMixins after it has been destroyed.`, !this.isMetaDestroyed());
let map = this._getOrCreateOwnMap('_mixins');
map[subkey] = value;
}
Expand Down Expand Up @@ -444,7 +445,7 @@ export class Meta {
}

clearMixins() {
assert(`Cannot call clearMixins after the object is destroyed.`, !this.isMetaDestroyed());
assert(`Cannot clear mixins on \`${toString(this.source)}\` after it has been destroyed.`, !this.isMetaDestroyed());

this._mixins = undefined;
}
Expand All @@ -458,7 +459,7 @@ export class Meta {
}

writeBindings(subkey, value) {
assert(`Cannot call writeBindings after the object is destroyed.`, !this.isMetaDestroyed());
assert(`Cannot add a binding for \`${subkey}\` on \`${toString(this.source)}\` after it has been destroyed.`, !this.isMetaDestroyed());

let map = this._getOrCreateOwnMap('_bindings');
map[subkey] = value;
Expand Down Expand Up @@ -487,7 +488,7 @@ export class Meta {
}

clearBindings() {
assert(`Cannot call clearBindings after the object is destroyed.`, !this.isMetaDestroyed());
assert(`Cannot clear bindings on \`${toString(this.source)}\` after it has been destroyed.`, !this.isMetaDestroyed());
this._bindings = undefined;
}

Expand All @@ -500,7 +501,7 @@ export class Meta {
}

writeValues(subkey, value) {
assert(`Cannot call writeValues after the object is destroyed.`, !this.isMetaDestroyed());
assert(`Cannot set the value of \`${subkey}\` on \`${toString(this.source)}\` after it has been destroyed.`, !this.isMetaDestroyed());

let map = this._getOrCreateOwnMap('_values');
map[subkey] = value;
Expand Down
18 changes: 17 additions & 1 deletion packages/ember-metal/tests/computed_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import {
set,
isWatching,
addObserver,
_addBeforeObserver
_addBeforeObserver,
meta as metaFor
} from '..';

let obj, count;
Expand Down Expand Up @@ -481,6 +482,21 @@ testBoth('throws assertion if brace expansion notation has spaces', function (ge
}, /cannot contain spaces/);
});

testBoth('throws an assertion if an uncached `get` is called after object is destroyed', function(get, set) {
equal(isWatching(obj, 'bar'), false, 'precond not watching dependent key');

let meta = metaFor(obj);
meta.destroy();

obj.toString = () => '<custom-obj:here>';

expectAssertion(() => {
get(obj, 'foo', 'bar');
}, 'Cannot modify dependent keys for `foo` on `<custom-obj:here>` after it has been destroyed.');

equal(isWatching(obj, 'bar'), false, 'deps were not updated');
});

// ..........................................................
// CHAINED DEPENDENT KEYS
//
Expand Down
90 changes: 90 additions & 0 deletions packages/ember-metal/tests/meta_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,93 @@ QUnit.test('meta.listeners deduplication', function(assert) {
assert.equal(matching.length, 3);
assert.equal(matching[0], t);
});

QUnit.test('meta.writeWatching issues useful error after destroy', function(assert) {
let target = {
toString() { return '<special-sauce:123>'; }
};
let targetMeta = meta(target);

targetMeta.destroy();

expectAssertion(() => {
targetMeta.writeWatching('hello', 1);
}, 'Cannot update watchers for `hello` on `<special-sauce:123>` after it has been destroyed.');
});

QUnit.test('meta.clearWatching issues useful error after destroy', function(assert) {
let target = {
toString() { return '<special-sauce:123>'; }
};
let targetMeta = meta(target);

targetMeta.destroy();

expectAssertion(() => {
targetMeta.clearWatching();
}, 'Cannot clear watchers on `<special-sauce:123>` after it has been destroyed.');
});
QUnit.test('meta.writableTag issues useful error after destroy', function(assert) {
let target = {
toString() { return '<special-sauce:123>'; }
};
let targetMeta = meta(target);

targetMeta.destroy();

expectAssertion(() => {
targetMeta.writableTag(() => {});
}, 'Cannot create a new tag for `<special-sauce:123>` after it has been destroyed.');
});

QUnit.test('meta.writableChainWatchers issues useful error after destroy', function(assert) {
let target = {
toString() { return '<special-sauce:123>'; }
};
let targetMeta = meta(target);

targetMeta.destroy();

expectAssertion(() => {
targetMeta.writableChainWatchers(() => {});
}, 'Cannot create a new chain watcher for `<special-sauce:123>` after it has been destroyed.');
});

QUnit.test('meta.writableChains issues useful error after destroy', function(assert) {
let target = {
toString() { return '<special-sauce:123>'; }
};
let targetMeta = meta(target);

targetMeta.destroy();

expectAssertion(() => {
targetMeta.writableChains(() => {});
}, 'Cannot create a new chains for `<special-sauce:123>` after it has been destroyed.');
});

QUnit.test('meta.writeValues issues useful error after destroy', function(assert) {
let target = {
toString() { return '<special-sauce:123>'; }
};
let targetMeta = meta(target);

targetMeta.destroy();

expectAssertion(() => {
targetMeta.writeValues('derp', 'ohai');
}, 'Cannot set the value of `derp` on `<special-sauce:123>` after it has been destroyed.');
});

QUnit.test('meta.writeDeps issues useful error after destroy', function(assert) {
let target = {
toString() { return '<special-sauce:123>'; }
};
let targetMeta = meta(target);

targetMeta.destroy();

expectAssertion(() => {
targetMeta.writeDeps('derp', 'ohai', 1);
}, 'Cannot modify dependent keys for `ohai` on `<special-sauce:123>` after it has been destroyed.');
});

0 comments on commit 0392461

Please sign in to comment.