From 77c03bedb91e9934e7e8c069737e3899293dd46b Mon Sep 17 00:00:00 2001 From: Chris Ng Date: Thu, 28 May 2020 17:38:34 -0400 Subject: [PATCH 1/2] [FEATURE deprecate-get-with-default] Deprecate getWithDefault based on RFC 0554 --- .../-internals/metal/lib/property_get.ts | 12 +- .../metal/tests/accessors/get_test.js | 120 +++++++++--------- 2 files changed, 74 insertions(+), 58 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/property_get.ts b/packages/@ember/-internals/metal/lib/property_get.ts index feb503394f9..b4413397525 100644 --- a/packages/@ember/-internals/metal/lib/property_get.ts +++ b/packages/@ember/-internals/metal/lib/property_get.ts @@ -2,7 +2,7 @@ @module @ember/object */ import { HAS_NATIVE_PROXY, isEmberArray, isProxy, symbol } from '@ember/-internals/utils'; -import { assert } from '@ember/debug'; +import { assert, deprecate } from '@ember/debug'; import { DEBUG } from '@glimmer/env'; import { consumeTag, @@ -181,12 +181,22 @@ export function _getPath(root: T, path: string | string[]): an @param {Object} defaultValue The value to return if the property value is undefined @return {Object} The property value or the defaultValue. @public + @deprecated */ export function getWithDefault>( root: T, key: K, defaultValue: T[K] ): T[K] { + deprecate( + 'Using getWithDefault has been deprecated. Instead, consider using Ember get and explicitly checking for undefined.', + false, + { + id: 'ember-metal.get-with-default', + until: '4.0.0', + } + ); + let value = get(root, key); if (value === undefined) { diff --git a/packages/@ember/-internals/metal/tests/accessors/get_test.js b/packages/@ember/-internals/metal/tests/accessors/get_test.js index 9765be3872a..30596be0db0 100644 --- a/packages/@ember/-internals/metal/tests/accessors/get_test.js +++ b/packages/@ember/-internals/metal/tests/accessors/get_test.js @@ -207,35 +207,37 @@ moduleFor( 'getWithDefault', class extends AbstractTestCase { ['@test should get arbitrary properties on an object'](assert) { - let obj = { - string: 'string', - number: 23, - boolTrue: true, - boolFalse: false, - nullValue: null, - }; + expectDeprecation(() => { + let obj = { + string: 'string', + number: 23, + boolTrue: true, + boolFalse: false, + nullValue: null, + }; - for (let key in obj) { - if (!Object.prototype.hasOwnProperty.call(obj, key)) { - continue; + for (let key in obj) { + if (!Object.prototype.hasOwnProperty.call(obj, key)) { + continue; + } + assert.equal(getWithDefault(obj, key, 'fail'), obj[key], key); } - assert.equal(getWithDefault(obj, key, 'fail'), obj[key], key); - } - obj = { - undef: undefined, - }; + obj = { + undef: undefined, + }; - assert.equal( - getWithDefault(obj, 'undef', 'default'), - 'default', - 'explicit undefined retrieves the default' - ); - assert.equal( - getWithDefault(obj, 'not-present', 'default'), - 'default', - 'non-present key retrieves the default' - ); + assert.equal( + getWithDefault(obj, 'undef', 'default'), + 'default', + 'explicit undefined retrieves the default' + ); + assert.equal( + getWithDefault(obj, 'not-present', 'default'), + 'default', + 'non-present key retrieves the default' + ); + }, /Using getWithDefault has been deprecated. Instead, consider using Ember get and explicitly checking for undefined./); } ['@test should call unknownProperty if defined and value is undefined'](assert) { @@ -253,24 +255,26 @@ moduleFor( } ['@test if unknownProperty is present, it is called using getFromEmberMetal()/set()'](assert) { - let obj = { - unknownProperty(key) { - if (key === 'foo') { - assert.equal(key, 'foo', 'should pass key'); - return 'FOO'; - } - }, - }; - assert.equal( - getWithDefault(obj, 'foo', 'fail'), - 'FOO', - 'should return value from unknownProperty' - ); - assert.equal( - getWithDefault(obj, 'bar', 'default'), - 'default', - 'should convert undefined from unknownProperty into default' - ); + expectDeprecation(() => { + let obj = { + unknownProperty(key) { + if (key === 'foo') { + assert.equal(key, 'foo', 'should pass key'); + return 'FOO'; + } + }, + }; + assert.equal( + getWithDefault(obj, 'foo', 'fail'), + 'FOO', + 'should return value from unknownProperty' + ); + assert.equal( + getWithDefault(obj, 'bar', 'default'), + 'default', + 'should convert undefined from unknownProperty into default' + ); + }, /Using getWithDefault has been deprecated. Instead, consider using Ember get and explicitly checking for undefined./); } ['@test if unknownProperty is present, it is called using accessors'](assert) { @@ -320,23 +324,25 @@ moduleFor( ['@test (regression) watched properties on unmodified inherited objects should still return their original value']( assert ) { - let MyMixin = Mixin.create({ - someProperty: 'foo', - propertyDidChange: observer('someProperty', () => { - /* nothing to do */ - }), - }); + expectDeprecation(() => { + let MyMixin = Mixin.create({ + someProperty: 'foo', + propertyDidChange: observer('someProperty', () => { + /* nothing to do */ + }), + }); - let baseObject = MyMixin.apply({}); - let theRealObject = Object.create(baseObject); + let baseObject = MyMixin.apply({}); + let theRealObject = Object.create(baseObject); - assert.equal( - getWithDefault(theRealObject, 'someProperty', 'fail'), - 'foo', - 'should return the set value, not false' - ); + assert.equal( + getWithDefault(theRealObject, 'someProperty', 'fail'), + 'foo', + 'should return the set value, not false' + ); - run(() => destroy(baseObject)); + run(() => destroy(baseObject)); + }, /Using getWithDefault has been deprecated. Instead, consider using Ember get and explicitly checking for undefined./); } ['@test should respect prototypical inheritance when subclasses override CPs'](assert) { From f2d836404616a942bdbd2a5c3818009f094fbd34 Mon Sep 17 00:00:00 2001 From: Chris Ng Date: Thu, 28 May 2020 18:54:31 -0400 Subject: [PATCH 2/2] Add url for deprecate getWithDefault, Add expectDeprecation for get_test and computed_test, Add docs for getWithDefault in observable --- .../-internals/metal/lib/property_get.ts | 1 + .../metal/tests/tracked/get_test.js | 38 ++++++++++--------- .../runtime/lib/mixins/observable.js | 1 + .../tests/system/object/computed_test.js | 6 ++- 4 files changed, 26 insertions(+), 20 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/property_get.ts b/packages/@ember/-internals/metal/lib/property_get.ts index b4413397525..4f891b6c3c2 100644 --- a/packages/@ember/-internals/metal/lib/property_get.ts +++ b/packages/@ember/-internals/metal/lib/property_get.ts @@ -194,6 +194,7 @@ export function getWithDefault { + let obj = createObj(); - for (let key in obj) { - this.assert.equal(getWithDefault(obj, key, 'fail'), obj[key], key); - } + for (let key in obj) { + this.assert.equal(getWithDefault(obj, key, 'fail'), obj[key], key); + } - class Obj { - @tracked undef = undefined; - } + class Obj { + @tracked undef = undefined; + } - let obj2 = new Obj(); + let obj2 = new Obj(); - this.assert.equal( - getWithDefault(obj2, 'undef', 'default'), - 'default', - 'explicit undefined retrieves the default' - ); - this.assert.equal( - getWithDefault(obj2, 'not-present', 'default'), - 'default', - 'non-present key retrieves the default' - ); + this.assert.equal( + getWithDefault(obj2, 'undef', 'default'), + 'default', + 'explicit undefined retrieves the default' + ); + this.assert.equal( + getWithDefault(obj2, 'not-present', 'default'), + 'default', + 'non-present key retrieves the default' + ); + }, /Using getWithDefault has been deprecated. Instead, consider using Ember get and explicitly checking for undefined./); } } ); diff --git a/packages/@ember/-internals/runtime/lib/mixins/observable.js b/packages/@ember/-internals/runtime/lib/mixins/observable.js index 06c8e59cf5b..840bda8f4ca 100644 --- a/packages/@ember/-internals/runtime/lib/mixins/observable.js +++ b/packages/@ember/-internals/runtime/lib/mixins/observable.js @@ -417,6 +417,7 @@ export default Mixin.create({ @param {Object} defaultValue The value to return if the property value is undefined @return {Object} The property value or the defaultValue. @public + @deprecated */ getWithDefault(keyName, defaultValue) { return getWithDefault(this, keyName, defaultValue); diff --git a/packages/@ember/-internals/runtime/tests/system/object/computed_test.js b/packages/@ember/-internals/runtime/tests/system/object/computed_test.js index 03ed6291321..9eeecd27211 100644 --- a/packages/@ember/-internals/runtime/tests/system/object/computed_test.js +++ b/packages/@ember/-internals/runtime/tests/system/object/computed_test.js @@ -18,8 +18,10 @@ function K() { function testWithDefault(assert, expect, x, y, z) { assert.equal(get(x, y), expect); - assert.equal(getWithDefault(x, y, z), expect); - assert.equal(x.getWithDefault(y, z), expect); + expectDeprecation(() => { + assert.equal(getWithDefault(x, y, z), expect); + assert.equal(x.getWithDefault(y, z), expect); + }, /Using getWithDefault has been deprecated. Instead, consider using Ember get and explicitly checking for undefined./); } moduleFor(