From e3204ac06e154d97ca2d2d437e26a418558f4f7b Mon Sep 17 00:00:00 2001 From: Chris Ng Date: Fri, 8 Nov 2019 15:53:34 -0500 Subject: [PATCH 01/12] RFC Deprecate getWithDefault --- text/0000-deprecate-getwithdefault.md | 71 +++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 text/0000-deprecate-getwithdefault.md diff --git a/text/0000-deprecate-getwithdefault.md b/text/0000-deprecate-getwithdefault.md new file mode 100644 index 0000000000..81d2f12bb1 --- /dev/null +++ b/text/0000-deprecate-getwithdefault.md @@ -0,0 +1,71 @@ +- Start Date: (fill me in with today's date, YYYY-MM-DD) +- Relevant Team(s): Ember.js +- RFC PR: (after opening the RFC PR, update this with a link to it and update the file name) +- Tracking: (leave this empty) + +# Deprecate getWithDefault + +## Summary + +Deprecate support for `getWithDefault` in Ember's Object module (@ember/object) because its expected behaviour is confusing to Ember developers. + +## Motivation + +The problem with `getWithDefault` is that its behaviour is confusing to Ember developers. The API will only return the default value when the value of the property retrieved is `undefined`. This behaviour is often overlooked when using the function where a developer might expect that `null` or other falsy values will also return the default value. + +Given the JavaScript language will soon (currently in Stage 3) give us the appropriate tool for this use case using the [Nullish Coalescing Operator `??`](https://github.com/tc39/proposal-nullish-coalescing), we can deprecate usage of `getWithDefault` and use that instead. + +## Transition Path + +Ember will start logging deprecation messages for `getWithDefault` usage. + +We can codemod our current usage of `getWithDefault` with the equivalent behaviour using plain JavaScript. The migration guide will cover this example: + +Before: + +```js +let result = getWithDefault(obj, ‘some.key', defaultValue); +``` + +After: + +```js +let result = get(obj, 'some.key'); +if (result === undefined) { + result = defaultValue; +} +``` + +### Nullish Coalescing Operator + +The expected behaviour of the function is to only return the default value if it is strictly `undefined` so we cannot codemod usage directly into the nullish coalescing operator. + +However [Babel](https://babeljs.io/) already supports the [nullish coalescing operator](https://babeljs.io/docs/en/next/babel-plugin-proposal-nullish-coalescing-operator.html) so we can use that for future use cases where we need to check if a property is `null` or `undefined` before applying a default value. + +Similarly [TypeScript](https://github.com/microsoft/TypeScript) as of [version 3.7](https://devblogs.microsoft.com/typescript/announcing-typescript-3-7/#nullish-coalescing) also supports the operator so we will not be breaking that flow either. + +## How We Teach This + +Add the transition path to the [Ember Deprecation Guide](https://deprecations.emberjs.com/). + +The references to `getWithDefault` will need to be removed from the [API docs](https://api.emberjs.com/ember/release/functions/@ember%2Fobject/getWithDefault). + +There are no changes needed for the [Ember Guides](https://guides.emberjs.com/release/) since we do not use it anywhere. + +## Drawbacks + +The downside to deprecating `getWithDefault` would be an increase to the line length of component files that use it. This change will also cause some deprecation noise but could be mitigated with a codemod. + +## Alternatives + +### Adding `null` as a condition + +We could add `null` as a condition alongside `undefined` which would return the default value provided. This is similar to what is proposed in [Nullish Coalescing for JavaScript](https://github.com/tc39/proposal-nullish-coalescing). This would however still be a breaking change since people who are depending on it working the way it does for null today will be broken if we change it. + +### Do nothing + +We could keep support in place, and provide more guidance around using it. There are already [some](https://dockyard.com/blog/2016/03/18/get-with-default) of articles cautioning usage of `getWithDefault` when dealing with null or falsey values. + +## Unresolved questions + +None at the moment. From 9a71ea79c517efb467257571a9a45d840aa0ef73 Mon Sep 17 00:00:00 2001 From: Chris Ng Date: Fri, 8 Nov 2019 17:00:28 -0500 Subject: [PATCH 02/12] Update RFC PR --- text/0000-deprecate-getwithdefault.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-deprecate-getwithdefault.md b/text/0000-deprecate-getwithdefault.md index 81d2f12bb1..4502295ea0 100644 --- a/text/0000-deprecate-getwithdefault.md +++ b/text/0000-deprecate-getwithdefault.md @@ -1,6 +1,6 @@ - Start Date: (fill me in with today's date, YYYY-MM-DD) - Relevant Team(s): Ember.js -- RFC PR: (after opening the RFC PR, update this with a link to it and update the file name) +- RFC PR: https://github.com/emberjs/rfcs/pull/554 - Tracking: (leave this empty) # Deprecate getWithDefault From a5ae7a1ec48c14cc634e19107bb0298e80bdd300 Mon Sep 17 00:00:00 2001 From: Chris Ng Date: Fri, 8 Nov 2019 17:01:30 -0500 Subject: [PATCH 03/12] Update date --- text/0000-deprecate-getwithdefault.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-deprecate-getwithdefault.md b/text/0000-deprecate-getwithdefault.md index 4502295ea0..19e39e94aa 100644 --- a/text/0000-deprecate-getwithdefault.md +++ b/text/0000-deprecate-getwithdefault.md @@ -1,4 +1,4 @@ -- Start Date: (fill me in with today's date, YYYY-MM-DD) +- Start Date: 2019-11-08 - Relevant Team(s): Ember.js - RFC PR: https://github.com/emberjs/rfcs/pull/554 - Tracking: (leave this empty) From 7dddd82662d11135f48d2796b4c31fbc06f453c4 Mon Sep 17 00:00:00 2001 From: Chris Ng Date: Fri, 8 Nov 2019 17:58:00 -0500 Subject: [PATCH 04/12] Fix typos and formatting --- text/0000-deprecate-getwithdefault.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/text/0000-deprecate-getwithdefault.md b/text/0000-deprecate-getwithdefault.md index 19e39e94aa..6ac1172f2c 100644 --- a/text/0000-deprecate-getwithdefault.md +++ b/text/0000-deprecate-getwithdefault.md @@ -11,7 +11,7 @@ Deprecate support for `getWithDefault` in Ember's Object module (@ember/object) ## Motivation -The problem with `getWithDefault` is that its behaviour is confusing to Ember developers. The API will only return the default value when the value of the property retrieved is `undefined`. This behaviour is often overlooked when using the function where a developer might expect that `null` or other falsy values will also return the default value. +The problem with `getWithDefault` is that its behaviour is confusing to Ember developers. The API will only return the default value when the value of the property retrieved is `undefined`. This behaviour is often overlooked when using the function where a developer might expect that `null` or other _falsy_ values will also return the default value. Given the JavaScript language will soon (currently in Stage 3) give us the appropriate tool for this use case using the [Nullish Coalescing Operator `??`](https://github.com/tc39/proposal-nullish-coalescing), we can deprecate usage of `getWithDefault` and use that instead. @@ -38,7 +38,7 @@ if (result === undefined) { ### Nullish Coalescing Operator -The expected behaviour of the function is to only return the default value if it is strictly `undefined` so we cannot codemod usage directly into the nullish coalescing operator. +The expected behaviour of the `getWithDefault` function is to only return the default value if it is strictly `undefined` so we cannot codemod usage directly into the nullish coalescing operator. However [Babel](https://babeljs.io/) already supports the [nullish coalescing operator](https://babeljs.io/docs/en/next/babel-plugin-proposal-nullish-coalescing-operator.html) so we can use that for future use cases where we need to check if a property is `null` or `undefined` before applying a default value. @@ -60,11 +60,11 @@ The downside to deprecating `getWithDefault` would be an increase to the line le ### Adding `null` as a condition -We could add `null` as a condition alongside `undefined` which would return the default value provided. This is similar to what is proposed in [Nullish Coalescing for JavaScript](https://github.com/tc39/proposal-nullish-coalescing). This would however still be a breaking change since people who are depending on it working the way it does for null today will be broken if we change it. +We could add `null` as a condition alongside `undefined` which would return the default value provided. This is similar to what is proposed in [Nullish Coalescing for JavaScript](https://github.com/tc39/proposal-nullish-coalescing). This would however still be a breaking change since people who are depending on `getWithDefault` to work the way it does for `null` today will be broken if we change it. ### Do nothing -We could keep support in place, and provide more guidance around using it. There are already [some](https://dockyard.com/blog/2016/03/18/get-with-default) of articles cautioning usage of `getWithDefault` when dealing with null or falsey values. +We could keep support in place, and provide more guidance around using it. There are already [some](https://dockyard.com/blog/2016/03/18/get-with-default) articles cautioning usage of `getWithDefault` when dealing with `null` or _falsy_ values. ## Unresolved questions From c204d9840d4c6184740b06287d5afb3e6757b15a Mon Sep 17 00:00:00 2001 From: Chris Ng Date: Fri, 8 Nov 2019 18:20:28 -0500 Subject: [PATCH 05/12] Rename 0000-deprecate-getwithdefault.md to 0554-deprecate-getwithdefault.md --- ...precate-getwithdefault.md => 0554-deprecate-getwithdefault.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename text/{0000-deprecate-getwithdefault.md => 0554-deprecate-getwithdefault.md} (100%) diff --git a/text/0000-deprecate-getwithdefault.md b/text/0554-deprecate-getwithdefault.md similarity index 100% rename from text/0000-deprecate-getwithdefault.md rename to text/0554-deprecate-getwithdefault.md From f45dd40f0c31b726877ca0d4790c1ba5fe348005 Mon Sep 17 00:00:00 2001 From: Chris Ng Date: Fri, 8 Nov 2019 21:53:58 -0500 Subject: [PATCH 06/12] Update text/0554-deprecate-getwithdefault.md Co-Authored-By: Thomas Wang --- text/0554-deprecate-getwithdefault.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/text/0554-deprecate-getwithdefault.md b/text/0554-deprecate-getwithdefault.md index 6ac1172f2c..e0138df46c 100644 --- a/text/0554-deprecate-getwithdefault.md +++ b/text/0554-deprecate-getwithdefault.md @@ -30,6 +30,8 @@ let result = getWithDefault(obj, ‘some.key', defaultValue); After: ```js +import { get } from '@ember/object'; + let result = get(obj, 'some.key'); if (result === undefined) { result = defaultValue; From fffc32eee64147ee870cfeac3fadf8fe2964fef1 Mon Sep 17 00:00:00 2001 From: Chris Ng Date: Fri, 8 Nov 2019 21:54:05 -0500 Subject: [PATCH 07/12] Update text/0554-deprecate-getwithdefault.md Co-Authored-By: Thomas Wang --- text/0554-deprecate-getwithdefault.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/text/0554-deprecate-getwithdefault.md b/text/0554-deprecate-getwithdefault.md index e0138df46c..76e83019b8 100644 --- a/text/0554-deprecate-getwithdefault.md +++ b/text/0554-deprecate-getwithdefault.md @@ -24,6 +24,8 @@ We can codemod our current usage of `getWithDefault` with the equivalent behavio Before: ```js +import { getWithDefault } from '@ember/object'; + let result = getWithDefault(obj, ‘some.key', defaultValue); ``` From 95a8cdc681ee04cade1ef1bb5aea2f4b5b76d39c Mon Sep 17 00:00:00 2001 From: Chris Ng Date: Fri, 15 Nov 2019 12:25:33 -0500 Subject: [PATCH 08/12] Explicitly say both function and class methods Explicitly say both function and class methods are being deprecated --- text/0554-deprecate-getwithdefault.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0554-deprecate-getwithdefault.md b/text/0554-deprecate-getwithdefault.md index 76e83019b8..d134d69805 100644 --- a/text/0554-deprecate-getwithdefault.md +++ b/text/0554-deprecate-getwithdefault.md @@ -7,7 +7,7 @@ ## Summary -Deprecate support for `getWithDefault` in Ember's Object module (@ember/object) because its expected behaviour is confusing to Ember developers. +Deprecate support for `getWithDefault` in Ember's Object module (@ember/object) – both the [function](https://api.emberjs.com/ember/release/functions/@ember%2Fobject/getWithDefault) and the [class method](https://api.emberjs.com/ember/release/classes/EmberObject/methods/getWithDefault?anchor=getWithDefault) – because its expected behaviour is confusing to Ember developers. ## Motivation From 2e4416b4c61f6790a7bafa951591d6cbe4b80164 Mon Sep 17 00:00:00 2001 From: Chris Ng Date: Fri, 22 Nov 2019 17:02:01 -0500 Subject: [PATCH 09/12] Examples using nullish coalescing and obj dest --- text/0554-deprecate-getwithdefault.md | 63 ++++++++++++++++++++++++--- 1 file changed, 58 insertions(+), 5 deletions(-) diff --git a/text/0554-deprecate-getwithdefault.md b/text/0554-deprecate-getwithdefault.md index d134d69805..f84730ce7d 100644 --- a/text/0554-deprecate-getwithdefault.md +++ b/text/0554-deprecate-getwithdefault.md @@ -26,7 +26,7 @@ Before: ```js import { getWithDefault } from '@ember/object'; -let result = getWithDefault(obj, ‘some.key', defaultValue); +let result = getWithDefault(obj, 'some.key', defaultValue); ``` After: @@ -40,13 +40,66 @@ if (result === undefined) { } ``` -### Nullish Coalescing Operator +#### Using Nullish Coalescing Operator -The expected behaviour of the `getWithDefault` function is to only return the default value if it is strictly `undefined` so we cannot codemod usage directly into the nullish coalescing operator. +We cannot codemod directly into the nullish coalescing operator since the expected behaviour of `getWithDefault` is to only return the default value if it is strictly `undefined`. The nullish coalescing operator accepts either `null` or `undefined` as _falsey_ values to show the default value. -However [Babel](https://babeljs.io/) already supports the [nullish coalescing operator](https://babeljs.io/docs/en/next/babel-plugin-proposal-nullish-coalescing-operator.html) so we can use that for future use cases where we need to check if a property is `null` or `undefined` before applying a default value. +The function `getWithDefault` **will not return** the default value if the provided value is `null`. The function will **only return** the default value for `undefined`: -Similarly [TypeScript](https://github.com/microsoft/TypeScript) as of [version 3.7](https://devblogs.microsoft.com/typescript/announcing-typescript-3-7/#nullish-coalescing) also supports the operator so we will not be breaking that flow either. +```js +let defaultValue = 1; +let obj = { + nullValue: null, +}; + +// Returns defaultValue 1, undefinedKey = 1 +let undefinedValue = getWithDefault(obj, 'undefinedKey', defaultValue); + +// Returns null, nullValue = null +let nullValue = getWithDefault(obj, 'nullValue', defaultValue); +``` + +The nullish coalescing operator (`??`) **will return** the default value when the provided value is `undefined` or `null`: + +```js +let defaultValue = 1; +let obj = { + nullValue: null, +}; + +// Returns defaultValue 1, undefinedKey = 1 +let undefinedValue = get(obj, 'undefinedKey') ?? defaultValue; + +// Returns defaultValue 1, nullValue = 1 +let nullValue = get(obj, 'nullValue') ?? defaultValue; +``` + +This can be an option if we are aware that either `null` or `undefined` should return the default value. + +Tooling Support: + +- [Babel](https://babeljs.io/) already supports the [nullish coalescing operator](https://babeljs.io/docs/en/next/babel-plugin-proposal-nullish-coalescing-operator.html) so we can use that for future use cases where we need to check if a property is `null` or `undefined` before applying a default value. + +- [TypeScript](https://github.com/microsoft/TypeScript), similarly, as of [version 3.7](https://devblogs.microsoft.com/typescript/announcing-typescript-3-7/#nullish-coalescing) also supports the operator so we will not be breaking that flow either. + +#### Using Object Destructuring With Defaults + +If we are only looking to check If both `null` or `undefined` should return the default value in a particular use case, using [object destructuring](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment) with defaults can also be an option. + +Object destructuring with defaults **will return** the default value when the provided value is `undefined` or `null`: + +```js +let defaultValue = 1; +let obj = { + nullValue: null, +}; + +// Returns defaultValue 1, undefinedKey = 1 +let { undefinedKey = defaultValue } = obj; + +// Returns defaultValue 1, nullValue = 1 +let { nullValue = defaultValue } = obj; +``` ## How We Teach This From d1a8d8b0b550e738e425cffecb90c171d4d9229e Mon Sep 17 00:00:00 2001 From: Chris Ng Date: Wed, 4 Dec 2019 13:27:28 -0500 Subject: [PATCH 10/12] Update text/0554-deprecate-getwithdefault.md Co-Authored-By: Robert Jackson --- text/0554-deprecate-getwithdefault.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0554-deprecate-getwithdefault.md b/text/0554-deprecate-getwithdefault.md index f84730ce7d..b3df006280 100644 --- a/text/0554-deprecate-getwithdefault.md +++ b/text/0554-deprecate-getwithdefault.md @@ -42,7 +42,7 @@ if (result === undefined) { #### Using Nullish Coalescing Operator -We cannot codemod directly into the nullish coalescing operator since the expected behaviour of `getWithDefault` is to only return the default value if it is strictly `undefined`. The nullish coalescing operator accepts either `null` or `undefined` as _falsey_ values to show the default value. +We cannot codemod directly into the nullish coalescing operator since the expected behaviour of `getWithDefault` is to only return the default value if it is strictly `undefined`. The nullish coalescing operator accepts either `null` or `undefined` to show the default value. The function `getWithDefault` **will not return** the default value if the provided value is `null`. The function will **only return** the default value for `undefined`: From 7b6bb991f3a7120073ea7a86c12e4ba425bee03f Mon Sep 17 00:00:00 2001 From: Chris Ng Date: Wed, 4 Dec 2019 13:30:19 -0500 Subject: [PATCH 11/12] Update text/0554-deprecate-getwithdefault.md Co-Authored-By: Robert Jackson --- text/0554-deprecate-getwithdefault.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0554-deprecate-getwithdefault.md b/text/0554-deprecate-getwithdefault.md index b3df006280..3c547a5023 100644 --- a/text/0554-deprecate-getwithdefault.md +++ b/text/0554-deprecate-getwithdefault.md @@ -84,7 +84,7 @@ Tooling Support: #### Using Object Destructuring With Defaults -If we are only looking to check If both `null` or `undefined` should return the default value in a particular use case, using [object destructuring](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment) with defaults can also be an option. +If we would like to return the default value if the existing value is falsey, using [object destructuring](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment) with defaults can also be an option. Object destructuring with defaults **will return** the default value when the provided value is `undefined` or `null`: From e5e9d8249a26fcfe6c0199f7fe0e092876ff1e1f Mon Sep 17 00:00:00 2001 From: Chris Ng Date: Wed, 4 Dec 2019 13:42:27 -0500 Subject: [PATCH 12/12] Fix obj dest and add boolean false as a case --- text/0554-deprecate-getwithdefault.md | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/text/0554-deprecate-getwithdefault.md b/text/0554-deprecate-getwithdefault.md index 3c547a5023..f8ea820bab 100644 --- a/text/0554-deprecate-getwithdefault.md +++ b/text/0554-deprecate-getwithdefault.md @@ -50,6 +50,7 @@ The function `getWithDefault` **will not return** the default value if the provi let defaultValue = 1; let obj = { nullValue: null, + falseValue: false, }; // Returns defaultValue 1, undefinedKey = 1 @@ -57,6 +58,9 @@ let undefinedValue = getWithDefault(obj, 'undefinedKey', defaultValue); // Returns null, nullValue = null let nullValue = getWithDefault(obj, 'nullValue', defaultValue); + +// Returns obj's falseValue, falseValue = false +let falseValue = getWithDefault(obj, 'falseValue', defaultValue); ``` The nullish coalescing operator (`??`) **will return** the default value when the provided value is `undefined` or `null`: @@ -65,6 +69,7 @@ The nullish coalescing operator (`??`) **will return** the default value when th let defaultValue = 1; let obj = { nullValue: null, + falseValue: false, }; // Returns defaultValue 1, undefinedKey = 1 @@ -72,6 +77,9 @@ let undefinedValue = get(obj, 'undefinedKey') ?? defaultValue; // Returns defaultValue 1, nullValue = 1 let nullValue = get(obj, 'nullValue') ?? defaultValue; + +// Returns obj's falseValue, falseValue = false +let falseValue = get(obj, 'falseValue') ?? defaultValue; ``` This can be an option if we are aware that either `null` or `undefined` should return the default value. @@ -84,21 +92,25 @@ Tooling Support: #### Using Object Destructuring With Defaults -If we would like to return the default value if the existing value is falsey, using [object destructuring](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment) with defaults can also be an option. +If we would like to return the default value if the existing value is `undefined` we can also use [object destructuring](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment) with defaults. -Object destructuring with defaults **will return** the default value when the provided value is `undefined` or `null`: +Object destructuring with defaults **will return** the default value when the provided value is `undefined`: ```js let defaultValue = 1; let obj = { nullValue: null, + falseValue: false, }; // Returns defaultValue 1, undefinedKey = 1 let { undefinedKey = defaultValue } = obj; -// Returns defaultValue 1, nullValue = 1 +// Returns defaultValue 1, nullValue = null let { nullValue = defaultValue } = obj; + +// Returns obj's falseValue, falseValue = false +let { falseValue = defaultValue } = obj; ``` ## How We Teach This