Skip to content
This repository was archived by the owner on Feb 22, 2018. It is now read-only.

[dccd] Changing ref to Iterables cause an exception #1158

Closed
vicb opened this issue Jun 17, 2014 · 3 comments
Closed

[dccd] Changing ref to Iterables cause an exception #1158

vicb opened this issue Jun 17, 2014 · 3 comments

Comments

@vicb
Copy link
Contributor

vicb commented Jun 17, 2014

@chirayuk could you please take a look at this:

      iit('should support switching refs', async(() {
        var list = [0];

        var record = detector.watch(list, null, null);
        microLeap();
        var iterator = detector.collectChanges()..moveNext();

        record.object = [1, 0];
        microLeap();
        iterator = detector.collectChanges()..moveNext();
        expect(iterator.current.currentValue, toEqualCollectionRecord(
            collection: ['1[null -> 0]', '0[0->1]'],
            additions: ['1[null -> 0]'],
            moves: ['0[0->1]'],
            removals: []));
      }));

The error is
`Test failed: Caught The null object does not have a setter '_next@0x1d3970e2='.

Is the usage wrong or is this a bug ?

(You can ignore the async and microLeap - they are not related to the issue)

@vicb
Copy link
Contributor Author

vicb commented Jun 18, 2014

@chirayuk seems like detector.collectChangesshould be called twice for this to work, is that expected ?

      iit('should support switching from List to ObservableList', sync(() {
        var list = [0];

        var record = detector.watch(list, null, null);
        detector.collectChanges();
        detector.collectChanges();

        var iterator;
        record.object = [1, 0];
        iterator = detector.collectChanges()..moveNext();
        expect(iterator.current.currentValue, toEqualCollectionRecord(
            collection: ['1[null -> 0]', '0[0 -> 1]'],
            previous: ['0[0 -> 1]'],
            additions: ['1[null -> 0]'],
            moves: ['0[0 -> 1]'],
            removals: []));

        record.object = [2, 1, 0];
        iterator = detector.collectChanges()..moveNext();
        expect(iterator.current.currentValue, toEqualCollectionRecord(
            collection: ['2[null -> 0]', '1[null -> 1]', '0[0 -> 2]'],
            previous: ['0[0 -> 2]'],
            additions: ['2[null -> 0]', '1[null -> 1]'],
            moves: ['0[0 -> 2]'],
            removals: []));
      }));

Notice how the third list is compared to the first one (the test passes)

@chirayuk
Copy link
Contributor

chirayuk commented Jul 1, 2014

In your test case, you're only calling collectChanges once.  In real dirty checking, we call again if there were changes.  This is required for the proper resetting for collection watching (specifically to stay efficient while detecting simultaneous mutation + ref change).

You could rewrite it like this (ref: chirayuk@9f69f73):

iit('ckck: issue 1158: should support switching refs', async(() {
  var list = [0];

  var record = detector.watch(list, null, null);
  if (detector.collectChanges().moveNext()) {
    detector.collectChanges();
  }

  record.object = [1, 0];
  var iterator = detector.collectChanges()..moveNext();
  expect(iterator.current.currentValue, toEqualCollectionRecord(
      collection: ['1[null -> 0]', '0[0 -> 1]'],
      previous: ['0[0 -> 1]'],
      additions: ['1[null -> 0]'],
      moves: ['0[0 -> 1]'],
      removals: []));
}));

@vicb
Copy link
Contributor Author

vicb commented Jul 22, 2014

@chirayuk thanks for your help, I've submitted a PR (#1253)

vicb added a commit to vicb/angular.dart that referenced this issue Aug 4, 2014
vicb added a commit that referenced this issue Aug 13, 2014
vicb added a commit that referenced this issue Aug 13, 2014
vicb added a commit that referenced this issue Aug 13, 2014
@vicb vicb closed this as completed in f930851 Aug 14, 2014
vicb added a commit that referenced this issue Aug 15, 2014
dsalsbury pushed a commit to dsalsbury/angular.dart that referenced this issue Aug 16, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants