Supporting didUpdateAttrs

[GavinJoyce]

Having the addon only support didReceiveAttrs makes it much less valuable as it's only a partial solution to attribute change tracking and leaves consuming app having to implement their own tracking logic when they are interested in changes in didUpdateAttrs.

I'm interested in helping to explore how this addon might support didUpdateAttrs as well as the currently supported didReceiveAttrs (see emberjs/rfcs#191 (comment) for some context).

I'm just beginning to look into it now, I'm creating this issue to track any conversation or progress.

/cc @workmanw

[GavinJoyce]

[GavinJoyce]

The mixin approach works great, here's the WIP API that I'm using:

import Ember from 'ember';
import DidChangeAttrs from 'ember-did-change-attrs';

export default Ember.Component.extend(DidChangeAttrs, {
 didChangeAttrsConfig: {
   attrs: ['email', 'isAdmin']
 },
 
 didChangeAttrs(changes) {
   if(changes.email) {
     console.log('email previous', changes.email.previous);
     console.log('email current', changes.email.current);
   }
 }
});

[GavinJoyce]

WIP PR: #5

[GavinJoyce]

For comparison:

Current API:

import diffAttrs from 'ember-diff-attrs';

export default Ember.Component.extend({
  didReceiveAttrs: diffAttrs('email', 'isAdmin', function(changedAttrs, ...args) {
    this._super(...args);

    if(changedAttrs && changedAttrs.email) {
      let oldEmail = changedAttrs.email[0],
          newEmail = changedAttrs.email[1];
      // Do stuff
    }
  })
});

New API (which works with both didReceiveAttrs and didUpdateAttrs use cases):

import DidChangeAttrs from 'ember-did-change-attrs';

export default Ember.Component.extend(DidChangeAttrs, {
  didChangeAttrsConfig: {
    attrs: ['email', 'isAdmin']
  },
 
  didChangeAttrs(changes) {
    if(changes.email) {
      let oldEmail = changes.email.previous,
          newEmail = changes.email.current;
      // Do stuff
    }
  }
});

(hopefully we can come up with a better name than didChangeAttrsConfig)

[GavinJoyce]

@workmanw, a couple of (possibly controversial) thoughts:

• we should retire the diffAttrs API in favour of the Mixin approach
• we should rename this addon to ember-did-change-attrs

[workmanw]

@GavinJoyce I'd like to know if anyone else (who might be lurking 👀) would feel about that.

I understand the goal of that change would be to only have one way to do use it. I'm a little hesitant to go whole-hog and do away with the decorator approach because I genuinely think there are a lot of people against Mixins. Again, I'm not one of them, had I just written this for myself (and not as an Addon) I would have reached for a Mixin solution.

Maybe the best option would be for us to create a new addon ember-did-change-attrs with this approach, and cross link the README.mds. That would be non controversial and make it easy for us to track adoption. I have some free cycles right now, I could bootstrap the addon and add you to it as a committer if you'd like.

Thoughts?

[GavinJoyce]

Maybe the best option would be for us to create a new addon ember-did-change-attrs with this approach, and cross link the README.mds.

Sounds like a good approach to me, thanks

[workmanw]

@GavinJoyce I'm on it. Hold tight.

[workmanw]

@GavinJoyce et al, done! ember-did-change-attrs is now a bootstrapped addon. I'm happy to continue collaborating over there. I'll open an issue and cross link it.