From 039246156975da845c3fc1d9da87d4469fcf0964 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Sat, 10 Jun 2017 11:16:43 -0400 Subject: [PATCH] [BUGFIX beta] Make better errors for meta updates after object destruction. 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). --- packages/ember-metal/lib/meta.js | 25 +++--- packages/ember-metal/tests/computed_test.js | 18 ++++- packages/ember-metal/tests/meta_test.js | 90 +++++++++++++++++++++ 3 files changed, 120 insertions(+), 13 deletions(-) diff --git a/packages/ember-metal/lib/meta.js b/packages/ember-metal/lib/meta.js index 2d554208e8b..0e7167c8a37 100644 --- a/packages/ember-metal/lib/meta.js +++ b/packages/ember-metal/lib/meta.js @@ -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'; @@ -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]; @@ -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); @@ -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); @@ -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) { @@ -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; } @@ -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; } @@ -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; } @@ -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; } @@ -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; @@ -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; } @@ -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; diff --git a/packages/ember-metal/tests/computed_test.js b/packages/ember-metal/tests/computed_test.js index b48b4b7adc8..3711d227ff3 100644 --- a/packages/ember-metal/tests/computed_test.js +++ b/packages/ember-metal/tests/computed_test.js @@ -10,7 +10,8 @@ import { set, isWatching, addObserver, - _addBeforeObserver + _addBeforeObserver, + meta as metaFor } from '..'; let obj, count; @@ -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 = () => ''; + + expectAssertion(() => { + get(obj, 'foo', 'bar'); + }, 'Cannot modify dependent keys for `foo` on `` after it has been destroyed.'); + + equal(isWatching(obj, 'bar'), false, 'deps were not updated'); +}); + // .......................................................... // CHAINED DEPENDENT KEYS // diff --git a/packages/ember-metal/tests/meta_test.js b/packages/ember-metal/tests/meta_test.js index 2c732b53861..c31c7fd5ebf 100644 --- a/packages/ember-metal/tests/meta_test.js +++ b/packages/ember-metal/tests/meta_test.js @@ -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 ''; } + }; + let targetMeta = meta(target); + + targetMeta.destroy(); + + expectAssertion(() => { + targetMeta.writeWatching('hello', 1); + }, 'Cannot update watchers for `hello` on `` after it has been destroyed.'); +}); + +QUnit.test('meta.clearWatching issues useful error after destroy', function(assert) { + let target = { + toString() { return ''; } + }; + let targetMeta = meta(target); + + targetMeta.destroy(); + + expectAssertion(() => { + targetMeta.clearWatching(); + }, 'Cannot clear watchers on `` after it has been destroyed.'); +}); +QUnit.test('meta.writableTag issues useful error after destroy', function(assert) { + let target = { + toString() { return ''; } + }; + let targetMeta = meta(target); + + targetMeta.destroy(); + + expectAssertion(() => { + targetMeta.writableTag(() => {}); + }, 'Cannot create a new tag for `` after it has been destroyed.'); +}); + +QUnit.test('meta.writableChainWatchers issues useful error after destroy', function(assert) { + let target = { + toString() { return ''; } + }; + let targetMeta = meta(target); + + targetMeta.destroy(); + + expectAssertion(() => { + targetMeta.writableChainWatchers(() => {}); + }, 'Cannot create a new chain watcher for `` after it has been destroyed.'); +}); + +QUnit.test('meta.writableChains issues useful error after destroy', function(assert) { + let target = { + toString() { return ''; } + }; + let targetMeta = meta(target); + + targetMeta.destroy(); + + expectAssertion(() => { + targetMeta.writableChains(() => {}); + }, 'Cannot create a new chains for `` after it has been destroyed.'); +}); + +QUnit.test('meta.writeValues issues useful error after destroy', function(assert) { + let target = { + toString() { return ''; } + }; + let targetMeta = meta(target); + + targetMeta.destroy(); + + expectAssertion(() => { + targetMeta.writeValues('derp', 'ohai'); + }, 'Cannot set the value of `derp` on `` after it has been destroyed.'); +}); + +QUnit.test('meta.writeDeps issues useful error after destroy', function(assert) { + let target = { + toString() { return ''; } + }; + let targetMeta = meta(target); + + targetMeta.destroy(); + + expectAssertion(() => { + targetMeta.writeDeps('derp', 'ohai', 1); + }, 'Cannot modify dependent keys for `ohai` on `` after it has been destroyed.'); +});