default readOnly CP

[stefanpenner]

• add writable()
• mode to put ember in CP's readOnly by default
• update internals to use writable correctly
• make test suite also run with this flag. (we can likely just use the feature flag tech we currently flag)

[kanongil]

Nice, looking forward to this.

[benlesh]

Tertiary to this, and maybe this is fixed in a newer version... but I've noticed this behavior in components:

willNotSurvive: function() {
  return this.get('a') + this.get('b');
}.property('a', 'b'),

// magic here: adding key, value arguments seem to keep the computed property
// from being overwritten
willSurvive: function(key, value) {
  return this.get('a') + this.get('b');
}.property('a', 'b'),

When you set them both, willNotSurvive will be overwritten with whatever value you set it to, where willSurvive will not. Seems odd:

{{my-component willSurvive=foo willNotSurvive=foo}}

Relevant JSBin: http://emberjs.jsbin.com/dowah/1/edit?html,js,output

[stefanpenner]

When you set them both, willNotSurvive will be overwritten with whatever value you set it to, where willSurvive will not. Seems odd:

works as expected, but yes odd for someone not aware.

[benlesh]

Whoa! That's expected? TIL.

I was genuinely surprised by this behavior. Generally, when I think of a function in JavaScript, I don't expect it to behave differently based on the presence of argument variables. I guess I just wouldn't expect something to be analyzing that part of the function to see if it's somehow different. Then again, I never really liked the "implicit injection" thing with Angular, either.

JSHint actually hates what I did above, because I'm not using key or value. It also warrants me adding a comment above it like // key,value added to prevent property from being overwritten.

Seems like something like what you're proposing could solve this:

foo: function(){
   return this.a + this.b;
}.property().writable(),

But it still isn't very descriptive as to why I'm making it "writable". Really I'm trying to make it "untrampleable" or "unoverwriteable".

[stefanpenner]

Generally, when I think of a function in JavaScript, I don't expect it to behave differently based on the presence of argument variables

this feature was meant as an interim step to allow backwards compatibility, but we never transitioned out. I hope to get us out for 2.0.

plan:

// writable
fullName: Ember.computed('firstName', 'lastName', {
  get: function(key) {
    return this.get('firstName') + ' ' + this.get('lastName');
  },

  set: function(key, value) {
    var name = value.split(' ');
    this.setProperties({
      firstName: name[0],
      lastName: name[1]
    });
  } 
});

// readOnly
fullName: Ember.computed('firstName', 'lastName', {
  get: function(key) {
    return this.get('firstName') + ' ' + this.get('lastName');
  }
  } 
});

// writable
fullName: Ember.computed('firstName', 'lastName', {
  get: function(key) {
    return this.get('firstName') + ' ' + this.get('lastName');
  }
  } 
}).writable();

// writable for now, future readOnly
fullName: Ember.computed('firstName', 'lastName', function(key) {
    return this.get('firstName') + ' ' + this.get('lastName');
  }
);

// writable always
fullName: Ember.computed('firstName', 'lastName', function(key) {
    return this.get('firstName') + ' ' + this.get('lastName');
}).writable();

[OpakAlex]

+1

[cibernox]

@stefanpenner Have you seen this? #9489

[cibernox]

In my understanding, if a computed property is initialized with an object with get and set functions it should be writable. If its initialized with a function (or an object with only get), its readOnly unless you specify otherwise, right? Specify writable() without defining a set function means that the property will just be replaced by the value when seted.

CPs are the piece of the codebase I understand better. I can this things in that very PR if you want so you can focus in HTMLBars.

[stefanpenner]

@cibernox nice! I will review this weekend.

https://github.com/emberjs/rfcs/pull/12/files is the RFC, core decided we need to do some last refinements before it gets merged. But i would love help implementing.

[stefanpenner]

@cibernox also, your feedback on the RFC would be wonderful

[cibernox]

After reading the RFC, I think my PR is perfectly aligned.

Steps done:

• Raise deprecation warnings when passing cacheable or readOnly to the constructor. No extrictly necessary, but is not used anywhere in the code since 2012 and simplifies things.
• Deprecate usage of cacheable() and readOnly(). Are the default.

Steps pending:

• Modify CPs to work internally with setter and getters. A way of doing that gracefully without people even noticing it could be count the arity of the function passed to Ember.computed. If is arity <=1, create only a setter. If has arity > 1, assign the same function to getter and setter. Passing an object also works. No deprecations yet.
• Deprecate passing functions with arity > 1. Recommend user {get, set} instead.
• Implement logic in this RFC: Create writable and/or overridable and determina default values as explained in the code example above.
• Deprecate all the things!
• Ember 2.0. Delete all the things!

[stefanpenner]

@cibernox nice, do you mind moving your checklist to your PR, (it will make it easier to track)

I have added it to my queue to review.:)

[devinrhode2]

Regarding: key as first argument. I've been digging all around GH issues and the ember source trying to figure out why the key is the first argument. Perhaps it is needed internally; but what would a typical user typically be using the key for? Could we hide the key in this.key? I think a new value being set is the most important parameter; I would consider making it the first parameter a 'nice to have'

[stefanpenner]

@devinrhode2 the descriptor can be shared between multiple properties, and this is the current receiver of get

[rwjblue]

@devinrhode2 - Think of computed property generators like DS.attr(), they use the key argument to determine what they are working on. This is a pretty common pattern in Ember libraries.

[stefanpenner]

this is likely not going to happen due to making stubbing mocking in tests trickier