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

Ember.arrayComputed should consider compound dependent keys #4684

Closed
searls opened this issue Apr 10, 2014 · 10 comments
Closed

Ember.arrayComputed should consider compound dependent keys #4684

searls opened this issue Apr 10, 2014 · 10 comments

Comments

@searls
Copy link

searls commented Apr 10, 2014

In attempting to work around #4681 at @mmun's suggestion I ended up with something like this, expecting two invocations of addItem for each item (one empty object before setupData was called and once after content was set):

Ember.arrayComputed "messages.@each.content"

However, even then I was seeing two invocations, one with content set. But another property I needed (receivedAt) was still not set, so I tried:

Ember.arrayComputed "messages.@each.{content,receivedAt}"

And it continued to only fire twice, with receivedAt still being undefined both times.

@searls
Copy link
Author

searls commented Apr 10, 2014

After reading the source, I think I found a workaround in my case: Ember.arrayComputed("messages.@each.data"), though it means it'll only ever update when ember-data is the one pushing the data.

@wagenet
Copy link
Member

wagenet commented Apr 15, 2014

@hjdivad ping.

@hjdivad
Copy link
Member

hjdivad commented Apr 17, 2014

@searls was "this" above meant to link to something, perhaps a jsbin?

does http://emberjs.jsbin.com/gafix/8/edit still exhibit the failing behaviour? I'm getting the following in the console:

My array computed is 2: HEY! 

@searls
Copy link
Author

searls commented Apr 17, 2014

Sorry about that. Totally lost the bin apparently.

See this approach with the DK of messages.@each.receivedAt. The receivedAt property is apparently processed last, so the whole summary displays correctly.
http://emberjs.jsbin.com/gafix/19/edit

Compare with this one with DK of messages.@each.{content,receivedAt}. The computed array is updated for content but not for receivedAt
http://emberjs.jsbin.com/gafix/20/edit

@wagenet
Copy link
Member

wagenet commented Jul 28, 2014

See #5268

@wagenet wagenet closed this as completed Jul 28, 2014
@wagenet wagenet mentioned this issue Jul 28, 2014
2 tasks
@drogus
Copy link
Contributor

drogus commented Sep 23, 2014

@searls I gave this thing a try today and I started by writing a failing test, which in fact doesn't fail. Could you take a look and see if anything here is different from your setup?

QUnit.module('arrayComputed - compound dependent keys', {
  setup: function() {
    callbackItems = [];
    run(function() {
      obj = EmberObject.createWithMixins({
        items: Ember.A([EmberObject.create({ n: 'one', m: 'two' })]),
        itemsNM: arrayComputed('items.@each.{n,m}', {
          addedItem: function(array, item, changeMeta, instanceMeta) {
            callbackItems.push('added:' + get(item, 'n') + ':' + get(item, 'm'));
          },
          removedItem: function(array, item, changeMeta, instanceMeta) {
            callbackItems.push('removed:' + get(item, 'n') + ':' + get(item, 'm'));
          }
        })
      });
    });
  },
  teardown: function() {
    run(function() {
      obj.destroy();
    });
  }
});

test("when compound keys are used, addedItem and removedItem callbacks should be called after each change", function() {
  var expected, items;

  items = get(obj, 'items');

  run(function() {
    obj.get('itemsNM');
  });

  run(function() {
    set(items.get('firstObject'), 'n', "two");
    set(items.get('firstObject'), 'm', "one");
  });

  expected = [
    "added:one:two",
    "removed:two:two",
    "added:two:two",
    "removed:two:one",
    "added:two:one"
  ];
  deepEqual(callbackItems, expected, "changeMeta includes previous values");
});

@searls
Copy link
Author

searls commented Sep 23, 2014

Has the SSOT branch been merged in? If so then I bet that fixed this.

On Tue, Sep 23, 2014 at 6:34 PM, Piotr Sarnacki notifications@github.com
wrote:

@searls I gave this thing a try today and I started by writing a failing test, which in fact doesn't fail. Could you take a look and see if anything here is different from your setup?

QUnit.module('arrayComputed - compound dependent keys', {
  setup: function() {
    callbackItems = [];
    run(function() {
      obj = EmberObject.createWithMixins({
        items: Ember.A([EmberObject.create({ n: 'one', m: 'two' })]),
        itemsNM: arrayComputed('items.@each.{n,m}', {
          addedItem: function(array, item, changeMeta, instanceMeta) {
            callbackItems.push('added:' + get(item, 'n') + ':' + get(item, 'm'));
          },
          removedItem: function(array, item, changeMeta, instanceMeta) {
            callbackItems.push('removed:' + get(item, 'n') + ':' + get(item, 'm'));
          }
        })
      });
    });
  },
  teardown: function() {
    run(function() {
      obj.destroy();
    });
  }
});
test("when compound keys are used, addedItem and removedItem callbacks should be called after each change", function() {
  var expected, items;
  items = get(obj, 'items');
  run(function() {
    obj.get('itemsNM');
  });
  run(function() {
    set(items.get('firstObject'), 'n', "two");
    set(items.get('firstObject'), 'm', "one");
  });
  expected = [
    "added:one:two",
    "removed:two:two",
    "added:two:two",
    "removed:two:one",
    "added:two:one"
  ];
  deepEqual(callbackItems, expected, "changeMeta includes previous values");
});

Reply to this email directly or view it on GitHub:
#4684 (comment)

@fishermand46
Copy link

What's the SSOT branch?

@fivetanley
Copy link
Member

@searls yeah, since beta.10, but you should use beta.11. @fishermand46 It's a branch of ember data designed to fix relationship bugs and increase consistency with adding and removing relationships on both sides of the relationship. E.g. if I add or remove from one side (post.get('comments')) it should show up on the other side('comment.get('post')). It was merged in beta.10 but we had some regressions that should be fixed in beta.11. Let us know!

@fishermand46
Copy link

Thanks for the response @fivetanley! In that case, I think my issue is unrelated to this. I think #5268 is the ticket for me.

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

No branches or pull requests

6 participants