fix(repeat): handle pass-through converters

Fixes aurelia/framework#258
aurelia · Dec 1, 2015 · b2741c9 · b2741c9
1 parent 3eb4ee0
commit b2741c9
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 5 deletions.
diff --git a/src/repeat.js b/src/repeat.js
@@ -155,13 +155,15 @@ export class Repeat {
     let newItems = this.sourceExpression.evaluate(this.scope, this.lookupFunctions);
     this.observerLocator.taskQueue.queueMicroTask(() => this.ignoreMutation = false);
 
-    // collection change?
+    // call itemsChanged...
     if (newItems === this.items) {
-      return;
+      // call itemsChanged directly.
+      this.itemsChanged();
+    } else {
+      // call itemsChanged indirectly by assigning the new collection value to
+      // the items property, which will trigger the self-subscriber to call itemsChanged.
+      this.items = newItems;
     }
-    // assign the new value to the items property, which will trigger the
-    // self-subscriber to call itemsChanged.
-    this.items = newItems;
   }
 
   _observeInnerCollection() {

diff --git a/test/repeat-integration.spec.js b/test/repeat-integration.spec.js
@@ -52,6 +52,9 @@ function createViewResources(container) {
   // slice value converter
   resources.registerValueConverter('slice', { toView: array => array ? array.slice(0) : array });
 
+  // no-op value converter
+  resources.registerValueConverter('noopValueConverter', { toView: value => value });
+
   // toLength value converter
   resources.registerValueConverter('toLength', { toView: collection => collection ? (collection.length || collection.size || 0) : 0 });
 
@@ -277,6 +280,55 @@ function describeArrayTests(viewsRequireLifecycle) {
     });
   });
 
+  describe('with converter that returns original instance', () => {
+    beforeEach(() => {
+      let template = `<template><div repeat.for="item of items | noopValueConverter">\${item}</div></template>`;
+      viewModel = { items: ['a', 'b', 'c'] };
+      controller = createController(template, viewModel, viewsRequireLifecycle);
+      validateState();
+    });
+
+    afterEach(() => {
+      controller.unbind();
+      expect(hasSubscribers(viewModel, 'items')).toBe(false);
+      expect(hasArraySubscribers(viewModel.items)).toBe(false);
+    });
+
+    it('handles mutation', done => {
+      viewModel.items.push('d');
+      nq(() => validateState());
+      nq(() => viewModel.items.pop());
+      nq(() => validateState());
+      nq(() => viewModel.items.reverse());
+      nq(() => validateState());
+      nq(() => done());
+    });
+
+    it('handles property change', done => {
+      let observer = observerLocator.getArrayObserver(viewModel.items);
+      viewModel.items = null;
+      nq(() => {
+        expect(select(controller, 'div').length).toBe(0);
+        expect(observer.hasSubscribers()).toBe(false);
+      });
+      nq(() => {
+        viewModel.items = ['x', 'y', 'z'];
+        observer = observerLocator.getArrayObserver(viewModel.items);
+      });
+      nq(() => {
+        validateState();
+        viewModel.items = undefined;
+      });
+      nq(() => {
+        expect(select(controller, 'div').length).toBe(0);
+        expect(observer.hasSubscribers()).toBe(false);
+      });
+      nq(() => viewModel.items = []);
+      nq(() => validateState());
+      nq(() => done());
+    });
+  });
+
   describe('with converter and behavior', () => {
     beforeEach(() => {
       let template = `<template><div repeat.for="item of items | slice & noopBehavior">\${item}</div></template>`;