Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Make remove() auto-cleanup more events? #2045

Closed
philfreo opened this Issue · 9 comments

8 participants

@philfreo

My understanding is that events aren't auto cleaned up in View#remove if they were created like:

  • this.model.once('change', this.render, this) if the change event is never fired
  • this.model.on('change', this.render, this)

The second scenario is fixed when people use this.listenTo(model, 'change', this.render) instead, but seems like there's no easy way to have once events currently cleaned up.

Possible proposals:

  • add a listenToOnce method
  • add this.model.off(null, null, this); this.collection.off(null, null, this); into View#remove? (like the original dispose implementation of #1461), to take care of cases like this automatically? It would auto-cleanup both for usages of once and just other places where people use on instead of listenTo.
  • make this.model.on(..., ..., view) automatically get added to the events that get removed when view.stopListening is called?
@caseywebdev
Collaborator

I think a listenToOnce would be more fool-proof than relying on this.model or this.collection in a view.

@tgriesser
Collaborator

I'd vote listenToOnce so it can be used outside of the view.

@philfreo

One more question I wanted to check

view1.listenTo(view1, 'foo', view1.foo);
view2.listenTo(view1, 'foo', view2.bar);
view1.remove();

Are all the events properly cleaned up? (A view listening to itself)

@gsamokovarov

Seems so, see #2050.

@eastridge

listenTo fills this gap, and I think further enhancements including model and collection unbindings should be left to implementors if they choose to not use listenTo.

@ewang

What about adding some kind of onRemove() method to views that by default is a no-op. onRemove() will be called automatically by remove() on-top of all the existing functionalities provided by remove(). Alternatively, remove() could also emit an event that can be listened to optionally...

This would allow for implementors to tack on additional cleanup routine that gets called automatically.

A use-case I have for this would be to simplify the cleanup of nested views, without overriding the default remove() method.

@philfreo

What about adding some kind of onRemove() method to views that by default is a no-op. onRemove() will be called automatically by remove() on-top of all the existing functionalities provided by remove(). Alternatively, remove() could also emit an event that can be listened to optionally...

Doesn't seem like much of a benefit over just overriding remove

@tbranyen
Collaborator

This is a really good point that I did not consider. People upgrading LayoutManager may still have this.on... syntax instead of listenTo. I'll continue to clean those up.

@jashkenas
Owner

There's now a listenToOnce, so this should be sorted.

@jashkenas jashkenas closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.