Skip to content

Commit

Permalink
Merge pull request #11746 from martndemus/cleanup-ember-get
Browse files Browse the repository at this point in the history 
[CLEANUP Beta] Cleanup Ember.get
  • Loading branch information
@stefanpenner
stefanpenner committed Jul 15, 2015
2 parents 5d18fcf + e3208b1 commit 6413ad8
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 187 deletions.
22 changes: 7 additions & 15 deletions packages/ember-metal/lib/property_get.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
hasThis as pathHasThis
} from 'ember-metal/path_cache';
import { symbol } from 'ember-metal/utils';
import isNone from 'ember-metal/is_none';

var FIRST_KEY = /^([^\.]+)/;

Expand Down Expand Up @@ -51,24 +50,17 @@ export let UNHANDLED_GET = symbol('UNHANDLED_GET');
@public
*/
export function get(obj, keyName) {
Ember.assert(`Get must be called with two arguments; an object and a property key`, arguments.length === 2);
Ember.assert(`Cannot call get with '${keyName}' on an undefined object.`, obj !== undefined && obj !== null);
Ember.assert(`The key provided to get must be a string, you passed ${keyName}`, typeof keyName === 'string');
Ember.assert(`'this' in paths is not supported`, !pathHasThis(keyName));

// Helpers that operate with 'this' within an #each
if (keyName === '') {
return obj;
}

if (!keyName && 'string' === typeof obj) {
keyName = obj;
obj = Ember.lookup;
}

Ember.assert(`Cannot call get with ${keyName} key.`, !!keyName);
Ember.assert(`Cannot call get with '${keyName}' on an undefined object.`, obj !== undefined);

if (isNone(obj)) {
return _getPath(obj, keyName);
}

if (obj && typeof obj[INTERCEPT_GET] === 'function') {
if (typeof obj[INTERCEPT_GET] === 'function') {
let result = obj[INTERCEPT_GET](obj, keyName);
if (result !== UNHANDLED_GET) { return result; }
}
Expand Down Expand Up @@ -169,7 +161,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
67 changes: 0 additions & 67 deletions packages/ember-metal/tests/accessors/get_path_test.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
/*globals Foo:true $foo:true */

import { get } from 'ember-metal/property_get';

var obj;
Expand All @@ -22,27 +20,10 @@ var moduleOpts = {
nar: 'foo'
}
};

window.Foo = {
bar: {
baz: { biff: 'FooBiff' }
}
};

window.aProp = 'aPropy';

window.$foo = {
bar: {
baz: { biff: '$FOOBIFF' }
}
};
},

teardown() {
obj = undefined;
window.Foo = undefined;
window.aProp = undefined;
window.$foo = undefined;
}
};

Expand All @@ -64,18 +45,6 @@ QUnit.test('[obj, foothis.bar] -> obj.foothis.bar', function() {
deepEqual(get(obj, 'foothis.bar'), obj.foothis.bar);
});

QUnit.test('[obj, this.foo] -> obj.foo', function() {
deepEqual(get(obj, 'this.foo'), obj.foo);
});

QUnit.test('[obj, this.foo.bar] -> obj.foo.bar', function() {
deepEqual(get(obj, 'this.foo.bar'), obj.foo.bar);
});

QUnit.test('[obj, this.Foo.bar] -> (undefined)', function() {
equal(get(obj, 'this.Foo.bar'), undefined);
});

QUnit.test('[obj, falseValue.notDefined] -> (undefined)', function() {
equal(get(obj, 'falseValue.notDefined'), undefined);
});
Expand Down Expand Up @@ -103,39 +72,3 @@ QUnit.test('[obj, Foo] -> (undefined)', function() {
QUnit.test('[obj, Foo.bar] -> (undefined)', function() {
equal(get(obj, 'Foo.bar'), undefined);
});

// ..........................................................
// NULL TARGET
//

QUnit.test('[null, Foo] -> Foo', function() {
equal(get(null, 'Foo'), Foo);
});

QUnit.test('[null, Foo.bar] -> Foo.bar', function() {
deepEqual(get(null, 'Foo.bar'), Foo.bar);
});

QUnit.test('[null, $foo] -> $foo', function() {
equal(get(null, '$foo'), window.$foo);
});

QUnit.test('[null, aProp] -> null', function() {
equal(get(null, 'aProp'), null);
});

// ..........................................................
// NO TARGET
//

QUnit.test('[Foo] -> Foo', function() {
deepEqual(get('Foo'), Foo);
});

QUnit.test('[aProp] -> aProp', function() {
deepEqual(get('aProp'), window.aProp);
});

QUnit.test('[Foo.bar] -> Foo.bar', function() {
deepEqual(get('Foo.bar'), Foo.bar);
});
43 changes: 34 additions & 9 deletions packages/ember-metal/tests/accessors/get_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,24 @@ testBoth('should call unknownProperty on watched values if the value is undefine
equal(get(obj, 'foo'), 'FOO', 'should return value from unknown');
});

QUnit.test('warn on attemps to call get with no arguments', function() {
expectAssertion(function() {
get('aProperty');
}, /Get must be called with two arguments;/i);
});

QUnit.test('warn on attemps to call get with only one argument', function() {
expectAssertion(function() {
get('aProperty');
}, /Get must be called with two arguments;/i);
});

QUnit.test('warn on attemps to call get with more then two arguments', function() {
expectAssertion(function() {
get({}, 'aProperty', true);
}, /Get must be called with two arguments;/i);
});

QUnit.test('warn on attempts to get a property of undefined', function() {
expectAssertion(function() {
get(undefined, 'aProperty');
Expand All @@ -107,28 +125,35 @@ QUnit.test('warn on attempts to get a property path of undefined', function() {
}, /Cannot call get with 'aProperty.on.aPath' on an undefined object/);
});

QUnit.test('returns null when fetching a complex local path on a null context', function() {
equal(get(null, 'aProperty.on.aPath'), null);
QUnit.test('warn on attempts to get a property of null', function() {
expectAssertion(function() {
get(null, 'aProperty');
}, /Cannot call get with 'aProperty' on an undefined object/);
});

QUnit.test('returns null when fetching a simple local path on a null context', function() {
equal(get(null, 'aProperty'), null);
QUnit.test('warn on attempts to get a property path of null', function() {
expectAssertion(function() {
get(null, 'aProperty.on.aPath');
}, /Cannot call get with 'aProperty.on.aPath' on an undefined object/);
});

QUnit.test('warn on attempts to get a falsy property', function() {
QUnit.test('warn on attempts to use get with an unsupported property path', function() {
var obj = {};
expectAssertion(function() {
get(obj, null);
}, /Cannot call get with null key/);
}, /The key provided to get must be a string, you passed null/);
expectAssertion(function() {
get(obj, NaN);
}, /Cannot call get with NaN key/);
}, /The key provided to get must be a string, you passed NaN/);
expectAssertion(function() {
get(obj, undefined);
}, /Cannot call get with undefined key/);
}, /The key provided to get must be a string, you passed undefined/);
expectAssertion(function() {
get(obj, false);
}, /Cannot call get with false key/);
}, /The key provided to get must be a string, you passed false/);
expectAssertion(function() {
get(obj, 42);
}, /The key provided to get must be a string, you passed 42/);
});

// ..........................................................
Expand Down
67 changes: 2 additions & 65 deletions packages/ember-metal/tests/computed_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ import {
_addBeforeObserver
} from 'ember-metal/observer';

var originalLookup = Ember.lookup;
var obj, count, Global, lookup;
var obj, count;

QUnit.module('computed');

Expand Down Expand Up @@ -444,9 +443,6 @@ testBoth('throws assertion if brace expansion notation has spaces', function (ge
var func;
var moduleOpts = {
setup() {
originalLookup = Ember.lookup;
lookup = Ember.lookup = {};

obj = {
foo: {
bar: {
Expand All @@ -457,18 +453,6 @@ var moduleOpts = {
}
};

Global = {
foo: {
bar: {
baz: {
biff: 'BIFF'
}
}
}
};

lookup['Global'] = Global;

count = 0;
func = function() {
count++;
Expand All @@ -477,8 +461,7 @@ var moduleOpts = {
},

teardown() {
obj = count = func = Global = null;
Ember.lookup = originalLookup;
obj = count = func = null;
}
};

Expand Down Expand Up @@ -528,52 +511,6 @@ testBoth('depending on simple chain', function(get, set) {
equal(count, 8, 'should be not have invoked computed again');
});

testBoth('depending on Global chain', function(get, set) {
// assign computed property
defineProperty(obj, 'prop', computed(function() {
count++;
return get('Global.foo.bar.baz.biff') + ' ' + count;
}).property('Global.foo.bar.baz.biff'));

equal(get(obj, 'prop'), 'BIFF 1');

set(get(Global, 'foo.bar.baz'), 'biff', 'BUZZ');
equal(get(obj, 'prop'), 'BUZZ 2');
equal(get(obj, 'prop'), 'BUZZ 2');

set(get(Global, 'foo.bar'), 'baz', { biff: 'BLOB' });
equal(get(obj, 'prop'), 'BLOB 3');
equal(get(obj, 'prop'), 'BLOB 3');

set(get(Global, 'foo.bar.baz'), 'biff', 'BUZZ');
equal(get(obj, 'prop'), 'BUZZ 4');
equal(get(obj, 'prop'), 'BUZZ 4');

set(get(Global, 'foo'), 'bar', { baz: { biff: 'BOOM' } });
equal(get(obj, 'prop'), 'BOOM 5');
equal(get(obj, 'prop'), 'BOOM 5');

set(get(Global, 'foo.bar.baz'), 'biff', 'BUZZ');
equal(get(obj, 'prop'), 'BUZZ 6');
equal(get(obj, 'prop'), 'BUZZ 6');

set(Global, 'foo', { bar: { baz: { biff: 'BLARG' } } });
equal(get(obj, 'prop'), 'BLARG 7');
equal(get(obj, 'prop'), 'BLARG 7');

set(get(Global, 'foo.bar.baz'), 'biff', 'BUZZ');
equal(get(obj, 'prop'), 'BUZZ 8');
equal(get(obj, 'prop'), 'BUZZ 8');

defineProperty(obj, 'prop');
set(obj, 'prop', 'NONE');
equal(get(obj, 'prop'), 'NONE');

set(Global, 'foo', { bar: { baz: { biff: 'BLARG' } } });
equal(get(obj, 'prop'), 'NONE'); // should do nothing
equal(count, 8, 'should be not have invoked computed again');
});

testBoth('chained dependent keys should evaluate computed properties lazily', function(get, set) {
defineProperty(obj.foo.bar, 'b', computed(func));
defineProperty(obj.foo, 'c', computed(function() {}).property('bar.b'));
Expand Down
31 changes: 1 addition & 30 deletions packages/ember-runtime/tests/legacy_1x/mixins/observable/observable_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,29 +155,10 @@ QUnit.test('raise if the provided object is undefined', function() {
});

QUnit.test('should work when object is Ember (used in Ember.get)', function() {
equal(get('Ember.RunLoop'), Ember.RunLoop, 'Ember.get');
equal(get(Ember, 'RunLoop'), Ember.RunLoop, 'Ember.get(Ember, RunLoop)');
});

QUnit.module('Ember.get() with paths', {
setup() {
lookup = Ember.lookup = {};
},

teardown() {
Ember.lookup = originalLookup;
}
});

QUnit.test('should return a property at a given path relative to the lookup', function() {
lookup.Foo = ObservableObject.extend({
Bar: ObservableObject.extend({
Baz: computed(function() { return 'blargh'; }).volatile()
}).create()
}).create();

equal(get('Foo.Bar.Baz'), 'blargh');
});
QUnit.module('Ember.get() with paths');

QUnit.test('should return a property at a given path relative to the passed object', function() {
var foo = ObservableObject.create({
Expand All @@ -189,16 +170,6 @@ QUnit.test('should return a property at a given path relative to the passed obje
equal(get(foo, 'bar.baz'), 'blargh');
});

QUnit.test('should return a property at a given path relative to the lookup - JavaScript hash', function() {
lookup.Foo = {
Bar: {
Baz: 'blargh'
}
};

equal(get('Foo.Bar.Baz'), 'blargh');
});

QUnit.test('should return a property at a given path relative to the passed object - JavaScript hash', function() {
var foo = {
bar: {
Expand Down
2 changes: 1 addition & 1 deletion packages/ember-views/lib/streams/should_display.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export default function shouldDisplay(predicate) {

if (type === 'boolean') { return predicate; }

if (type && type === 'object') {
if (type && type === 'object' && predicate !== null) {
var isTruthy = get(predicate, 'isTruthy');
if (typeof isTruthy === 'boolean') {
return isTruthy;
Expand Down

0 comments on commit 6413ad8

Please sign in to comment.