Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Providing a bare bones convention for nested views #2490

Closed
tgriesser opened this Issue Apr 17, 2013 · 74 comments

Comments

Projects
None yet
@tgriesser
Copy link
Collaborator

tgriesser commented Apr 17, 2013

The biggest source of questions/complaints/issues I've encountered with Backbone deals with handling the issue of nested views. Any sufficiently complex application (see: real world applications) has a need for managing views within views, and cleaning up after them properly. The (well received) addition of listenTo helps with this to a degree, but I think Backbone could do a bit more.

This is something I add to pretty much whatever I'm working with, and it does the job without much overhead or complexity:

var View = Backbone.View;
Backbone.View = Backbone.View.extend({

  constructor: function() {
    this.subviews = [];
    View.apply(this, arguments);
  },

  addSubview: function(view) {
    if (!(view instanceof Backbone.View)) {
      throw new Error("Subviews must be a Backbone.View");  
    }
    this.subviews.push(view);
    return view;
  },

  removeSubviews: function() {
    var children = this.subviews;
    for (var i = 0, l = children.length; i<l; i++) {
      children[i].remove();
    }
    this.subviews = [];
    return this;
  },

  remove: function() {
    this.removeSubviews();
    View.prototype.remove.apply(this, arguments);
  }
});

I completely understand and agree with Backbone's position to keep things simple and not implement applications specific components; at the same time I think providing a very, very basic starting point for this problem would be a great help to many.

This starting point could then be extended upon with more opinionated libraries or frameworks like layoutmanager, thorax, marionette, etc.

/cc @tbranyen

@jiyinyiyong

This comment has been minimized.

Copy link

jiyinyiyong commented Apr 17, 2013

+1 Nesting views is so much complex thing in Backbone.

@braddunbar

This comment has been minimized.

Copy link
Collaborator

braddunbar commented Apr 17, 2013

Mornin' Tim! While I can certainly understand the impetus for this change (I've written similar code many times), I don't think subview management is appropriate for inclusion.

Backbone Views are almost more convention than they are actual code.

Powerful primitives and good conventions to build upon. The code above is a rather small amount of boilerplate and doesn't address any hard issues like disposal and data binding that people mostly don't agree about. However, it implies that Backbone should handle those when it shouldn't. Further, many views don't contain other views and allocating an array for them is silly.

Leaving this unimplemented allows for creativity in solving hard problems and working with Backbone instead of around it.

@jashkenas

This comment has been minimized.

Copy link
Owner

jashkenas commented Apr 17, 2013

The other nasty thing about this pattern is that it stores references ... meaning that you now have to go back and manually remove your child views.

In a more agnostic setup, if the HTML for your subviews is removed at the same time that the models stop being referenced, it all just gets garbage collected for you. The more unnecessary references you have in JS, the less garbage collection gets to do its job. We should try to have as few as possible mandated by core Backbone.

@jashkenas jashkenas closed this Apr 17, 2013

@tgriesser

This comment has been minimized.

Copy link
Collaborator Author

tgriesser commented Apr 17, 2013

... allocating an array for them is silly.

Good point, this could be simplified to (this.subviews || this.subviews = []).push(view)

The other nasty thing about this pattern is that it stores references ... meaning that you now have to go back and manually remove your child views.

If you call remove on the top level view, it then calls remove on child views automatically, and you're good... it doesn't seem too nasty.

... working with Backbone instead of around it. ... as few as possible mandated by core Backbone.

Makes sense, and I agree... I really only brought it up because I've heard so many questions about it (probably just behind nested models/collections).

It might be worth mentioning something small in the docs about ways to deal with views within views, for the cases when the html for subviews aren't removed at the same time as other objects the view maintains references to. Any opinions on that?

@tbranyen

This comment has been minimized.

Copy link
Collaborator

tbranyen commented Apr 18, 2013

Allocating an array isn't silly, bringing it up as a valid reason for not doing something is though.

@braddunbar https://github.com/pathable/quilt/blob/master/quilt.js#L28 ?

@tbranyen

This comment has been minimized.

Copy link
Collaborator

tbranyen commented Apr 18, 2013

"Maintaining Views is, in my opinion, the most difficult part of a Backbone application because you must make sure that you properly dispose of your views, detach events, dereference objects, etc or you absolutely will run into memory leaks."

http://deserialized.com/javascript/our-experience-with-backbone-js-and-why-were-considering-angularjs-as-a-replacement/

This post is from today and there are many more like it. In my opinion it is not a contrived problem. Other than plugins and blog posts, what is the best way to make this library effective for creating Views to newcomers, in the same way that Models and Collections attempt to provide? I would love to work on Layout Manager to the point where it could be merged, but I fear it would be an effort wasted.

If you don't mind me asking, what would be the main objections, other than "we don't want it", that you'd have towards integrating well tested and properly handled View management?

@tgriesser

This comment has been minimized.

Copy link
Collaborator Author

tgriesser commented Apr 18, 2013

I think one of the main issues that @tbranyen touched upon is that Backbone views are typically promoted as jQuery, with better structure... keep your logic/state out of the DOM, all of that good stuff.

The problem is that when you're coming from $ land, you don't have to think about references and gc (almost) at all, the black box handles cleaning up references for you in a big way. With Backbone, there isn't much done for that outside of models/collections.

If you're dereferencing your models and views simultaneously (as @jashkenas often mentions), then you'll end up with more "loading" screens and server hits than your application may have needed if more data was kept around in memory... There are many valid cases where a view would be observing events on an object that would outlive it - or if subviews are added in a view's "render" block, referencing the parent's model, and the parent view is re-rendered, you now have views that hang around until the parent model is gone, unless you explicitly kept track of and removed the views at the beginning of the render block...

We should try to have as few as possible mandated by core Backbone.

Also, looking at this comment again I have to say that I agree fully, but I also think there's a big difference between "mandating" something and providing an outlet for handling something that's rather common and a major pain point and source of confusion, in a simple and entirely optional fashion.

If the 70 cloc Backbone "view" section is as complicated as it's going to get (vs. the 473 for models and collections, not counting underscore)... that's fine, but I can certainly understand why nested views aren't immediately obvious and have sprung so many blog posts, half baked plugins, and frustration.

@braddunbar

This comment has been minimized.

Copy link
Collaborator

braddunbar commented Apr 18, 2013

@tbranyen You're right, that wasn't a great reason and I shouldn't have mentioned it. I think my main point still stands though. I've been known to do all sorts of silly things, not that I recommend them all. :)

If you're dereferencing your models and views simultaneously (as @jashkenas often mentions), then you'll end up with more "loading" screens and server hits than your application may have needed if more data was kept around in memory

I've done this both ways and I've found it to be a trade off. Keeping things around in memory can improve perceived load time, but is often accompanied by a high cost in complexity and cache management (leaving view disposal/management out altogether is a huge win). Both are valid strategies, and the choice depends on your constraints.

@jashkenas jashkenas reopened this Apr 18, 2013

@jashkenas

This comment has been minimized.

Copy link
Owner

jashkenas commented Apr 18, 2013

I still don't think that we should be merging a change like this. To repeat myself -- if we maintain references to subviews in Backbone core, then suddenly lots of folks that previously didn't have any problems working with GC properly now have to start manually removing every subview ... a feature regression -- and a possible initial step down the (possibly) wrong path for beginners.

That said, there are of course many cases where you do want this style of view hierarchy. Let's solve this with documentation. Anyone want to cook up a nice "Nested Views" section for the homepage? Talking briefly about 1) how to make your views flow nicely with GC, and then 2) how to maintain an explicit hierarchy with automatic granular unbinding on remove, demonstrating the code above?

@tgriesser

This comment has been minimized.

Copy link
Collaborator Author

tgriesser commented Apr 18, 2013

That'd be great - I can take a shot at putting together some of those docs.

@jashkenas

This comment has been minimized.

Copy link
Owner

jashkenas commented Apr 18, 2013

In addition, the docs needn't include all of the code above. Maybe make it a tgriesser/backbone-nested official plugin, and link over to the reference implementation?

@dgbeck

This comment has been minimized.

Copy link

dgbeck commented Apr 18, 2013

Is the above code intended to manage a fixed number of subviews or to manage a variable number of subviews? In my opinion those are fundamentally two different beasts and they warrant totally separate solutions from the start.

If you split up a section of UI into separate subviews for organizational / code reuse purposes, then you have a fixed number of subviews. It is useful to reference those subviews with names. A hash keyed by subview name is most appropriate data structure for storing the subviews in this case.

In contrast, if you are rendering a collection, then you have an arbitrary number of subviews and an array is more appropriate for storing these subviews.

The code above is not a sufficient solution for either case, IMO. The first case is simpler, but even with that case, there is one question that immediately arise and need to be addressed. Are the subviews re-initialized when the parent is re-rendered, or are they just re-rendered? For most cases you just want them to be re-rendered. But it is non-trivial to replace the parent HTML without loosing the events that are attached to the subview DOM elements.

The "minimal" solution for rendering a fixed number of subviews that we have been able to distill is the Backbone.Subviews mixin. Any solution that does not address the re-rending issue, IMO, is incomplete.

The case of a variable number of subviews is another matter. I will not go into that more complicated case but to add the new Backbone.CollectionView plugin as one of the available solutions.

@tgriesser

This comment has been minimized.

Copy link
Collaborator Author

tgriesser commented Apr 18, 2013

Are the subviews re-initialized when the parent is re-rendered, or are they just re-rendered?

I actually find that in the majority of cases if the parent is re-rendered, it is easiest to both re-initialize and re-render the view... in which case the above should work just fine (as long as you call removeSubviews in the topmost render)

When splitting up the UI, I many times won't use a hash or named subviews, as the subviews will re-render themselves based on model changes, etc so I don't need to reference them from the parent at all... But I guess this just confirms what Jeremy said, that there is no one size fits all scenario for view management, and is something that shouldn't be addressed of in Backbone.

@wyuenho

This comment has been minimized.

Copy link
Contributor

wyuenho commented Apr 28, 2013

if we maintain references to subviews in Backbone core, then suddenly lots of folks that previously didn't have any problems working with GC properly now have to start manually removing every subview ... a feature regression -- and a possible initial step down the (possibly) wrong path for beginners.

@jashkenas I don't quite understand this sentiment. Suppose you only have to one view, the proper way to remove it other than closing the tab is to call View#remove. You have to document this somewhere somehow with a big bold letter that says DONT JUST USE $.fn.remove() and $.fn.empty() OR GET MEM LEAKS anyway. @tgriesser 's suggested change is a good start and actually will simplify memory management by automatically propagating the remove call.

I don't know how many beginner's View code you have seen, but from what I've seen, beginners will absolutely, definitively, not get it (it being the so-called structure). For people whose brains had been warped by jQuery for half a decade, the jQuery chain IS the structure. They don't like it, so they look for guidance in Backbone but Backbone offers next to no help and is refusing to do so. The single most likely place to induce partially digested stomach content ejection jQuery code is the lack of a blessed way to nest views. They will put code that belongs to the subviews in the parent view, implement half-ass solutions to propagate Backbone events from deeply nested subviews to the ancestors and many times just a big blob of jQuery pasta within render because they don't know how to deal with subviews. After about 2 years with Backbone, a lot of people are starting to converge towards the solutions suggested herein. I sincerely hope you can reconsider trying to hash out a View class that solves most of the problems people have with the current implementation.

It is useful to reference those subviews with names. A hash keyed by subview name is most appropriate data structure for storing the subviews in this case.

@dgbeck Actually a hash is a sufficient and appropriate solution for all situation since if you are rendering a collection of views, the keys can simple be the index. You just have to make sure the subviews come out in the right order. @caseywebdev et al. 's book actually suggests using a hash.

Are the subviews re-initialized when the parent is re-rendered, or are they just re-rendered? For most cases you just want them to be re-rendered. But it is non-trivial to replace the parent HTML without loosing the events that are attached to the subview DOM elements.

Most cases the parent will just call $.fn.empty() and re-render the subviews, and in each subview's render method, the DOM events will be re-delegated. Yes this is a lot of work (as in CPU cycles) done and makes Backbone's View even more tightly bound to jQuery. I'd love to see a clean implementation that actually downgrades jQuery importance to be side-by-side with just the DOM API for many reasons, but that's a whole other blog posts...

@steverandy

This comment has been minimized.

Copy link

steverandy commented Apr 28, 2013

Actually backbone could just use "views" hash to track child views and provide few extra methods to make view management easier. This is what I did. https://github.com/steverandy/backbone.managedview/blob/master/backbone.managedview.coffee

@RStankov

This comment has been minimized.

Copy link
Contributor

RStankov commented Apr 28, 2013

I am actually interested in what approaches people are using in real software, not theory realms. Since a lot of people are solving this problems, and there are probably similar patterns. I have used two patterns, which I liked:

Backbone.Support approach, I like like it because the view rendering is included in the inclusion of subview.

The other way I have used subviews is via my own Backbone.Handlebars /which I don't try to promote or anything/. But the idea there is to hide the subviews into the templating engine. There is a method to access the subview via a obscure renderedSubViews, but I can't recall ever use it.

@jashkenas One possible solution about removing a subview from imaginary this.subviews is to extend Backbone.View with Backbone.Events and have rendered, removed, subview:added, subview.removed and so events. I understand that subview rendering is very very dependent on the tempting engine. But for me the one biggest Backbone weaknesses (and strengths) is that it doesn't have a clear template engine pipeline.

@wyuenho

This comment has been minimized.

Copy link
Contributor

wyuenho commented Apr 28, 2013

@RStankov Subview rendering does not depend on template engines at all. You can perfectly nest views, where each view only represents 1 HTML element constructed by jQuery in View's constructor. Backbone.Support does not address @dgbeck 's issues because it uses an array. Sometimes you need to be able to name your subviews. I also don't see how giving extra events is going to help as there's not an obvious way to construct a view hierarchy in Backbone. Backbone events also don't bubble like the DOM's. Events are only useful when all your subviews are sharing the same model/collection.

If you want to look at real world code, here's Backgrid. The whole thing is entirely composed of nested views using a slightly less elegant variation of @tgriesser 's solution. This is sufficient only because I happen to have no need to name my subviews, but many other situations will certainly call for a hash. It really should be the default way to go as Underscore can trivially make a hash to behave like an array.

@RStankov

This comment has been minimized.

Copy link
Contributor

RStankov commented Apr 29, 2013

@wyuenho Nice grid :)

My point was mainly that we should look for patterns used in projects, since I noticed everybody handling this is a different way. And I'm more and more moving to have subviews in template helper and see the subviews in the markup not in the view code. But I understand this my pattern and is not the right solution for generic stuff.

The need for view event like removed is that if you have a subview. And its model is removed this triggers removing of the subview, it will be removed from DOM, but will still be collection of its parent view. The other way is to have reference for parent view into the subview and upon removal to remove the child from parent.

Most of the places I take heavy use of subviews, the hard part is creating them and putting them into the correct DOM places. Then they self managed, and the way they communicated with their parent was with custom DOM events. And I can't remember a case where the parent needed to call a method of a subview.

@acstll

This comment has been minimized.

Copy link

acstll commented May 2, 2013

@RStankov I'd like to follow your comment and share the way I handle nested views. Backbone.View already inherits Events.

I add or mix in these to methods to all my views I know will be nested/parents:

dispose: function () {
  this.trigger('dispose', this.cid);
  this.remove();
},

register: function () {
  if (this.options.parent) {
    this.listenTo(this.options.parent, 'dispose', this.dispose);
  } else {
    console.log('Tried to register child view with no parent:', this);
  }
}

Then whenever I instantiate a 'child' view inside another view I pass the parent as an argument { parent: this }. And on the child view's initialize function I call the register method.

No hash or anything. Every child view holds a reference to its parent and listens for a 'dispose' event. When a parent view gets disposed, all child views do so as well.

@dgbeck

This comment has been minimized.

Copy link

dgbeck commented May 2, 2013

Catching up on this thread -

Most cases the parent will just call $.fn.empty() and re-render the subviews, and in each subview's render method, the DOM events will be re-delegated.

@wyuenho, DOM events are delegated when views are initialized, not when they are rendered. You will loose your DOM events on subviews if you simply re-render subviews when the parent is re-rendered. A work around is to detach them and then reattach them to the DOM. (If you take the other approach discussed, re-initializing views after the parent is rendered, you loose all state data in the subviews, for which I know of no easy work around.)

I agree with you that keeping references to subviews within the parent should not be a deal breaker as long as subviews are removed when .remove() is called on the parent. @jashkenas this seems pretty low cost as it is already a requirement that people call .remove() in order to that their views be properly garbaged collected.

I also don't see how giving extra events is going to help as there's not an obvious way to construct a view hierarchy in Backbone. Backbone events also don't bubble like the DOM's.

Try Backbone.Courier. You'll like it, I promise =).

@RStankov your good point about needing an event to remove a subviews from the parent only applies to case (2) below, I think. In case (1) subviews are almost never removed, and when they are one could just re-render the parent and the parent could then drop any old references to removed subviews.

@acstll if the approach you described works for you then go for it but keeping explicit references to a parent view in a child view is generally not a good idea.

To recap:

  • There are two common cases when subviews are used:
    1. When splitting up a view into smaller regions for organization and / or view reuse.
    2. When rendering a collection of models.
  • I'm not sure that including a section in the documentation about managing subviews is the best route because both cases have significant complexities that average backbone users shouldn't need to understand.
  • IMO a solution might be able to be hashed together and polished enough to be included in backbone core for case (1), which would provide a lot of value on its own. Case (2) I think definitely needs to be left for plugins.

Thanks @tgriesser for starting this thread.

@tgriesser

This comment has been minimized.

Copy link
Collaborator Author

tgriesser commented May 2, 2013

Well typically 1 and 2 go hand in hand, you're splitting up a view into smaller components because you have different models/collections for organization/code re-use.

Just to illustrate, this is the classic example of an incorrect implementation (going off the nested models/collections example in Backbone docs) of where I think Backbone could provide a little bit of guidance, whatever that may be:

class MailboxView extends Backbone.View
   initialize: ->
      @listenTo(@model, 'change', @render)
   render: ->
      @$el.html JST['/some/template'](@model.toJSON())
      @$('.some-class').append(new ListView(collection: @model.messages).render().el)
      this

class ListView extends Backbone.View
   initialize: ->
      @listenTo(@collection, 'add', @addItem)
      @listenTo(@collection, 'reset', @render)
   addItem: (m) ->
      @$el.prepend(new MessageView(model: m).render().el)
   render: ->
      @collection.each (m) =>
        @$el.append(new MessageView(model: m).render().el)
      this

class MessageView extends Backbone.View
   initialize: ->
      @listenTo(@model, 'change', @render)
      @listenTo(@model, 'destroy', @remove)
   render: ->
      @$el.html JST['/some/template'](model: @model)
      this

So here, imagine the mailbox model changes, maybe you changed the "last_fetched" attribute on it, but the collection remains intact as a property on that model - thus the collection/ associated models outlive their views...

It won't make a huge difference here because there's not much going on in the child views' render, but it ends up being an issue in a lot of cases, and leads to frustration if you don't understand why.

This would then be easily solved, by adding @removeSubviews() in the top level render function, and by wrapping each child view's constructor with @addSubview.

I didn't write up the docs for it yet because I wanted to hear a bit more discussion, but here is the plugin I'm thinking should handle a good bit of this confusion... it's really only two very simple functions, though... I still think it is something that could be considered for addition to Backbone directly.

Also, looking back through the thread at @braddunbar's comment...

Leaving this unimplemented allows for creativity in solving hard problems and working with Backbone instead of around it.

In my opinion, this is such a boilerplate situation, I don't see why it needs to a) be a hard problem and b) be solved creatively by each plugin differently, each time. If it needs to be done creatively, nothing is forcing this implementation, but I think Backbone could provide something for the 90%+ case.

P.S. - @dgbeck nice stackoverflow link (asked by me, about a year and a half ago)... It wasn't immediately obvious how view relationships should be handled when getting started (the answer being, there isn't really an answer), which is probably why I brought it up.

@acstll

This comment has been minimized.

Copy link

acstll commented May 2, 2013

@dgbeck , @tgriesser thanks. I'm glad about posting my aproach, because it demonstrates there's some of us doing things wrong after so long. I agree:

If it needs to be done creatively, nothing is forcing this implementation, but I think Backbone could provide something for the 90%+ case.

@wyuenho

This comment has been minimized.

Copy link
Contributor

wyuenho commented May 3, 2013

DOM events are delegated when views are initialized, not when they are rendered. You will loose your DOM events on subviews if you simply re-render subviews when the parent is re-rendered. A work around is to detach them and then reattach them to the DOM. (If you take the other approach discussed, re-initializing views after the parent is rendered, you loose all state data in the subviews, for which I know of no easy work around

@dgbeck I know :) What I mean is when I re-render a parent view I usually do something like this:

render: function () {
  this.$el.empty();
  this.removeSubviews();
  // ...
  // render parent view..
  // ...
  this.renderSubviews();
  this.delegateEvents();
}

This, of course, only works if all the subviews inherit from a View superclass that implements this.

The way communication goes in my view hierarchy is that I always make sure they all share the same collection/model and I simply trigger custom namespaced events from the shared collection/model like so:

model.trigger("namespace:event", param1, param2, ...);

Interested parties can just listen to those events and act accordingly, so in a sense the collection/model is a reversed-DOM. I've never found the need to set up an event chain among my views or abuse the actual DOM to propagate Backbone events.

I agree with @tgriesser that your 2 cases typically go hand in hand.

I happen to be working on two projects that use nested views quite heavily and have two custom View classes. One is similar to @tgriesser 's backbone-nested, but uses both a hash and an array to keep track of subviews. Another one offers a jQuery-free mode but 100% compatible with the existing View, but I digress... Might open source some plugins and file some PRs this weekend...

@wyuenho

This comment has been minimized.

Copy link
Contributor

wyuenho commented May 3, 2013

BTW, @jashkenas is there policy in this project that sets up a path to merge good plugins (for some definition of good) into the core or is the policy once a plugin, always a plugin?

@jashkenas

This comment has been minimized.

Copy link
Owner

jashkenas commented May 3, 2013

I'd like to think that there are no policies -- at least formulated as such...

If a plugin is a good idea for core, then we should merge it. 'Nuff said.

@dgbeck

This comment has been minimized.

Copy link

dgbeck commented May 5, 2013

@tgriesser thanks for the repo. Definitely helps make the discussion more concrete.

There are two problems with the code as it is that would prevent us from using it, even for the simpler case of splitting up a view into smaller subviews.

First, we find it useful to name subviews. In the above Mailbox example, what if the MailboxView wants to listen to an event on the ListView? Or what if it wants to tell the ListBox view to collapse all messages when a collapse button the MailboxView is clicked? In this case it is useful to be able to reference it by name:

this.listenTo( this.subviews.listView, "messageSelected", this._onMessageSelected ); 

or

this.subviews.listView.collapseAllMessages(); 

This problem can be solved easily by storing the subviews as a hash. Here is a fork of the code with one incarnation of that change. In this fork people who want to supply the keys for their subviews can do so, and if no keys are supplied the behavior will fall back to using numbered indexes as hash keys.

Second, our views have state information and we can't loose that information when the parent view is re-rendered. (Nor does it seem very efficient to re-initialize subviews each time a parent view is re-rendered.) Are we the only ones who store state information on our views? For instance, again using the above example, the listView might contain state information about which message(s) are selected. Storing that information as a selected attribute of on messages themselves does not seem appropriate to me because it is not persisted to the server nor shared among different users. In my mind this kind of temporary "view state" information belongs in the view layer, not in models.

This second problem is harder to solve simply but maybe it could be left for plugins. I could not find a way distill a solution further than Backbone.Subviews. However, if hash version of backbone-nested was integrated into core we could re-write the Backbone.Subviews plugin on top of it.

Thanks again for the repo and for your efforts in helping people through this common issue.

@tgriesser

This comment has been minimized.

Copy link
Collaborator Author

tgriesser commented May 5, 2013

So to address those two points...

  1. I agree that named views are good to have... but in many cases you don't need them... Because addSubview returns the view that was added, you can easily attach the subview to the parent view like so:
this.listView = this.addSubview(new ListView(collection: this.model.messages))
this.$('.some-class').append(this.listView.render().el)

In my opinion this feels a bit cleaner (this.listView vs this.subviews.listView)... and still maintains the same necessary cleanup needed with removeSubviews.

  1. In this case, since you're dependent on each of the views maintaining their own state, you probably just wouldn't want to let parent view(s) re-render at all... any only really use them for the structure they provide, but keep all of the child views as siblings and let them re-render independently. Then you don't have to worry about any of that.
@wyuenho

This comment has been minimized.

Copy link
Contributor

wyuenho commented May 17, 2013

@philfreo You could argue the same for Collection no? Those methods never existed on View, why would they be confused? You can still do anything you want with this change.

@philfreo

This comment has been minimized.

Copy link
Contributor

philfreo commented May 17, 2013

A collection is specifically meant to hold models.

I'm not against subview management per se, just pointing out that View#add isn't completely obvious and without possibility for confusion, due to the fact that the primary purpose of a View isn't to hold other views.

@wyuenho

This comment has been minimized.

Copy link
Contributor

wyuenho commented May 17, 2013

I see. I'm fine with either way actually.

@acstll

This comment has been minimized.

Copy link

acstll commented May 17, 2013

…just pointing out that View#add isn't completely obvious and without possibility for confusion, due to the fact that the primary purpose of a View isn't to hold other views.

@wyuenho @philfreo Exactly. But also that "adding" could lead beginners to think about rendering or even the DOM. When the case here is just keeping a reference in a hash. And this is what I wanted to point out, nothing else. :)

@andreypopp

This comment has been minimized.

Copy link

andreypopp commented May 17, 2013

@wyuenho @acstll @philfreo I asked only about dropping sub suffix — so I'm ok with addView, removeView instead of addSubview, removeSubview.

@dgbeck

This comment has been minimized.

Copy link

dgbeck commented May 17, 2013

@wyuenho, bummer! In what order do you need to keep the views? Are they for splitting a view into a fixed number of subviews or rendering a collection?

@andreypopp

This comment has been minimized.

Copy link

andreypopp commented May 17, 2013

@dgbeck @wyuenho I think we need both use cases

see my take on the issue
View subclass with subviews stored in hash — https://github.com/andreypopp/backbone.viewx/blob/master/src/view.coffee#L53
CollectionView which subclasses Backbone.Viewx.View and also stores subviews in array — https://github.com/andreypopp/backbone.viewx/blob/master/src/collectionview.coffee#L3

@andreypopp

This comment has been minimized.

Copy link

andreypopp commented May 17, 2013

@dgbeck @wyuenho but CollectionView is out of scope of this discussion so the examples above just can provide a proof that view with subviews stored in a hash can be a suitable building block for further abstractions like CollectionView.

@wyuenho

This comment has been minimized.

Copy link
Contributor

wyuenho commented May 17, 2013

I asked only about dropping sub suffix — so I'm ok with addView, removeView instead of addSubview, removeSubview.

I agree that addView and removeView are fine, addressing @philfreo 's point, just add may not provide sufficient information to what it is exactly you are adding, but addSubview and removeSubview are redundant. When you are adding a view into a view, the new view is implicitly a subview.

@andreypopp @dgbeck I'm not sure talking about CollectionView is out of the scope of this discussion, I mainly use subviews to render collection elements. I haven't had too many occasions to split large views into smaller views. I suppose that could happen quite often also but just that I'm biased.

The second example on backgrid could easily be built using a top level view but I still wouldn't keep the components inside a hash. I'll just attach the filter, grid and the paginator inside instance vars this.filter, this.grid and this.paginator and set them up inside the constructor, but I suppose keeping them inside a hash with an explicit name can minimize name collisions, however unlikely that maybe.

Heh I feel like I'm backtracking from my previous position of using a hash as the backing store. When I split a large view into subviews I'd name my subcomponents using instance vars to convey their purposes.

I think the choice of data structure really comes down to how you would want to render the subviews.

For the case of "splitting a view" as @dgbeck termed it (I would use the term "heterogeneous subviews"), hash is more convenient as you'd want to be able to get them out individually, render and attach them to well defined points inside a template (a sufficiently smart solution will/can keep the ids in the template the same as the names of the subviews so you can just loop over the hash).

For the case of "rendering a collection" (homogeneous subviews), array is more natural as you can easily loop over them and just append the rendered subviews to this.el. Names aren't important individually to each subview in this case, but collectively, the collection of subviews may need to be named, such as a body of class Body has a list of rows of class Row.

If we classify subview use cases this way, it does seem like the 2 are mutually exclusive (under what circumstances would you require both?).

@wyuenho

This comment has been minimized.

Copy link
Contributor

wyuenho commented May 17, 2013

Actually, for the homogeneous subview case, a hash would also be fine but to keep the ordering, the keys would have to be the model cids inside a collection attached to the parent. It would be nice if there was a default render implementation that can render both cases automatically that people can still override. An even more flexible implementation can of course make use of nested models as well, but that's a whole other story.

@akre54

This comment has been minimized.

Copy link
Collaborator

akre54 commented May 17, 2013

I agree with @wyuenho's earlier point that storing subviews as both an array and a hash would be desirable. I've worked on several projects where maintaining the order of subviews is vital and a hash alone wouldn't work (mostly for subsections that need to be processed in a particular order).

Chaplin's take on this is to overload View#subview to be an accessor and a setter, and then also expose both the subviewsByName hash and the subviews array on the view instance. Chaplin's concept allows specifing an arbitrary name key into the subviewsByName hash, but mostly I've found keying by cid to work better.

var parentView = new ParentView();
var childView = new ChildView();

// Set
parentView.subview(childView); // when passed a View, stores it

// Retrieve
parentView.subview('view23').on('complete', this.closeForm); // when passed a string, retrieves by cid

// Iterating
_.each(parentView.subviews, function(v) {
  v.trigger('complete');
});


// Remove
parentView.removeSubview('view23');
parentView.removeSubview(childView); // equivalent: pass either the view instance or the key
@dgbeck

This comment has been minimized.

Copy link

dgbeck commented May 17, 2013

I've worked on several projects where maintaining the order of subviews is vital and a hash alone wouldn't work (mostly for subsections that need to be processed in a particular order).

@akre54 sounds like this was for the "fixed number of heterogeneous subviews" case? Can you elaborate on what order the subviews needed to be iterated through?

@jashkenas

This comment has been minimized.

Copy link
Owner

jashkenas commented May 17, 2013

As commented (far, far) above, I'd still love to have a better section in the docs about this. But I'm afraid that I still don't think we should be baking in this patch to Backbone core. Fundamentally, it's basically just a bit of code around an array -- and if asking a programmer to keep a list of views in an array or in a hash is too much to ask, then they're probably not going to get very far building their app in the first place.

If we were going to add a patch to Backbone core to make removal of subviews more automatic, it should probably merely look something like this (in View):

remove: function() {
  if (this.subviews) _.invoke(this.subviews, 'remove');
  ... rest of current implementation continues here ...
}

Then it would handle both arrays and hashes, and you could optionally be registering your subviews in whatever way you see fit.

That said, I still think that including that line in Backbone would be starting to lead folks down an unfortunate path. Manually removing all subviews and all event listeners on those views is a great way to seriously hurt performance when you have a decently complex UI. That said, if you have to do it, then you have to do it.

But the alternative -- going with the flow and structuring your object lifecycles so that only your top-most view needs to be removed ... and perhaps a collection needs to be reset, and then a whole swatch of views and models are simply GC'd as Eich and Nature intended, is a more pleasing path.

@wyuenho

This comment has been minimized.

Copy link
Contributor

wyuenho commented May 17, 2013

if asking a programmer to keep a list of views in an array or in a hash is too much to ask, then they're probably not going to get very far building their app in the first place.

@jashkenas I think the problem is more like that beginners won't even start thinking about array and hash until they are really tired of creating DOM subtrees, attaching events to them the same old way with jQuery inside render and start wondering why they chose to use Backbone in the first place. I may not be a competent UI dev, but I don't think I'm so stupid that I'll have to try 3 or 4 times to get to the point where I could say mmm I really need an array or a hash and some loops to manage my views, but that's what happened. I love Backbone.Model and Backbone.Collection, but I feel that Backbone.View is designed only for devs in small shops who only have to churn out some small 5 pagers that are mostly static marketing content, who just happen to occasionally have to localize their messy jQuery code inside a couple of render methods that only serve as their prep area to call out to other jQuery plugins.

Manually removing all subviews and all event listeners on those views is a great way to seriously hurt performance when you have a decently complex UI.

Again, I think that GC is just a tiny subproblem for a much bigger problem - there's just not enough basic help provided by Backbone.View. Anyone that uses Backbone.View will sooner or later try to nest views. You just can't expect Backbone.View to be a sufficient foundation to a large and complex dashboard console UI.

Judging from the numerous code sample people have submitted so far in this thread, a common theme is already starting to emerge, instead of having everyone else solve the same problems again and again using numerous incompatible way, why not try to make some decisions and bake that into the core? Isn't the purpose of a library to provide solutions to common problems such that apps built on top of it can keep their code DRY?

But the alternative -- going with the flow and structuring your object lifecycles so that only your top-most view needs to be removed ... and perhaps a collection needs to be reset, and then a whole swatch of views and models are simply GC'd as Eich and Nature intended, is a more pleasing path.

I'd say that's a much more pleasant way to code a modern, complex web UI instead of just mostly jQuery.

@philfreo

This comment has been minimized.

Copy link
Contributor

philfreo commented May 17, 2013

mmm I really need an array or a hash and some loops to manage my views, but that's what happened

you don't need a hash or loop to (properly, including events) manage subviews, it's just the way you would implement it if you wanted to automate the process to reduce each subview from needing 3 lines of code (initialize, render, remove), to 1 line of code (addSubview)

Backbone.View is designed only for devs in small shops who only have to churn out some small 5 pagers that are mostly static marketing content

the fact that backbone.view has been so opened ended is precisely because it's NOT just for people who need to do simple stuff.

@jashkenas

This comment has been minimized.

Copy link
Owner

jashkenas commented May 17, 2013

Perhaps to rephrase the above in a more positive light ... what would the proponents of this ticket think about only adding this single line to remove, and leaving it at that (with documentation):

if (this.subviews) _.invoke(this.subviews, 'remove');
@wookiehangover

This comment has been minimized.

Copy link
Collaborator

wookiehangover commented May 17, 2013

@jashkenas +1 for adding this as a one-liner to remove

@philfreo

This comment has been minimized.

Copy link
Contributor

philfreo commented May 17, 2013

@jashkenas While that's exactly how I handle subviews myself, I think it'd be strange for Backbone to magically know about a view's subview property without providing any other way to add to or display those subviews.

Seems like if subview management is going to be left to custom implementations and plugins, we should leave those same plugins/custom code to simply patch View#remove to do this as necessary

@andreypopp

This comment has been minimized.

Copy link

andreypopp commented May 17, 2013

@jashkenas -1 for adding this

This seems like a partial implementation and it's not clear how saving 1 line from typing could help implementors of a fully featured subview management routines.

Personally, I'm against including any code into the core, but I'm totally for choosing an official plugin for nested heterogenous and/or homogenous views.

@wyuenho

This comment has been minimized.

Copy link
Contributor

wyuenho commented May 17, 2013

you don't need a hash or loop to (properly, including events) manage subviews, it's just the way you would implement it if you wanted to automate the process to reduce each subview from needing 3 lines of code (initialize, render, remove), to 1 line of code (addSubview)

No. That's not the point, the point is to provide a better structure. The current Backbone.View provides too little.

the fact that backbone.view has been so opened ended is precisely because it's NOT just for people who need to do simple stuff.

@philfreo How does not solving any problem solve any problems?

@jashkenas I'd like to see how you would document this subview issue. For now my preference would be no change to the code. It doesn't do anything substantial. All it does is outsourced a responsibility.

@tgriesser

This comment has been minimized.

Copy link
Collaborator Author

tgriesser commented May 17, 2013

Eh, at this point whatever everyone else thinks... I don't think the one liner isn't quite enough, feels a bit like the dispose implementation a little while back which was much better solved by listenTo.

I was really just trying to see if we could provide an optional "best practice" minimal implementation as part of the library to address a pain point I've heard mentioned a lot as people are learning how all of this stuff works, but it seems to have opened pandora's box on potential implementations... I'd be fine with docs.

@wyuenho

This comment has been minimized.

Copy link
Contributor

wyuenho commented May 17, 2013

it seems to have opened pandora's box

For very good reasons. Eek. Let's wait and see what the doc looks like.

@braddunbar

This comment has been minimized.

Copy link
Collaborator

braddunbar commented May 17, 2013

-1 for the one liner. I've tried this both ways (among others) and letting views be "GC'd as Eich and Nature intended" is far simpler and easier to maintain. I don't think we should discourage either approach, but we certainly shouldn't nudge users toward the more complex of the two.

@jashkenas

This comment has been minimized.

Copy link
Owner

jashkenas commented May 18, 2013

Awesome. That's reassuring to hear from you, Brad. Closing this ticket for a bit of quiet, but anyone who wants to send a PR for a nice docs section on the merits of no-reference vs. array-reference vs. named-reference will be cheered.

@jashkenas jashkenas closed this May 18, 2013

@tbranyen

This comment has been minimized.

Copy link
Collaborator

tbranyen commented May 18, 2013

Good call. This issue is all or nothing. Don't implement something that won't help, because others have already done it better. Maybe link to popular projects that newcomers can look to in the documentation. I'm amazed at how many tweets I see where people are complaining and never heard of Marionette, Chaplin, LayoutManager, etc.

@dgbeck

This comment has been minimized.

Copy link

dgbeck commented May 18, 2013

Totally hear your arguments @jashkenas.

Removing a whole swatch of views "nature's way", is pretty advanced stuff, and easy to get wrong. Seems like the cases where you would need to take this approach due to performance are pretty far between, and in those cases whoever is calling the shots can just call this.$el.remove() instead of this.remove for the root view. That being said maybe these cases are more common than I am supposing - not sure how expensive it really is to remove lots of views.

One other added benefit to integrating something like Tim's code, besides a little but important added structure, is that it would standardize a place to put subviews. Plugins and mixins could then leverage that standard location. Right now there is fragmentation between the popular frameworks that makes it messy to build cross-framework mixins related to subviews.

The one liner is pretty damn slick but I also agree it seems too incomplete.

I'm still curious to explore the array vs hash question, and to find a compelling case where an array is necessary. If there are no or very, very few such cases to be found then that would be something to work with. Otherwise we don't got much.

@ErichBSchulz

This comment has been minimized.

Copy link

ErichBSchulz commented May 19, 2013

as a noob - struggling with this right now... maybe a competition to see who can redo the "todo" app with some nested views... maybe a phone book with multiple email/telephone numbers per contact

@raine

This comment has been minimized.

Copy link

raine commented May 20, 2013

@ErichBSchulz or a todo list with sub-tasks.

@DjebbZ

This comment has been minimized.

Copy link
Contributor

DjebbZ commented May 21, 2013

@ErichBSchulz @raneksi Tastejs will hopefully provide the example you (and I) need about subviews management, plus many more topics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.