Detach tests, improve region/CV code readability, remove triggerMetho…

…dMany

src/backbone.marionette.js

@@ -42,8 +42,7 @@ import {
 
 import {
   triggerMethod,
-  triggerMethodOn,
-  triggerMethodMany
+  triggerMethodOn
 } from './trigger-method';
 
 var previousMarionette = Backbone.Marionette;
@@ -73,7 +72,6 @@ Marionette.isNodeAttached = isNodeAttached;
 Marionette.deprecate = deprecate;
 Marionette.triggerMethod = proxy(triggerMethod);
 Marionette.triggerMethodOn = triggerMethodOn;
-Marionette.triggerMethodMany = triggerMethodMany;
 Marionette.isEnabled = isEnabled;
 Marionette.setEnabled = setEnabled;
 Marionette.MonitorViewEvents = MonitorViewEvents;

src/collection-view.js

@@ -6,10 +6,8 @@ import Backbone           from 'backbone';
 import ChildViewContainer from 'backbone.babysitter';
 import MarionetteError    from './error';
 import ViewMixin          from './mixins/view';
-import BehaviorsMixin     from './mixins/behaviors';
-import UIMixin            from './mixins/ui';
-import CommonMixin        from './mixins/common';
 import MonitorViewEvents  from './monitor-view-events';
+import destroyBackboneView from './utils/destroyBackboneView';
 import { triggerMethodOn } from './trigger-method';
 
 // A view that iterates over a Backbone.Collection
@@ -51,23 +49,20 @@ const CollectionView = Backbone.View.extend({
 
   _endBuffering() {
     const shouldTriggerAttach = !!this._isAttached;
+    const triggerOnChildren = shouldTriggerAttach ? this._getImmediateChildren() : [];
 
     this._isBuffering = false;
 
-    if (shouldTriggerAttach) {
-      _.each(this._getImmediateChildren(), child => {
-        triggerMethodOn(child, 'before:attach', child);
-      });
-    }
+    _.each(triggerOnChildren, child => {
+      triggerMethodOn(child, 'before:attach', child);
+    });
 
     this.attachBuffer(this, this._createBuffer());
 
-    if (shouldTriggerAttach) {
-      _.each(this._getImmediateChildren(), child => {
-        child._isAttached = true;
-        triggerMethodOn(child, 'attach', child);
-      });
-    }
+    _.each(triggerOnChildren, child => {
+      child._isAttached = true;
+      triggerMethodOn(child, 'attach', child);
+    });
 
     this._bufferedChildren = [];
   },
@@ -123,13 +118,11 @@ const CollectionView = Backbone.View.extend({
   // An efficient rendering used for filtering. Instead of modifying the whole DOM for the
   // collection view, we are only adding or removing the related childrenViews.
   setFilter(filter, {preventRender} = {}) {
-    const viewCanBeRendered = this._isRendered && !this._isDestroyed;
-    // The same filter or a `prevent` option won't render the filter.
-    // Nevertheless, a `prevent` option will modify the value.
-    if (!viewCanBeRendered || this.filter === filter) {
-      return;
-    }
-    if (!preventRender) {
+    const canBeRendered = this._isRendered && !this._isDestroyed;
+    const filterChanged = this.filter !== filter;
+    const shouldRender = canBeRendered && filterChanged && !preventRender;
+
+    if (shouldRender) {
       this.triggerMethod('before:apply:filter', this);
       const previousModels = this._filteredSortedModels();
       this.filter = filter;
@@ -475,36 +468,16 @@ const CollectionView = Backbone.View.extend({
   // Remove the child view and destroy it. This function also updates the indices of later views
   // in the collection in order to keep the children in sync with the collection.
   _removeChildView(view) {
-    if (!view) { return view; }
+    if (!view || view._isDestroyed) {
+      return;
+    }
 
     this.triggerMethod('before:remove:child', this, view);
 
-    if (!view.supportsDestroyLifecycle) {
-      triggerMethodOn(view, 'before:destroy', view);
-    }
-
-    // call 'destroy' or 'remove', depending on which is found
     if (view.destroy) {
       view.destroy();
     } else {
-      const shouldTriggerDetach = !!view._isAttached;
-
-      if (shouldTriggerDetach) {
-        triggerMethodOn(view, 'before:detach', view);
-      }
-
-      view.remove();
-
-      if (shouldTriggerDetach) {
-        view._isAttached = false;
-        triggerMethodOn(view, 'detach', view);
-      }
-
-      view._isDestroyed = true;
-    }
-
-    if (!view.supportsDestroyLifecycle) {
-      triggerMethodOn(view, 'destroy', view);
+      destroyBackboneView(view);
     }
 
     delete view._parent;
@@ -514,8 +487,6 @@ const CollectionView = Backbone.View.extend({
 
     // decrement the index of views after this one
     this._updateIndices(view, false);
-
-    return view;
   },
 
   // check if the collection is empty or optionally whether an array of pre-processed models is empty

src/mixins/view.js

@@ -1,12 +1,16 @@
 // ViewMixin
 //  ---------
 
-import Backbone           from 'backbone';
-import _                  from 'underscore';
-import getUniqueEventName from '../utils/getUniqueEventName';
-import MarionetteError    from '../error';
-import View               from '../view';
-import { triggerMethod }  from '../trigger-method';
+import Backbone from 'backbone';
+import _ from 'underscore';
+import MarionetteError from '../error';
+import BehaviorsMixin from './behaviors';
+import CommonMixin from './common';
+import DelegateEntityEventsMixin from './delegate-entity-events';
+import TriggersMixin from './triggers';
+import UIMixin from './ui';
+import View from '../view';
+import { triggerMethod } from '../trigger-method';
 
 var ViewMixin = {
   supportsRenderLifecycle: true,

src/region.js

@@ -7,6 +7,7 @@ import isNodeAttached from './utils/isNodeAttached';
 import MarionetteObject from './object';
 import MarionetteError from './error';
 import MonitorViewEvents from './monitor-view-events';
+import destroyBackboneView from './utils/destroyBackboneView';
 import { triggerMethodOn } from './trigger-method';
 
 const Region = MarionetteObject.extend({
@@ -85,7 +86,7 @@ const Region = MarionetteObject.extend({
 
   _attachView(view) {
     const shouldTriggerAttach = !view._isAttached && isNodeAttached(this.el);
-    const shouldReplaceEl = !!this.replaceElement;
+    const shouldReplaceEl = !!this.getOption('replaceElement');
 
     if (shouldTriggerAttach) {
       triggerMethodOn(view, 'before:attach', view);
@@ -183,8 +184,7 @@ const Region = MarionetteObject.extend({
 
   // Destroy the current view, if there is one. If there is no current view, it does
   // nothing and returns immediately.
-  empty({preventDestroy} = {}) {
-    const shouldPreventDestroy = !!preventDestroy;
+  empty(options) {
     const view = this.currentView;
 
     // If there is no view in the region we should not remove anything
@@ -197,10 +197,8 @@ const Region = MarionetteObject.extend({
       this._restoreEl();
     }
 
-    if (shouldPreventDestroy) {
-      this._detachView(view);
-    } else {
-      this._destroyView(view);
+    if (!view._isDestroyed) {
+      this._removeView(view, options);
     }
 
     delete this.currentView._parent;
@@ -210,6 +208,21 @@ const Region = MarionetteObject.extend({
     return this;
   },
 
+  _removeView(view, {preventDestroy} = {}) {
+    const shouldPreventDestroy = !!preventDestroy;
+
+    if (shouldPreventDestroy) {
+      this._detachView(view);
+      return;
+    }
+
+    if (view.destroy) {
+      view.destroy();
+    } else {
+      destroyBackboneView(view);
+    }
+  },
+
   _detachView(view) {
     const shouldTriggerDetach = !!view._isAttached;
 
@@ -225,40 +238,6 @@ const Region = MarionetteObject.extend({
     }
   },
 
-  // Call 'destroy' or 'remove', depending on which is found on the view (if showing a raw
-  // Backbone view or a Marionette View)
-  _destroyView(view) {
-    if (view._isDestroyed) { return; }
-
-    if (!view.supportsDestroyLifecycle) {
-      triggerMethodOn(view, 'before:destroy', view);
-    }
-    if (view.destroy) {
-      view.destroy();
-    } else {
-      const shouldTriggerDetach = !!view._isAttached;
-
-      if (shouldTriggerDetach) {
-        triggerMethodOn(view, 'before:detach', view);
-      }
-
-      view.remove();
-
-      if (shouldTriggerDetach) {
-        view._isAttached = false;
-        triggerMethodOn(view, 'detach', view);
-      }
-
-      // appending _isDestroyed to raw Backbone View allows regions to throw a
-      // ViewDestroyedError for this view
-      view._isDestroyed = true;
-    }
-
-    if (!view.supportsDestroyLifecycle) {
-      triggerMethodOn(view, 'destroy', view);
-    }
-  },
-
   // Checks whether a view is currently present within the region. Returns `true` if there is
   // and `false` if no view is present.
   hasView() {

src/trigger-method.js

@@ -48,16 +48,3 @@ export function triggerMethodOn(context, ...args) {
   var fnc = _.isFunction(context.triggerMethod) ? context.triggerMethod : triggerMethod;
   return fnc.apply(context, args);
 }
-
-// triggerMethodMany invokes triggerMethod on many targets from a source
-// it's useful for standardizing a pattern where we propagate an event from a source
-// to many targets.
-//
-// For each target we want to follow the pattern
-// target.triggerMethod(event, target, ...args)
-// e.g childview.triggerMethod('attach', childView, ...args)
-export function triggerMethodMany(targets, eventName, ...args) {
-  _.each(targets, function(target) {
-    triggerMethodOn(target, eventName, target, ...args);
-  });
-}

src/utils/destroyBackboneView.js

@@ -0,0 +1,26 @@
+import { triggerMethodOn } from '../trigger-method';
+
+export default function destroyBackboneView(view) {
+  if (!view.supportsDestroyLifecycle) {
+    triggerMethodOn(view, 'before:destroy', view);
+  }
+
+  const shouldTriggerDetach = !!view._isAttached;
+
+  if (shouldTriggerDetach) {
+    triggerMethodOn(view, 'before:detach', view);
+  }
+
+  view.remove();
+
+  if (shouldTriggerDetach) {
+    view._isAttached = false;
+    triggerMethodOn(view, 'detach', view);
+  }
+
+  view._isDestroyed = true;
+
+  if (!view.supportsDestroyLifecycle) {
+    triggerMethodOn(view, 'destroy', view);
+  }
+}

src/view.js

@@ -5,9 +5,6 @@ import _                  from 'underscore';
 import Backbone           from 'backbone';
 import ViewMixin          from './mixins/view';
 import RegionsMixin       from './mixins/regions';
-import BehaviorsMixin     from './mixins/behaviors';
-import UIMixin            from './mixins/ui';
-import CommonMixin        from './mixins/common';
 import MonitorViewEvents  from './monitor-view-events';
 import Renderer           from './renderer';
 

test/unit/collection-view.spec.js

@@ -851,27 +851,22 @@ describe('collection view', function() {
 
   describe('when removing a childView that does not have a "destroy" method', function() {
     beforeEach(function() {
+      const collection = new Backbone.Collection([{id: 1}]);
       this.collectionView = new this.CollectionView({
         childView: Backbone.View,
-        collection: new Backbone.Collection([{id: 1}])
+        collection
       });
 
       this.collectionView.render();
 
       this.childView = this.collectionView.children.findByIndex(0);
       this.sinon.spy(this.childView, 'remove');
-
-      this.sinon.spy(this.collectionView, '_removeChildView');
-      this.collectionView._removeChildView(this.childView);
+      collection.reset();
     });
 
     it('should call the "remove" method', function() {
       expect(this.childView.remove).to.have.been.called;
     });
-
-    it('should return the childView', function() {
-      expect(this.collectionView._removeChildView).to.have.returned(this.childView);
-    });
   });
 
   describe('when destroying all children', function() {