Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX release] Deprecate @each as a leaf node. #11994

Merged
merged 12 commits into from
Aug 8, 2015

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Aug 6, 2015

Update stable version with a deprecation when using foo.@each as a dependent key. This code is removed on beta (2.0.0) and canary (2.1.0) branches already.

Bill Heaton and others added 10 commits August 8, 2015 13:25
See: RFC/issue emberjs#80
- emberjs/rfcs#80

(cherry picked from commit 97e4441)
`Ember.K` should be a public method in favor of 6e1ad90.

(cherry picked from commit a867b32)
There are no alternative way to `Ember.merge` in Ember world.
But it is useful and widely used from other library.

For example:
* Ember Data: https://github.com/emberjs/data/blob/v1.13.7/packages/ember-data/lib/core.js#L29
* Liquid Fire: https://github.com/ef4/liquid-fire/blob/v0.21.0/app/transitions/scroll-then.js#L19

So this method may be public until `Object.assign` can be used natively.

(cherry picked from commit 1c79810)
Closes emberjs#11989
[ci skip]

(cherry picked from commit c3b61b8)
- compact
- every
- forEach
- without
- sortBy

Closes emberjs#12004
[ci skip]

(cherry picked from commit d9c3027)
This is fixed properly in canary/beta, but required changes to the build
system so I'm just patching this here to make it easier to get the right
live test page working.
* `TrackedArray`  and `SubArray` are still used by the RC/AC implementation in 1.13.x,
  but it should not trigger a `TrackedArray` deprecation.
* Fix a few tests that were using `{{with}}` and `controller=`.
@rwjblue rwjblue force-pushed the deprecate-at-each-leaf branch 4 times, most recently from 554a012 to 63a3e06 Compare August 8, 2015 19:20
Update stable version with a deprecation when using `foo.@each` as a
dependent key. This code is removed on beta (2.0.0) and canary (2.1.0)
branches already.
rwjblue added a commit that referenced this pull request Aug 8, 2015
[BUGFIX release] Deprecate `@each` as a leaf node.
@rwjblue rwjblue merged commit 034f823 into emberjs:stable Aug 8, 2015
@rwjblue rwjblue deleted the deprecate-at-each-leaf branch August 8, 2015 19:37
@rauhryan
Copy link
Contributor

@rwjblue just out of curiousity.

Is it still possible to track a sub-property of things in the array that are one level deep.

I assume what used to be

Ember.computed('foos.@each.bar')

is changed to

Ember.computed('foos.[].bar')

and all is well?

@stefanpenner
Copy link
Member

@rauhryan other way around:

Ember.computed('foos.[].bar') should be Ember.computed('foos.@each.bar')
Ember.computed('foos.@each') should be Ember.computed('foos.[]')

@each is for chaining
[] is to describe array mutations on the leaf IFF it is an arraylike

@rauhryan
Copy link
Contributor

thanks for clarification @stefanpenner

I'll pass it on to the team

@icecoldmax
Copy link

@stefanpenner How about observing ANY property changes on things like ObjectProxys? I've been doing Ember.observer("foos.@each") to catch any change (say the checked property for a checkbox). Will this still work with Ember.observer("foos.[]")?

@stefanpenner
Copy link
Member

@icecoldmax I don't believe that is a feature. @each and [] are for arrays and array proxies. Not objects or object proxies.

I'm dubious that this works as you describe.

@icecoldmax
Copy link

@stefanpenner I think I described it wrong.

Let me try my checkbox example better:

proxiedFoos: Ember.computed("foos", function() {
  return this.get("foos")
    .map((foo) => {
      return Ember.ObjectProxy.create({
        content: foo,
        checked: false,
      });
  });
})

Let's say these are now iterated over as a list of checkboxes where checking it updates the checked to true. We want to do something when any of them are checked, so we've been successfully using Ember.observer("proxiedFoos.@each").

Is this bad practice?

@stefanpenner
Copy link
Member

Ember.observer("proxiedFoos.@each").

since there is a map, proxiedFoos is always an array and the following should be true.

  • proxiedFoos.@each becomes proxiedFoos.[]
  • but proxiedFoos.@each.checked is used for chaining, and thus stays the same.

@aortbals
Copy link
Contributor

I found that the easiest way to compare these various computed properties was live. I was slightly confused about the behavior. Here is a twiddle: http://ember-twiddle.com/02ddca9b6c89f2f8f92b.

TLDR, from my understanding:

  • array.[] is for observing the array itself, such as adding or removing items and is supported in 2.0
  • array.@each.name will recomputed if any of the items name prop is updated and is supported in 2.0
  • array.@each is deprecated, and it does not observe all item attributes (I think this is what some users expect) in 1.12 or 1.13. It works identically to array.[] in 1.13 from my testing
  • You should use array.[] or array.@each.somePropThatYouCareAbout only

@icecoldmax
Copy link

@stefanpenner @aortbals Thanks for the clarification.

Is there actually a way to do what I'm thinking of? Say you have an array of objects, each with a ton of attributes and you want to observe when any of their properties change. Any way other than just observing/computing on @each.propName?

@stefanpenner
Copy link
Member

@each.propName?

no – only granular changes are currently supported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants