Skip to content

Instantly share code, notes, and snippets.

@hjdivad
Created April 23, 2014 21:09
Show Gist options
  • Star 3 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save hjdivad/11232504 to your computer and use it in GitHub Desktop.
Save hjdivad/11232504 to your computer and use it in GitHub Desktop.

Consider the example:

var MyObject = Ember.Object.extend({
  selectedContent: Ember.computed.filterBy('content', 'isSelected')
});

var obj = MyObject.create({
  content: [
    Em.Object.create({name: "one", isSelected: false}),
    Em.Object.create({name: "two", isSelected: false})
  ]
});

obj.get('selectedContent');
obj.get('content').shiftObject();
obj.set('content.firstObject.isSelected', true);

Let's walk through it and consider the state of the observer contexts and the tracked array.

Step 1

obj.get('selectedContent');

Here we have initialised the array. Tracked array looks like this:

retain:2

So there's a single operation that contains two observer contexts starting at index 0.

What happens next?

Step 2

obj.get('content').shiftObject();

Now our tracked array looks like this:

delete:1 retain:1

So we have two operations, one starting at index 0 and the other at index 1. The first operation's contexts includes "one" (the first object) and the second includes "two".

Step 3

obj.set('content.firstObject.isSelected', true);

Okay we've changed an observed item property (isSelected). Because we changed an item property rather than mutating the array we don't have the index of this item in the array, so we need to apply tracked array. There are two operations to apply: our delete:1 operation and our retain:1 operation.

Initially our delete:1 operation is at index 0 and our retain:1 operation is at index 1.

When we apply delete our retain:1 operation moves to index 0, but the observer context still thinks its at index 1.

updateIndexes: function (trackedArray, array) {
  // …
  trackedArray.apply(function (observerContexts, offset, operation) {
    // …

    // This is the loop that updates the observer context indexes
    forEach(observerContexts, function (context, index) {
      context.index = index + offset;
    });
  });
},

When we apply the second operation we need to update the observer context's index.

However, we want to skip the case of retain:n because then we know the observer contexts already have the right index and it would be a waste to iterate all of them: we'd be iterating the entire array! That's why we have this check in updateIndexes

  if (operation === TrackedArray.RETAIN && observerContexts.length === length && offset === 0) {
    return;
  }

But the length of the underlying array is now 1 and after we applied our delete:1 our tracked array now looks like retain:1, so the above test mistakenly thinks we're applying a no-op and skips the loop.

What we really wanted to test was whether the entire tracked array was

retain:n

But we actually only tested whether this particular operation, after the previous ones have been applied, was retain:n.

Because we skip this when the item is removed and added back, our changeMeta.index has its previous index value: 1, and not its new index value: 0. This, to use a somewhat technical term, breaks all kinds of shit.

So the way we can be sure the whole tracked array is retain:n is to only do this check for the first operation (if the first operation is for n items it must be the only operation as there are only n items in the underlying array to account for).

  if (operationIndex === 0 && operation === TrackedArray.RETAIN && observerContexts.length === length && offset === 0) {
    return;
  }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment