Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX beta] Make better errors for meta updates after object destruction. #15347

Merged
merged 1 commit into from
Jun 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hello?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi 👋

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.');
});