Comments on Marionette revisions #387

Closed
dcmaf opened this Issue Mar 25, 2013 · 2 comments

Projects

None yet

2 participants

@dcmaf
Collaborator
dcmaf commented Mar 25, 2013
  1. I think the memory leak discussion can be simplified since the potential for a leak has already been discussed in the Events section of the Backbone Basics chapter and listenTo/stopListening are introduced there as a solution to the problem. There really isn't even a need to introduce a close() function since Backbone.remove() will call stopListening.

    The real benefit that Marionette provides here seems to be the Region concept and how a region implicitly closes/removes an old view when a new one is attached to the region. I think reducing the section down to just showing how a zombie listener can occur while using listenTo (if you let a view go out of scope without calling remove()) is sufficient to explain the problem and set up how regions provide an elegant solution. I would also just use listenTo throughout the example rather than starting with on() and then changing to listenTo.

  2. At the end of the Region Management section it states that nothing will be called if the view doesn't have a close or remove method. Wouldn't any valid Backbone.View have a remove method since it should be inherited from View (unless they went out of their way to override it and not call the base method or removed it entirely)? If so, then this last sentence can probably be deleted.

@addyosmani
Owner

I'm happy to try addressing this tomorrow. Including @derickbailey here in case he has any further comments on the Marionette section and suggestions above.

@addyosmani
Owner

PS: Thanks a lot for reviewing!

@raDiesle raDiesle added a commit to raDiesle/backbone-fundamentals that referenced this issue Mar 29, 2013
@addyosmani @raDiesle + raDiesle For #387 - addresses point 2, references earlier chapter for listenTo…
…/stopListening
5e6bee9
@addyosmani addyosmani closed this Apr 16, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment