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 release] Add deprecation messages when using get/set in a deprecated way #11812

Merged
merged 1 commit into from Jul 19, 2015
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
8 changes: 7 additions & 1 deletion packages/ember-metal/lib/property_get.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,15 @@ export let UNHANDLED_GET = symbol("UNHANDLED_GET");
@public
*/
export function get(obj, keyName) {
Ember.deprecate(`Get must be called with two arguments; an object and a property key`,
arguments.length === 2);
// Helpers that operate with 'this' within an #each
if (keyName === '') {
return obj;
}

if (!keyName && 'string' === typeof obj) {
Ember.deprecate('Calling Ember.get with only a property key has been deprecated, please also specify a target object.');
keyName = obj;
obj = Ember.lookup;
}
Expand All @@ -65,6 +68,7 @@ export function get(obj, keyName) {
Ember.assert(`Cannot call get with '${keyName}' on an undefined object.`, obj !== undefined);

if (isNone(obj)) {
Ember.deprecate('Calling Ember.get without a target object has been deprecated, please specify a target object.');
return _getPath(obj, keyName);
}

Expand Down Expand Up @@ -159,6 +163,8 @@ export function _getPath(root, path) {
// detect complicated paths and normalize them
hasThis = pathHasThis(path);

Ember.deprecate(`Ember.get with 'this' in the path has been deprecated. Please use the same path without 'this'.`, !hasThis);

if (!root || hasThis) {
tuple = normalizeTuple(root, path);
root = tuple[0];
Expand All @@ -169,7 +175,7 @@ export function _getPath(root, path) {
parts = path.split(".");
len = parts.length;
for (idx = 0; root != null && idx < len; idx++) {
root = get(root, parts[idx], true);
root = get(root, parts[idx]);
if (root && root.isDestroyed) { return undefined; }
}
return root;
Expand Down
7 changes: 7 additions & 0 deletions packages/ember-metal/lib/property_set.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,17 @@ export let UNHANDLED_SET = symbol("UNHANDLED_SET");
export function set(obj, keyName, value, tolerant) {
if (typeof obj === 'string') {
Ember.assert(`Path '${obj}' must be global if no obj is given.`, isGlobalPath(obj));
Ember.deprecate('Calling Ember.set with only a property key and a value has been deprecated, please also specify a target object.');
value = keyName;
keyName = obj;
obj = Ember.lookup;
}

Ember.deprecate(`Set must be called with tree or four arguments; an object, a property key, a value and tolerant true/false`,
arguments.length === 3 || arguments.length === 4);
Ember.assert(`Cannot call set with '${keyName}' key.`, !!keyName);


if (obj === Ember.lookup) {
return setPath(obj, keyName, value, tolerant);
}
Expand All @@ -62,6 +66,7 @@ export function set(obj, keyName, value, tolerant) {

var isUnknown, currentValue;
if ((!obj || desc === undefined) && isPath(keyName)) {
Ember.deprecate('Calling Ember.set without a target object has been deprecated, please specify a target object.', !!obj);
return setPath(obj, keyName, value, tolerant);
}

Expand Down Expand Up @@ -139,6 +144,8 @@ function setPath(root, path, value, tolerant) {
// get the root
if (path !== 'this') {
root = getPath(root, path);
} else {
Ember.deprecate(`Ember.set with 'this' in the path has been deprecated. Please use the same path without 'this'.`);
}

if (!keyName || keyName.length === 0) {
Expand Down
10 changes: 10 additions & 0 deletions packages/ember-metal/tests/accessors/get_path_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,17 @@ QUnit.test('[obj, foothis.bar] -> obj.foothis.bar', function() {
});

QUnit.test('[obj, this.foo] -> obj.foo', function() {
expectDeprecation(/Ember.get with 'this' in the path has been deprecated. Please use the same path without 'this'./);
deepEqual(get(obj, 'this.foo'), obj.foo);
});

QUnit.test('[obj, this.foo.bar] -> obj.foo.bar', function() {
expectDeprecation(/Ember.get with 'this' in the path has been deprecated. Please use the same path without 'this'./);
deepEqual(get(obj, 'this.foo.bar'), obj.foo.bar);
});

QUnit.test('[obj, this.Foo.bar] -> (undefined)', function() {
expectDeprecation(/Ember.get with 'this' in the path has been deprecated. Please use the same path without 'this'./);
equal(get(obj, 'this.Foo.bar'), undefined);
});

Expand Down Expand Up @@ -109,18 +112,22 @@ QUnit.test('[obj, Foo.bar] -> (undefined)', function() {
//

QUnit.test('[null, Foo] -> Foo', function() {
expectDeprecation(/Calling Ember.get without a target object has been deprecated, please specify a target object./);
equal(get(null, 'Foo'), Foo);
});

QUnit.test('[null, Foo.bar] -> Foo.bar', function() {
expectDeprecation(/Calling Ember.get without a target object has been deprecated, please specify a target object./);
deepEqual(get(null, 'Foo.bar'), Foo.bar);
});

QUnit.test('[null, $foo] -> $foo', function() {
expectDeprecation(/Calling Ember.get without a target object has been deprecated, please specify a target object./);
equal(get(null, '$foo'), window.$foo);
});

QUnit.test('[null, aProp] -> null', function() {
expectDeprecation(/Calling Ember.get without a target object has been deprecated, please specify a target object./);
equal(get(null, 'aProp'), null);
});

Expand All @@ -129,13 +136,16 @@ QUnit.test('[null, aProp] -> null', function() {
//

QUnit.test('[Foo] -> Foo', function() {
expectDeprecation(/Calling Ember.get with only a property key has been deprecated, please also specify a target object/);
deepEqual(get('Foo'), Foo);
});

QUnit.test('[aProp] -> aProp', function() {
expectDeprecation(/Calling Ember.get with only a property key has been deprecated, please also specify a target object/);
deepEqual(get('aProp'), window.aProp);
});

QUnit.test('[Foo.bar] -> Foo.bar', function() {
expectDeprecation(/Calling Ember.get with only a property key has been deprecated, please also specify a target object/);
deepEqual(get('Foo.bar'), Foo.bar);
});
2 changes: 2 additions & 0 deletions packages/ember-metal/tests/accessors/get_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,12 @@ QUnit.test('warn on attempts to get a property path of undefined', function() {
});

QUnit.test('returns null when fetching a complex local path on a null context', function() {
expectDeprecation(/Calling Ember.get without a target object has been deprecated, please specify a target object./);
equal(get(null, 'aProperty.on.aPath'), null);
});

QUnit.test('returns null when fetching a simple local path on a null context', function() {
expectDeprecation(/Calling Ember.get without a target object has been deprecated, please specify a target object./);
equal(get(null, 'aProperty'), null);
});

Expand Down
9 changes: 9 additions & 0 deletions packages/ember-metal/tests/accessors/set_path_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,13 @@ QUnit.test('[obj, foo.bar] -> obj.foo.bar', function() {
});

QUnit.test('[obj, this.foo] -> obj.foo', function() {
expectDeprecation(/Ember.set with 'this' in the path has been deprecated. Please use the same path without 'this'./);
set(obj, 'this.foo', "BAM");
equal(get(obj, 'foo'), "BAM");
});

QUnit.test('[obj, this.foo.bar] -> obj.foo.bar', function() {
expectDeprecation(/Ember.get with 'this' in the path has been deprecated. Please use the same path without 'this'./);
set(obj, 'this.foo.bar', "BAM");
equal(get(obj, 'foo.bar'), "BAM");
});
Expand All @@ -77,10 +79,17 @@ QUnit.test('[obj, this.foo.bar] -> obj.foo.bar', function() {
//

QUnit.test('[null, Foo.bar] -> Foo.bar', function() {
expectDeprecation(/Calling Ember.set without a target object has been deprecated, please specify a target object./);
set(null, 'Foo.bar', "BAM");
equal(get(Ember.lookup.Foo, 'bar'), "BAM");
});

QUnit.test('[Foo.bar] -> Foo.bar', function() {
expectDeprecation(/Calling Ember.set with only a property key and a value has been deprecated, please also specify a target object./);
set('Foo.bar', "BAM");
equal(get(Ember.lookup.Foo, 'bar'), "BAM");
});

// ..........................................................
// DEPRECATED
//
Expand Down
1 change: 1 addition & 0 deletions packages/ember-metal/tests/computed_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,7 @@ testBoth('depending on simple chain', function(get, set) {
});

testBoth('depending on Global chain', function(get, set) {
expectDeprecation(/Calling Ember.get with only a property key has been deprecated, please also specify a target object/);

// assign computed property
defineProperty(obj, 'prop', computed(function() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ QUnit.test("raise if the provided object is undefined", function() {
});

QUnit.test("should work when object is Ember (used in Ember.get)", function() {
expectDeprecation(/Calling Ember.get with only a property key has been deprecated, please also specify a target object/);
equal(get('Ember.RunLoop'), Ember.RunLoop, 'Ember.get');
equal(get(Ember, 'RunLoop'), Ember.RunLoop, 'Ember.get(Ember, RunLoop)');
});
Expand All @@ -173,6 +174,7 @@ QUnit.module("Ember.get() with paths", {
});

QUnit.test('should return a property at a given path relative to the lookup', function() {
expectDeprecation(/Calling Ember.get with only a property key has been deprecated, please also specify a target object/);
lookup.Foo = ObservableObject.extend({
Bar: ObservableObject.extend({
Baz: computed(function() { return 'blargh'; }).volatile()
Expand All @@ -193,6 +195,7 @@ QUnit.test("should return a property at a given path relative to the passed obje
});

QUnit.test("should return a property at a given path relative to the lookup - JavaScript hash", function() {
expectDeprecation(/Calling Ember.get with only a property key has been deprecated, please also specify a target object/);
lookup.Foo = {
Bar: {
Baz: "blargh"
Expand Down
6 changes: 4 additions & 2 deletions packages/ember-runtime/tests/mixins/deferred_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,9 @@ if (!EmberDev.runningProdBuild) {
var obj = {};

Ember.deprecate = function(message) {
deprecationMade = message;
if (message === 'Usage of Ember.DeferredMixin or Ember.Deferred is deprecated.') {
deprecationMade = true;
}
};

deferred = EmberObject.extend(Deferred).create();
Expand All @@ -360,7 +362,7 @@ if (!EmberDev.runningProdBuild) {
deferred.then(function(value) {
equal(value, obj, "successfully resolved to given value");
});
equal(deprecationMade, 'Usage of Ember.DeferredMixin or Ember.Deferred is deprecated.');
equal(deprecationMade, true, 'the deprecation was made');

run(deferred, 'resolve', obj);
});
Expand Down