diff --git a/CHANGELOG.md b/CHANGELOG.md index d37b7252fff..6e7842285e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Ember Changelog +### Canary + +- [#3852](https://github.com/emberjs/ember.js/pull/3852) [BREAKING BUGFIX] Do not assume null Ember.get targets always refer to a global + ### 1.11.0-beta.1 (February 06, 2015) - [#10160](https://github.com/emberjs/ember.js/pull/10160) [FEATURE] Add index as an optional parameter to #each blocks [@tim-evans](https://github.com/tim-evans) diff --git a/packages/ember-metal/lib/path_cache.js b/packages/ember-metal/lib/path_cache.js index 168d3a4cf2e..243a489cf2a 100644 --- a/packages/ember-metal/lib/path_cache.js +++ b/packages/ember-metal/lib/path_cache.js @@ -1,7 +1,7 @@ import Cache from 'ember-metal/cache'; -var IS_GLOBAL = /^([A-Z$]|([0-9][A-Z$]))/; -var IS_GLOBAL_PATH = /^([A-Z$]|([0-9][A-Z$])).*[\.]/; +var IS_GLOBAL = /^[A-Z$]/; +var IS_GLOBAL_PATH = /^[A-Z$].*[\.]/; var HAS_THIS = 'this.'; var isGlobalCache = new Cache(1000, function(key) { return IS_GLOBAL.test(key); }); diff --git a/packages/ember-metal/lib/property_get.js b/packages/ember-metal/lib/property_get.js index f3a341d8db6..f9273ceaa9e 100644 --- a/packages/ember-metal/lib/property_get.js +++ b/packages/ember-metal/lib/property_get.js @@ -5,7 +5,7 @@ import Ember from "ember-metal/core"; import EmberError from "ember-metal/error"; import { - isGlobalPath, + isGlobal as detectIsGlobal, isPath, hasThis as pathHasThis } from "ember-metal/path_cache"; @@ -52,19 +52,14 @@ export function get(obj, keyName) { if (!keyName && 'string' === typeof obj) { keyName = obj; - obj = null; + 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 (obj === null) { - var value = _getPath(obj, keyName); - Ember.deprecate( - "Ember.get fetched '"+keyName+"' from the global context. This behavior will change in the future (issue #3852)", - !value || (obj && obj !== Ember.lookup) || isPath(keyName) || isGlobalPath(keyName+".") // Add a . to ensure simple paths are matched. - ); - return value; + if (!obj) { + return _getPath(obj, keyName); } var meta = obj['__ember_meta__']; @@ -113,46 +108,42 @@ export function get(obj, keyName) { */ export function normalizeTuple(target, path) { var hasThis = pathHasThis(path); - var isGlobal = !hasThis && isGlobalPath(path); + var isGlobal = !hasThis && detectIsGlobal(path); var key; - if (!target || isGlobal) { - target = Ember.lookup; + if (!target && !isGlobal) { + return [undefined, '']; } if (hasThis) { path = path.slice(5); } - Ember.deprecate( - "normalizeTuple will return '"+path+"' as a non-global. This behavior will change in the future (issue #3852)", - target === Ember.lookup || !target || hasThis || isGlobal || !isGlobalPath(path+'.') - ); + if (!target || isGlobal) { + target = Ember.lookup; + } - if (target === Ember.lookup) { + if (isGlobal && isPath(path)) { key = path.match(FIRST_KEY)[0]; target = get(target, key); path = path.slice(key.length+1); } // must return some kind of path to be valid else other things will break. - if (!path || path.length===0) { - throw new EmberError('Path cannot be empty'); - } + validateIsPath(path); return [target, path]; } +function validateIsPath(path) { + if (!path || path.length===0) { + throw new EmberError('Object in path '+path+' could not be found or was destroyed.'); + } +} + export function _getPath(root, path) { var hasThis, parts, tuple, idx, len; - // If there is no root and path is a key name, return that - // property from the global object. - // E.g. get('Ember') -> Ember - if (root === null && !isPath(path)) { - return get(Ember.lookup, path); - } - // detect complicated paths and normalize them hasThis = pathHasThis(path); diff --git a/packages/ember-metal/lib/property_set.js b/packages/ember-metal/lib/property_set.js index 38c9428b390..f2c8084d865 100644 --- a/packages/ember-metal/lib/property_set.js +++ b/packages/ember-metal/lib/property_set.js @@ -7,12 +7,11 @@ import { import { defineProperty } from "ember-metal/properties"; import EmberError from "ember-metal/error"; import { - isPath + isPath, + isGlobalPath } from "ember-metal/path_cache"; import { hasPropertyAccessors } from "ember-metal/platform/define_property"; -var IS_GLOBAL = /^([A-Z$]|([0-9][A-Z$]))/; - /** Sets the value of a property on an object, respecting computed properties and notifying observers and other listeners of the change. If the @@ -28,25 +27,27 @@ var IS_GLOBAL = /^([A-Z$]|([0-9][A-Z$]))/; */ export function set(obj, keyName, value, tolerant) { if (typeof obj === 'string') { - Ember.assert("Path '" + obj + "' must be global if no obj is given.", IS_GLOBAL.test(obj)); + Ember.assert("Path '" + obj + "' must be global if no obj is given.", isGlobalPath(obj)); value = keyName; keyName = obj; - obj = null; + obj = Ember.lookup; } Ember.assert("Cannot call set with "+ keyName +" key.", !!keyName); - if (!obj) { + if (obj === Ember.lookup) { return setPath(obj, keyName, value, tolerant); } - var meta = obj['__ember_meta__']; - var possibleDesc = obj[keyName]; - var desc = (possibleDesc !== null && typeof possibleDesc === 'object' && possibleDesc.isDescriptor) ? possibleDesc : undefined; + var meta, possibleDesc, desc; + if (obj) { + meta = obj['__ember_meta__']; + possibleDesc = obj[keyName]; + desc = (possibleDesc !== null && typeof possibleDesc === 'object' && possibleDesc.isDescriptor) ? possibleDesc : undefined; + } var isUnknown, currentValue; - - if (desc === undefined && isPath(keyName)) { + if ((!obj || desc === undefined) && isPath(keyName)) { return setPath(obj, keyName, value, tolerant); } @@ -57,7 +58,7 @@ export function set(obj, keyName, value, tolerant) { desc.set(obj, keyName, value); } else { - if (typeof obj === 'object' && obj !== null && value !== undefined && obj[keyName] === value) { + if (obj !== null && value !== undefined && typeof obj === 'object' && obj[keyName] === value) { return value; } diff --git a/packages/ember-metal/tests/accessors/get_path_test.js b/packages/ember-metal/tests/accessors/get_path_test.js index 3ef9debca77..c8141e38144 100644 --- a/packages/ember-metal/tests/accessors/get_path_test.js +++ b/packages/ember-metal/tests/accessors/get_path_test.js @@ -2,13 +2,6 @@ import { get } from 'ember-metal/property_get'; -function expectGlobalContextDeprecation(assertion) { - expectDeprecation( - assertion, - "Ember.get fetched 'localPathGlobal' from the global context. This behavior will change in the future (issue #3852)" - ); -} - var obj; var moduleOpts = { setup: function() { @@ -23,7 +16,10 @@ var moduleOpts = { baz: { biff: 'BIFF' } } }, - falseValue: false + falseValue: false, + Wuz: { + nar: 'foo' + } }; window.Foo = { @@ -32,20 +28,20 @@ var moduleOpts = { } }; + window.aProp = 'aPropy'; + window.$foo = { bar: { baz: { biff: '$FOOBIFF' } } }; - - window.localPathGlobal = 5; }, teardown: function() { obj = undefined; window.Foo = undefined; + window.aProp = undefined; window.$foo = undefined; - window.localPathGlobal = undefined; } }; @@ -75,39 +71,66 @@ 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] -> (null)', function() { - deepEqual(get(obj, 'this.Foo.bar'), undefined); +QUnit.test('[obj, this.Foo.bar] -> (undefined)', function() { + equal(get(obj, 'this.Foo.bar'), undefined); }); -QUnit.test('[obj, falseValue.notDefined] -> (null)', function() { - deepEqual(get(obj, 'falseValue.notDefined'), undefined); +QUnit.test('[obj, falseValue.notDefined] -> (undefined)', function() { + equal(get(obj, 'falseValue.notDefined'), undefined); }); // .......................................................... -// LOCAL PATHS WITH NO TARGET DEPRECATION +// GLOBAL PATHS TREATED LOCAL WITH GET // -QUnit.test('[null, length] returning data is deprecated', function() { - expectGlobalContextDeprecation(function() { - equal(5, get(null, 'localPathGlobal')); - }); +QUnit.test('[obj, Wuz] -> obj.Wuz', function() { + deepEqual(get(obj, 'Wuz'), obj.Wuz); }); -QUnit.test('[length] returning data is deprecated', function() { - expectGlobalContextDeprecation(function() { - equal(5, get('localPathGlobal')); - }); +QUnit.test('[obj, Wuz.nar] -> obj.Wuz.nar', function() { + deepEqual(get(obj, 'Wuz.nar'), obj.Wuz.nar); +}); + +QUnit.test('[obj, Foo] -> (undefined)', function() { + equal(get(obj, 'Foo'), undefined); +}); + +QUnit.test('[obj, Foo.bar] -> (undefined)', function() { + equal(get(obj, 'Foo.bar'), undefined); }); // .......................................................... -// NO TARGET +// NULL TARGET // QUnit.test('[null, Foo] -> Foo', function() { - deepEqual(get('Foo'), Foo); + equal(get(null, 'Foo'), Foo); }); QUnit.test('[null, Foo.bar] -> Foo.bar', function() { - deepEqual(get('Foo.bar'), Foo.bar); + 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); +}); diff --git a/packages/ember-metal/tests/accessors/get_test.js b/packages/ember-metal/tests/accessors/get_test.js index 1187d22a400..cd9c3babb8d 100644 --- a/packages/ember-metal/tests/accessors/get_test.js +++ b/packages/ember-metal/tests/accessors/get_test.js @@ -60,6 +60,14 @@ 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('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 falsy property', function() { var obj = {}; expectAssertion(function() { diff --git a/packages/ember-metal/tests/accessors/normalize_tuple_test.js b/packages/ember-metal/tests/accessors/normalize_tuple_test.js index 74aba6dea09..4d751cf76af 100644 --- a/packages/ember-metal/tests/accessors/normalize_tuple_test.js +++ b/packages/ember-metal/tests/accessors/normalize_tuple_test.js @@ -74,10 +74,8 @@ QUnit.test('[obj, this.Foo.bar] -> [obj, Foo.bar]', function() { // GLOBAL PATHS // -QUnit.test('[obj, Foo] -> [obj, Foo]', function() { - expectDeprecation(function() { - deepEqual(normalizeTuple(obj, 'Foo'), [obj, 'Foo']); - }, "normalizeTuple will return 'Foo' as a non-global. This behavior will change in the future (issue #3852)"); +QUnit.test('[obj, Foo] -> [Ember.lookup, Foo]', function() { + deepEqual(normalizeTuple(obj, 'Foo'), [Ember.lookup, 'Foo']); }); QUnit.test('[obj, Foo.bar] -> [Foo, bar]', function() { @@ -92,12 +90,26 @@ QUnit.test('[obj, $foo.bar.baz] -> [$foo, bar.baz]', function() { // NO TARGET // -QUnit.test('[null, Foo] -> EXCEPTION', function() { - throws(function() { - normalizeTuple(null, 'Foo'); - }, Error); +QUnit.test('[null, Foo] -> [Ember.lookup, Foo]', function() { + deepEqual(normalizeTuple(null, 'Foo'), [Ember.lookup, 'Foo']); }); QUnit.test('[null, Foo.bar] -> [Foo, bar]', function() { deepEqual(normalizeTuple(null, 'Foo.bar'), [Foo, 'bar']); }); + +QUnit.test("[null, foo] -> [undefined, '']", function() { + deepEqual(normalizeTuple(null, 'foo'), [undefined, '']); +}); + +QUnit.test("[null, foo.bar] -> [undefined, '']", function() { + deepEqual(normalizeTuple(null, 'foo'), [undefined, '']); +}); + +QUnit.test('[null, $foo] -> [Ember.lookup, $foo]', function() { + deepEqual(normalizeTuple(null, '$foo'), [Ember.lookup, '$foo']); +}); + +QUnit.test('[null, $foo.bar] -> [$foo, bar]', function() { + deepEqual(normalizeTuple(null, '$foo.bar'), [$foo, 'bar']); +}); diff --git a/packages/ember-metal/tests/accessors/set_path_test.js b/packages/ember-metal/tests/accessors/set_path_test.js index 5dad96f8362..fb4f44c9532 100644 --- a/packages/ember-metal/tests/accessors/set_path_test.js +++ b/packages/ember-metal/tests/accessors/set_path_test.js @@ -91,12 +91,9 @@ QUnit.module("set with path - deprecated", { }); QUnit.test('[null, bla] gives a proper exception message', function() { - var exceptionMessage = 'Property set failed: object in path \"bla\" could not be found or was destroyed.'; - try { + expectAssertion(function() { set(null, 'bla', "BAM"); - } catch(ex) { - equal(ex.message, exceptionMessage); - } + }, /You need to provide an object and key to `set`/); }); QUnit.test('[obj, bla.bla] gives a proper exception message', function() {