From a0103c1be975b751cd80976a791a96cd93f7191e Mon Sep 17 00:00:00 2001 From: Marten Schilstra Date: Wed, 15 Jul 2015 22:05:30 +0200 Subject: [PATCH] [CLEANUP beta] Cleanup Ember.set - Removes support for set with global paths - Removes support for set with 'this' paths - Removes support for set with null as first parameter - Path must be a string - Requires set to be passed in three or four arguments --- packages/ember-metal/lib/property_set.js | 25 ++++------- .../tests/accessors/set_path_test.js | 42 ------------------- .../ember-metal/tests/accessors/set_test.js | 30 +++++++++++++ 3 files changed, 39 insertions(+), 58 deletions(-) diff --git a/packages/ember-metal/lib/property_set.js b/packages/ember-metal/lib/property_set.js index 29aa1a6c328..0cfd218cea5 100644 --- a/packages/ember-metal/lib/property_set.js +++ b/packages/ember-metal/lib/property_set.js @@ -10,7 +10,7 @@ import { defineProperty } from 'ember-metal/properties'; import EmberError from 'ember-metal/error'; import { isPath, - isGlobalPath + hasThis as pathHasThis } from 'ember-metal/path_cache'; import { symbol } from 'ember-metal/utils'; @@ -32,18 +32,12 @@ export let UNHANDLED_SET = symbol('UNHANDLED_SET'); @public */ 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)); - value = keyName; - keyName = obj; - obj = Ember.lookup; - } - - Ember.assert(`Cannot call set with '${keyName}' key.`, !!keyName); - - if (obj === Ember.lookup) { - return setPath(obj, keyName, value, tolerant); - } + Ember.assert( + `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}' on an undefined object.`, obj !== undefined && obj !== null); + Ember.assert(`The key provided to set must be a string, you passed ${keyName}`, typeof keyName === 'string'); + Ember.assert(`'this' in paths is not supported`, !pathHasThis(keyName)); // This path exists purely to implement backwards-compatible // effects (specifically, setting a property on a view may @@ -61,17 +55,16 @@ export function set(obj, keyName, value, tolerant) { } var isUnknown, currentValue; - if ((!obj || desc === undefined) && isPath(keyName)) { + if (desc === undefined && isPath(keyName)) { return setPath(obj, keyName, value, tolerant); } - Ember.assert('You need to provide an object and key to `set`.', !!obj && keyName !== undefined); Ember.assert('calling set on destroyed object', !obj.isDestroyed); if (desc) { desc.set(obj, keyName, value); } else { - if (obj !== null && value !== undefined && typeof obj === 'object' && obj[keyName] === value) { + if (value !== undefined && typeof obj === 'object' && obj[keyName] === value) { return value; } diff --git a/packages/ember-metal/tests/accessors/set_path_test.js b/packages/ember-metal/tests/accessors/set_path_test.js index 9c4ef777cb1..2289fb3ae00 100644 --- a/packages/ember-metal/tests/accessors/set_path_test.js +++ b/packages/ember-metal/tests/accessors/set_path_test.js @@ -2,8 +2,6 @@ import Ember from 'ember-metal/core'; import { set, trySet } from 'ember-metal/property_set'; import { get } from 'ember-metal/property_get'; -var originalLookup = Ember.lookup; - var obj; function commonSetup() { obj = { @@ -13,25 +11,10 @@ function commonSetup() { } } }; - - Ember.lookup = { - Foo: { - bar: { - baz: { biff: 'FooBiff' } - } - }, - - $foo: { - bar: { - baz: { biff: '$FOOBIFF' } - } - } - }; } function commonTeardown() { obj = null; - Ember.lookup = originalLookup; } QUnit.module('set with path', { @@ -60,25 +43,6 @@ QUnit.test('[obj, foo.bar] -> obj.foo.bar', function() { equal(get(obj, 'foo.bar'), 'BAM'); }); -QUnit.test('[obj, this.foo] -> obj.foo', function() { - set(obj, 'this.foo', 'BAM'); - equal(get(obj, 'foo'), 'BAM'); -}); - -QUnit.test('[obj, this.foo.bar] -> obj.foo.bar', function() { - set(obj, 'this.foo.bar', 'BAM'); - equal(get(obj, 'foo.bar'), 'BAM'); -}); - -// .......................................................... -// NO TARGET -// - -QUnit.test('[null, Foo.bar] -> Foo.bar', function() { - set(null, 'Foo.bar', 'BAM'); - equal(get(Ember.lookup.Foo, 'bar'), 'BAM'); -}); - // .......................................................... // DEPRECATED // @@ -88,12 +52,6 @@ QUnit.module('set with path - deprecated', { teardown: commonTeardown }); -QUnit.test('[null, bla] gives a proper exception message', function() { - expectAssertion(function() { - set(null, 'bla', 'BAM'); - }, /You need to provide an object and key to `set`/); -}); - QUnit.test('[obj, bla.bla] gives a proper exception message', function() { var exceptionMessage = 'Property set failed: object in path \"bla\" could not be found or was destroyed.'; try { diff --git a/packages/ember-metal/tests/accessors/set_test.js b/packages/ember-metal/tests/accessors/set_test.js index 55173e4d4cf..dddee1c0de6 100644 --- a/packages/ember-metal/tests/accessors/set_test.js +++ b/packages/ember-metal/tests/accessors/set_test.js @@ -156,3 +156,33 @@ QUnit.test('should call setUnknownProperty if defined and value is undefined', f equal(obj.count, 1, 'should have invoked'); }); +QUnit.test('warn on attempts to call set with undefined as object', function() { + expectAssertion(function() { + set(undefined, 'aProperty', 'BAM'); + }, /Cannot call set with 'aProperty' on an undefined object./); +}); + +QUnit.test('warn on attempts to call set with null as object', function() { + expectAssertion(function() { + set(null, 'aProperty', 'BAM'); + }, /Cannot call set with 'aProperty' on an undefined object./); +}); + +QUnit.test('warn on attempts to use set with an unsupported property path', function() { + var obj = {}; + expectAssertion(function() { + set(obj, null, 42); + }, /The key provided to set must be a string, you passed null/); + expectAssertion(function() { + set(obj, NaN, 42); + }, /The key provided to set must be a string, you passed NaN/); + expectAssertion(function() { + set(obj, undefined, 42); + }, /The key provided to set must be a string, you passed undefined/); + expectAssertion(function() { + set(obj, false, 42); + }, /The key provided to set must be a string, you passed false/); + expectAssertion(function() { + set(obj, 42, 42); + }, /The key provided to set must be a string, you passed 42/); +});