From ded28effd8c2223c73452d16d39b981cc0c6e59e Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Thu, 30 Oct 2014 23:40:21 -0400 Subject: [PATCH 01/15] initial draft --- 0000-read-only-cp-syntax.md | 59 +++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 0000-read-only-cp-syntax.md diff --git a/0000-read-only-cp-syntax.md b/0000-read-only-cp-syntax.md new file mode 100644 index 0000000000..7983bf2233 --- /dev/null +++ b/0000-read-only-cp-syntax.md @@ -0,0 +1,59 @@ +- Start Date: 2014-19-30 +- RFC PR: (leave this empty) +- Ember Issue: (leave this empty) + +# Summary + +Getter only computed properties should be readOnly by default + +# Motivation + +Overridable CP's are unexpected, hard to learn, and wanted in the minority of +cases. When it happens unexpeditly it is nearly always unwanted and the resulting +choas is hard to debug. + +# Detailed design + +```js +firstName: Ember.computed(function() { + +}); // readOnly +``` + + +```js +firstName: Ember.computed(function() { + +}).writable(); // writable +``` + +```js +firstName: Ember.computed(function(key, value) { + +}); // writable +``` + +```js +firstName: Ember.computed(function(key, value) { + +}).readOnly(); // readOnly but why? +``` + +# migration: + +* 1.x introduce new syntax +* 1.x update internals to correctly use `writable` if needed (and runs tests against this) +* 1.x deprecate writability unless `writable` was explicitly called, or new `set` syntax was used +* 2.0 flip the switch to `readOnly` by default and never look back. + +# Drawbacks + +N/A + +# Alternatives + +N/A + +# Unresolved questions + +N/A From 2185c65239b476b58b97214800e0a5f0316b19ae Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Sun, 2 Nov 2014 12:47:41 -0500 Subject: [PATCH 02/15] Update 0000-read-only-cp-syntax.md --- 0000-read-only-cp-syntax.md | 1 - 1 file changed, 1 deletion(-) diff --git a/0000-read-only-cp-syntax.md b/0000-read-only-cp-syntax.md index 7983bf2233..7ccd457168 100644 --- a/0000-read-only-cp-syntax.md +++ b/0000-read-only-cp-syntax.md @@ -41,7 +41,6 @@ firstName: Ember.computed(function(key, value) { # migration: -* 1.x introduce new syntax * 1.x update internals to correctly use `writable` if needed (and runs tests against this) * 1.x deprecate writability unless `writable` was explicitly called, or new `set` syntax was used * 2.0 flip the switch to `readOnly` by default and never look back. From c69316bf902ef51708f5216a3d46269fbe260d72 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Sun, 2 Nov 2014 12:59:34 -0500 Subject: [PATCH 03/15] Update 0000-read-only-cp-syntax.md --- 0000-read-only-cp-syntax.md | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/0000-read-only-cp-syntax.md b/0000-read-only-cp-syntax.md index 7ccd457168..a2f95506f0 100644 --- a/0000-read-only-cp-syntax.md +++ b/0000-read-only-cp-syntax.md @@ -24,19 +24,21 @@ firstName: Ember.computed(function() { ```js firstName: Ember.computed(function() { -}).writable(); // writable +}).overridable(); // writable ``` ```js -firstName: Ember.computed(function(key, value) { - -}); // writable +firstName: Ember.computed({ + get: function() { }, + set: function() { }, +}).overridable(); // throw new Error("Overridable cannot be used if a setter already exists...."); ``` ```js -firstName: Ember.computed(function(key, value) { - -}).readOnly(); // readOnly but why? +firstName: Ember.computed({ + get: function() { }, + set: function() { } +}).readOnly(); // throw new Error("Cannot mark a CP as readOnly if it has an explicit setter"); ``` # migration: From 1ed3f2e78307a0199dd34495e5d08f359cc220fa Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Sun, 2 Nov 2014 13:00:54 -0500 Subject: [PATCH 04/15] Update 0000-read-only-cp-syntax.md --- 0000-read-only-cp-syntax.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/0000-read-only-cp-syntax.md b/0000-read-only-cp-syntax.md index a2f95506f0..62ab232a47 100644 --- a/0000-read-only-cp-syntax.md +++ b/0000-read-only-cp-syntax.md @@ -30,7 +30,7 @@ firstName: Ember.computed(function() { ```js firstName: Ember.computed({ get: function() { }, - set: function() { }, + set: function() { } }).overridable(); // throw new Error("Overridable cannot be used if a setter already exists...."); ``` From efd2fbe8eb61aed32640db7bc82846c0b693b6e7 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Sun, 2 Nov 2014 13:05:45 -0500 Subject: [PATCH 05/15] Update 0000-read-only-cp-syntax.md --- 0000-read-only-cp-syntax.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/0000-read-only-cp-syntax.md b/0000-read-only-cp-syntax.md index 62ab232a47..bfe004e28b 100644 --- a/0000-read-only-cp-syntax.md +++ b/0000-read-only-cp-syntax.md @@ -41,6 +41,19 @@ firstName: Ember.computed({ }).readOnly(); // throw new Error("Cannot mark a CP as readOnly if it has an explicit setter"); ``` +```js + +(super) firstName: Ember.computed({ + get: function() { }, + set: function(key, value) { /* is invoke */ } +}); // + +firstName: Ember.computed({ + get: function() { }, + set: function(key, value) { this._super(key, value); } +}); // +``` + # migration: * 1.x update internals to correctly use `writable` if needed (and runs tests against this) From 415cc0b0b179cadea1dbf9ba18935afbb31747b1 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Sun, 2 Nov 2014 13:06:14 -0500 Subject: [PATCH 06/15] Update 0000-read-only-cp-syntax.md --- 0000-read-only-cp-syntax.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/0000-read-only-cp-syntax.md b/0000-read-only-cp-syntax.md index bfe004e28b..9db2605cf7 100644 --- a/0000-read-only-cp-syntax.md +++ b/0000-read-only-cp-syntax.md @@ -43,7 +43,7 @@ firstName: Ember.computed({ ```js -(super) firstName: Ember.computed({ +/*(super)*/ firstName: Ember.computed({ get: function() { }, set: function(key, value) { /* is invoke */ } }); // From ce6ad7ff48f124c79d1b3197f9a48cd78137df98 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Sun, 2 Nov 2014 13:06:47 -0500 Subject: [PATCH 07/15] Update 0000-read-only-cp-syntax.md --- 0000-read-only-cp-syntax.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/0000-read-only-cp-syntax.md b/0000-read-only-cp-syntax.md index 9db2605cf7..0c2f5b3709 100644 --- a/0000-read-only-cp-syntax.md +++ b/0000-read-only-cp-syntax.md @@ -43,10 +43,10 @@ firstName: Ember.computed({ ```js -/*(super)*/ firstName: Ember.computed({ +firstName: Ember.computed({ get: function() { }, - set: function(key, value) { /* is invoke */ } -}); // + set: function(key, value) { /* is invoked */ } +}); // on super class firstName: Ember.computed({ get: function() { }, From f3400087c6d5c0e822816f6a7ee71c6b01b69366 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Sun, 2 Nov 2014 13:08:43 -0500 Subject: [PATCH 08/15] Update 0000-read-only-cp-syntax.md --- 0000-read-only-cp-syntax.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/0000-read-only-cp-syntax.md b/0000-read-only-cp-syntax.md index 0c2f5b3709..f1501c8f79 100644 --- a/0000-read-only-cp-syntax.md +++ b/0000-read-only-cp-syntax.md @@ -44,14 +44,13 @@ firstName: Ember.computed({ ```js firstName: Ember.computed({ - get: function() { }, - set: function(key, value) { /* is invoked */ } + get: function() { } }); // on super class firstName: Ember.computed({ get: function() { }, set: function(key, value) { this._super(key, value); } -}); // +}); // throw new Error("Overridable cannot be used if a setter already exists...."); ``` # migration: From a4d92d517d210f8d47b40992a33b9569a13e7da7 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Sun, 2 Nov 2014 13:10:48 -0500 Subject: [PATCH 09/15] Update 0000-read-only-cp-syntax.md --- 0000-read-only-cp-syntax.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/0000-read-only-cp-syntax.md b/0000-read-only-cp-syntax.md index f1501c8f79..fadede51c2 100644 --- a/0000-read-only-cp-syntax.md +++ b/0000-read-only-cp-syntax.md @@ -50,7 +50,7 @@ firstName: Ember.computed({ firstName: Ember.computed({ get: function() { }, set: function(key, value) { this._super(key, value); } -}); // throw new Error("Overridable cannot be used if a setter already exists...."); +}); // throw new Error("ReadOnly Error blah blah blah"); ``` # migration: From 0883d93d48ef676db69a09d8c53edfd33faf4bd8 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Sun, 2 Nov 2014 13:13:10 -0500 Subject: [PATCH 10/15] Update 0000-read-only-cp-syntax.md --- 0000-read-only-cp-syntax.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/0000-read-only-cp-syntax.md b/0000-read-only-cp-syntax.md index fadede51c2..22a7479e73 100644 --- a/0000-read-only-cp-syntax.md +++ b/0000-read-only-cp-syntax.md @@ -20,12 +20,26 @@ firstName: Ember.computed(function() { }); // readOnly ``` +Is equivalent to: + +```js +firstName: Ember.computed({ + get: function() { +}); // readOnly (same as above) +``` ```js firstName: Ember.computed(function() { }).overridable(); // writable ``` +Is equivalent to: + +```js +firstName: Ember.computed({ + get: function() { +}).overridable(); // writable +``` ```js firstName: Ember.computed({ From 1ef68e7a978bec685790f8f37413efdc66d34958 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Sun, 2 Nov 2014 13:15:26 -0500 Subject: [PATCH 11/15] Update 0000-read-only-cp-syntax.md --- 0000-read-only-cp-syntax.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/0000-read-only-cp-syntax.md b/0000-read-only-cp-syntax.md index 22a7479e73..8950dec88c 100644 --- a/0000-read-only-cp-syntax.md +++ b/0000-read-only-cp-syntax.md @@ -28,11 +28,14 @@ firstName: Ember.computed({ }); // readOnly (same as above) ``` +--- + ```js firstName: Ember.computed(function() { }).overridable(); // writable ``` + Is equivalent to: ```js @@ -41,6 +44,17 @@ firstName: Ember.computed({ }).overridable(); // writable ``` +--- + + +```js +firstName: Ember.computed(function(key, value) { + +}).overridable(); // throw new Error("Overridable cannot be used if a setter already exists...."); +``` + +Is equivalent to: + ```js firstName: Ember.computed({ get: function() { }, @@ -48,6 +62,8 @@ firstName: Ember.computed({ }).overridable(); // throw new Error("Overridable cannot be used if a setter already exists...."); ``` +--- + ```js firstName: Ember.computed({ get: function() { }, From c663575f587e873fa91ff2b7a0c4b527af58bed5 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Sun, 2 Nov 2014 13:28:31 -0500 Subject: [PATCH 12/15] Update 0000-read-only-cp-syntax.md --- 0000-read-only-cp-syntax.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/0000-read-only-cp-syntax.md b/0000-read-only-cp-syntax.md index 8950dec88c..efc77df116 100644 --- a/0000-read-only-cp-syntax.md +++ b/0000-read-only-cp-syntax.md @@ -12,6 +12,38 @@ Overridable CP's are unexpected, hard to learn, and wanted in the minority of cases. When it happens unexpeditly it is nearly always unwanted and the resulting choas is hard to debug. + +```js +var User = Ember.Object.extend({ + isOld: Ember.computed('age', function() { + return this.get('age') > 30; + }); +}); + +var user = User.create({ + age: 30 +}); + +user.get('isOld'); // => false + +user.set('age', 31); +user.get('isOld'); // => true + +user.set('isOld', false); +user.get('isOld'); // => false + +user.set('age', 30); +user.get('isOld'); // => false (EWUT?) +``` + + +What just happened: + +* although age > 30 : isOld remains false +* by explicitly setting isOld, we have diverged the CP to no longer invalidate based on its dependentKey. +* many bugs and hair loss + + # Detailed design ```js From 414a9c7b8bd8c4a1cebdecf22b57726b90854a23 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Sun, 2 Nov 2014 13:28:57 -0500 Subject: [PATCH 13/15] Update 0000-read-only-cp-syntax.md --- 0000-read-only-cp-syntax.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/0000-read-only-cp-syntax.md b/0000-read-only-cp-syntax.md index efc77df116..e9c74aeb55 100644 --- a/0000-read-only-cp-syntax.md +++ b/0000-read-only-cp-syntax.md @@ -42,6 +42,8 @@ What just happened: * although age > 30 : isOld remains false * by explicitly setting isOld, we have diverged the CP to no longer invalidate based on its dependentKey. * many bugs and hair loss +* no way to sensibly recover +* rarely wantedd # Detailed design From 58a973ea32549b96c30bc54769877af4c0affbd4 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Sun, 2 Nov 2014 13:29:02 -0500 Subject: [PATCH 14/15] Update 0000-read-only-cp-syntax.md --- 0000-read-only-cp-syntax.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/0000-read-only-cp-syntax.md b/0000-read-only-cp-syntax.md index e9c74aeb55..4533020b82 100644 --- a/0000-read-only-cp-syntax.md +++ b/0000-read-only-cp-syntax.md @@ -43,7 +43,7 @@ What just happened: * by explicitly setting isOld, we have diverged the CP to no longer invalidate based on its dependentKey. * many bugs and hair loss * no way to sensibly recover -* rarely wantedd +* rarely wanted # Detailed design From b07f76c13607ce7201aaba36970df66a5b3aaeaa Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Tue, 4 Nov 2014 09:03:56 -0500 Subject: [PATCH 15/15] Update 0000-read-only-cp-syntax.md --- 0000-read-only-cp-syntax.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/0000-read-only-cp-syntax.md b/0000-read-only-cp-syntax.md index 4533020b82..7591133bc8 100644 --- a/0000-read-only-cp-syntax.md +++ b/0000-read-only-cp-syntax.md @@ -10,7 +10,7 @@ Getter only computed properties should be readOnly by default Overridable CP's are unexpected, hard to learn, and wanted in the minority of cases. When it happens unexpeditly it is nearly always unwanted and the resulting -choas is hard to debug. +chaos is hard to debug. ```js