Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

CompositeView attempts to appendHtml of itemViews before itemViewContainer exists (before template is rendered) #533

Closed
wants to merge 1 commit into from

8 participants

@OliverJAsh

I have a CompositeView which takes a model (already downloaded from server) and a collection which is then fetched. As soon as the sync is finished, I want to render my CompositeView. However, the CompositeView appears to try appending the itemViews for the collection before it renders its own template, and so it can't find its own itemViewContainer, generating error Uncaught ItemViewContainerMissingError: The specifieditemViewContainerwas not found: .modal__body.

Here is a gist.

I should note that I have exactly the same setup elsewhere and it works with no problems, so it could well be something on my end, but I've spent long enough debugging this one and I can't seem to find the problem!

@OliverJAsh

Here is a screenshot of my call stack which might point you to something.

@OliverJAsh

Aha, the reason it was working in another part of my application is because I had a custom appendHtml. If I remove that, I have the same problem there too…

@OliverJAsh

This commit seems to attempt to fix the problem, but I had to go a little bit further and check that the CollectionView/CompositeView was rendered before executing appendHtml for the ItemViews. I wonder if we can prevent renderItemView from executing at all when a model is added to the collection, as currently this is what it does, even when the CollectionView/CompositeView is not rendered.

Glad I'm finally getting to the bottom of this. One thing is for sure, it has improved my JS debugging skills! Sorry I couldn't submit a spec for this.

@derickbailey

AHA! good catch, and totally makes sense now that you point it out. Will get this pulled in w/ a spec, along with some other bug fixes, ASAP.

@OliverJAsh

It does cause quite a few specs to fail, hope you can work around that!

@atorian

Hi everyone.
I have same problem, and some thoughts.

Before you apply this fix:
Does attribute isRendered used in CollectionView ?
This fix causes another problem. Collection views won't work properly.

So i added renderItemView method with isRendered condition to CompositeView class. And now it works fine.

I hope it can help you.

@OliverJAsh

@atorian Could you make a PR, and reference it here? I would be grateful to see your solution and it makes it easier for @derickbailey to just merge.

@lukaszfiszer

I think a much cleaner solution is the one proposed by @b4cedev in #377 - binding to collection events only after the CompositeView has been rendered:

_initialEvents: function(){
    this.once('render', function () {
      if (this.collection){
        this.listenTo(this.collection, "add", this.addChildView, this);
        this.listenTo(this.collection, "remove", this.removeItemView, this);
        this.listenTo(this.collection, "reset", this._renderChildren, this);
      }
    }, this);
  },

This way we avoid unnecessary calculations in all methods starting from addChildView, not just in appendHtml.

@wilson29thid

Thanks @lukaszfiszer and @b4cedev ! That's exactly what I needed.

@samccone
Owner

closing

@samccone samccone closed this
@mahnunchik

When this fix will be in the upstream?

@zazabe

:+1: Need this one too, it should be re opened, no ?

@samccone samccone reopened this
@samccone
Owner

I like @lukaszfiszer solution, anyone want to take a stab at opening a PR with this fix?

@lukaszfiszer

I can submit a PR during this weekend.

@lukaszfiszer lukaszfiszer referenced this pull request from a commit in lukaszfiszer/backbone.marionette
@lukaszfiszer lukaszfiszer Fix CompositeView adding child views before render
* in CompositeView._initialEvents wait for 'render' event before binding
to collection events.
* added specs

Fixes #533.
a38e969
@samccone samccone closed this in #763
@naegelyd naegelyd referenced this pull request in chop-dbhi/cilantro
Closed

Cannot save query #377

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 3 additions and 1 deletion.
  1. +3 −1 src/marionette.collectionview.js
View
4 src/marionette.collectionview.js
@@ -183,7 +183,9 @@ Marionette.CollectionView = Marionette.View.extend({
// render the item view
renderItemView: function(view, index) {
view.render();
- this.appendHtml(this, view, index);
+ if (this.isRendered) {
+ this.appendHtml(this, view, index);
+ }
},
// Build an `itemView` for every model in the collection.
Something went wrong with that request. Please try again.