Git Product home page Git Product logo

Comments (33)

PaulUithol avatar PaulUithol commented on June 30, 2024

Good question, don't know exactly what's going here. Setting up a new relation is a bit more complicated than just doing new Backbone.Collection( [ models ] ), so it won't use Backbone's default approach of silencing add when creating a new collection. Added to my backlog ;)

from backbone-relational.

 avatar commented on June 30, 2024

@cjroebuck

I had the exact same problem on a former project and it was a real show stopper because it messed up the rendering. Ended up having to remove backbone-relational altogether :-( ... Have you found a workaround that you could share or are you still stuck with this issue ?

from backbone-relational.

wiemann avatar wiemann commented on June 30, 2024

Same problem here. How can this behavior be prevented?

from backbone-relational.

PaulUithol avatar PaulUithol commented on June 30, 2024

Hmm, trying to replicate this, but I'm actually having the reverse problem in a slightly different scenario: I don't get any add events at all when using fetch (while the remove events did fire), because of the following line in Backbone.reset:

this.add(models, {silent: true, parse: options.parse});

Do any of you guys have a test case showcasing the behavior you get?

from backbone-relational.

cjroebuck avatar cjroebuck commented on June 30, 2024

Are you using backbone 0.5.3 as the same line in my version of Backbone's reset is just:

this.add(models, {silent: true});

from backbone-relational.

PaulUithol avatar PaulUithol commented on June 30, 2024

No, I'm probably on HEAD. However, the silent: true is what (should) prevent the add events from firing when calling reset; so I don't get why they would fire in your case?

from backbone-relational.

cjroebuck avatar cjroebuck commented on June 30, 2024

OK I ran into this issue again on my project and really want to resolve it once and for all.. (using latest versions of bb and bb-rel)

I'm polling a model which has a HasMany relation to a collection. When I repeatedly call fetch on the model, I get a ton of add callbacks when the collection attribute is set in the success callback of the fetch.

This line in backbone's model.fetch success callback:

if (!model.set(model.parse(resp, xhr), options)) return false;

causes Backbone-Relationals set method to kick in, which unblocks the global event queue:

// Try to run the global queue holding external events
Backbone.Relational.eventQueue.unblock();`

Once the queue becomes unblocked, my stack trace ends up in Backbone.Collection.trigger of backbone-relational:

     Backbone.Relational.eventQueue.add( function() {
      _trigger.apply( dit, args );
    });

which is then what triggers the 'add' event to fire.

from backbone-relational.

cjroebuck avatar cjroebuck commented on June 30, 2024

PS a workaround which I had been using up till now was to use a 'naked' $.ajax call (instead of model.fetch) and in the success callback just call set manually on the model in question with the returned JSON from the ajax response. This would trigger the right amount of add callbacks for me.

from backbone-relational.

PaulUithol avatar PaulUithol commented on June 30, 2024

Ah, right, now we're getting somewhere. Was looking at collections since you mentioned reset in the initial post. This makes it sound like an issue more related to #38?

But what happens if you just use model.fetch( { silent: true } )? Normally, I think I would want these add events to fire, since there's no other event that you can subscribe to to get this information (as opposed to collection's `reset', which does fire an event).

from backbone-relational.

mateusmaso avatar mateusmaso commented on June 30, 2024

facing the same problem here too, is there any fix going on soon?
cjroebuck workaround saved me for a while, I really enjoyed the framework but this bug is driving me crazy..

from backbone-relational.

PaulUithol avatar PaulUithol commented on June 30, 2024

Well, I still don't know how/why/when this problem occurs exactly. If you (or someone else) could supply a reduced testcase or jsFiddle showing the problem, that would help a lot in getting this fixed.

from backbone-relational.

aroman avatar aroman commented on June 30, 2024

I'm pretty sure I'm affected by a variant of this issue. When calling save() on RelationalModel, even if nothing changed, add:related will fire for the each of the related models to the RelationalModel.

I have a reasonable hack to work around this for my use case -- my related models each have a GUID, so I just stick that as a data-id attribute in a hidden div in the view of the model. On the add:related callback in the RelationalModel view, I check if a div with the incoming GUID already exists, and if it does, I simply return.

I'll try my hand at a test case for my particular issue and report back when I've got a jsFiddle working.

from backbone-relational.

tabacco avatar tabacco commented on June 30, 2024

This is a big blocker for a project of mine as well. Let's say I have a model called Foo with a relation to a collection called Bars, with key 'bars'. Now if I do this on page load:

var OneFoo = new Foo({id: 1, bars: [{id: 1}, {id: 2}]})

If I'm bound to 'add' on Bars, I'll see two events fire here, or as many events as there are elements in the collection. What really murders performance, especially in IE8, is if the collection that 'bars' represents has a comparator method, causing it to fire off a sort() every time an element is added.

from backbone-relational.

tabacco avatar tabacco commented on June 30, 2024

To clarify: I'm talking about the original issue as-written, not any of the other comments.

from backbone-relational.

DouweM avatar DouweM commented on June 30, 2024

@PaulUithol Here's a jsFiddle showing the problem: http://jsfiddle.net/SXn2B/2/

from backbone-relational.

DouweM avatar DouweM commented on June 30, 2024

It appears that the add event is
triggered by this.related.add(), which is
called from the relation's #tryAddRelated, which is
queued to be called from #_relatedModelAdded, which is
called in response to the relational:add event on Backbone.Relational.store, which is
triggered after the new model is added to the store's collection, which is
from Backbone.Relational.store's #register, which is
called from the new model's #set, which is
called from its constructor.
This constructor is
called from the relation's #createModel, which is
called from #findRelated, which is
called from #onChange, which is
called in response to the relational:change event, which is
triggered by #updateRelations, which is
called from the owner's #set, which is
called from the jsFiddle.

The reset event is
triggered by collection.reset(), which is
called from the relation's #prepareCollection, which is
called from #onChange, which is
called in response to the relational:change event, which is
triggered by #updateRelations, which is
called from the owner's #set, which is
called from the jsFiddle.

As you can see, the paths to the events are the same up to and including #onChange, where #prepareCollection causes the reset event, and #findRelated causes the add event(s).

It took me quite some time to figure all this out, so I hope it will be of use to somebody.

from backbone-relational.

PaulUithol avatar PaulUithol commented on June 30, 2024

Alright, after some debugging sessions, I do see some issues here that could use some fixing; I'm not sure these are the same ones that are bothering you guys though.. In any case, I think the desired behavior regarding the occurrence of the add, remove and reset events is, at best, hazy, since Backbone was simply not designed with nested relations in mind.

For example, in the fiddle supplied by @DouweM I'd say the reset event is erroneous, and the add events correct; the reset is merely fired as a side-effect of attempting to re-use an existing collection. This should be easy to fix; I don't really know how the add events could be suppressed though (since this is very context dependant.. you usually do want them, I suppose).

Passing silent: true here does seem to be broken though (try person.set( 'pets', [ 1, 2 ], { silent:true } )); fixing this should help as well. I still find that whole concept a bit funny, since events can sometimes just 'disappear' and stop propagating in unexpected places (no biggy in 'flat' applications, but it's a pain when you rely on events to propagate down in nested structures, like inside of relational itself). I've tried to counter that in relational by transforming the options object to silentChange: true, and checking for that value later on before firing events. However, it should probably be tranformed back to silent: true before calling the original Backbone methods from the overrides for add, remove and reset.

from backbone-relational.

PaulUithol avatar PaulUithol commented on June 30, 2024

Okay, this should help a bit; at least, passing silent: true should suppress the add events now, and the extra reset event introduced a while back should be gone.

from backbone-relational.

DouweM avatar DouweM commented on June 30, 2024

Why do you think the reset event is erroneous in my example, and not the add events?

Per BackboneJS.org:

reset collection.reset(models, [options])

Use reset to replace a collection with a new list of models.

add collection.add(models, [options])

Add a model (or an array of models) to the collection.

What we're doing by re-assigning pets is replace the old value of pets by the newly passed data. This is exactly what the reset method and events mean. These two calls should have the same effect:

pet_attributes = [
  {
    id: 1,
    name: "Spot"
  },
  {
    id: 2,
    name: "Whiskers"
  }
]

person.set("pets", pet_attributes)
person.get("pets").reset(pet_attributes)

from backbone-relational.

DouweM avatar DouweM commented on June 30, 2024

Why this reset/add thing is a problem for me, is because my view observes both events, and renders subviews in my view depending on the nature of the event.

  • For reset, I call #render, which clears the view's element, loops over the models in the collection, and inserts a subview for each of them. This means that the view will now show subviews for exactly those models that the collection was reset to.
  • For add, I call #insertItem(model), which appends a new subview to the end of the existing views.

When reassigning the relation's key's value, the collection's data is replaced by the newly built models, and reset is fired. This causes the view to be re-rendered, adding subviews for each of the collection's models. When after this an add event is fired for each of the collection's models, new subviews are appended to my view for each of the models. Because of this, my view will now contain two subviews for each of the views in the collection.

My setup works perfectly with regular Backbone.Collections, as when I reset those, only the reset event will fire. With the current Backbone-relational implementation, it doesn't work as it should, as I described in the previous paragraph. With your latest changes, which cause reset to no longer be fired when reassigning the relation, my setup would no longer show duplicate subviews, but it's still far from perfect: to have my view do the right thing, it needs to remove a DOM element in response to the remove event for each of the collection's old models, and it needs to add a DOM element in response to the add event for each of the collection's new models. This is a lot less efficient than re-rendering the entire DOM tree in one go in response to a single reset event.

from backbone-relational.

PaulUithol avatar PaulUithol commented on June 30, 2024

Have you considered binding to the update event, e.g. person.on( 'update:pets' )? Shorter syntax, more reliable (no Backbone interference) and you don't have to rebind when a different collection is set on the relation.

As to why I think the reset shouldn't be triggered when initializing a relation:

  • Backbone doesn't trigger reset either when initializing a collection (on the other hand it does not trigger add either..).
  • I don't really like how reset behaves, and is used in Backbone; the event is also (ab)used for other purposes, like being triggered on sort.

So.. maybe it's better to ditch both the reset and add events in HasMany.onChange, and rely on update:<key> instead for these cases?

from backbone-relational.

tabacco avatar tabacco commented on June 30, 2024

update is great except that backbone itself has things tied to add, like sort().

from backbone-relational.

PaulUithol avatar PaulUithol commented on June 30, 2024

@tabacco: Backbone only calls sort once for each cycle of additions. In your example, it wouldn't matter at all if you'd use relational or not. Except that you'd have to initialize the collection yourself a bit earlier, which would still calls add, and thus sort. If sorting is killing performance in your app, I'd profile the sorting first, and perhaps not specify a comparator and only sort the models manually when you really need to.

from backbone-relational.

tabacco avatar tabacco commented on June 30, 2024

That's not the behavior we were seeing. We were basically seeing n! sort operations when instanciating a HasMany-related collection within a model. In other words, every time an add event was fired, it was initiating a sort(). We've worked around it by manually sorting, but there's no way that should be expected behavior. I'd expect instantiating a collection to fire a reset event, which would trigger a single sort().

from backbone-relational.

PaulUithol avatar PaulUithol commented on June 30, 2024

Hmm.. yeah, you're correct. Backbone only calls sort once per add; however, relational adds them one at a time, so there we go. The call to sort is simply part of Backbone.Collection.add though; reset has nothing to do with it. This is a separate issue though, so please file a new issue for it.

from backbone-relational.

tabacco avatar tabacco commented on June 30, 2024

Isn't the subject of this bug that relational adds one at a time and fires one add event per item, though? That's really the root of the issue for me. It seems like on initializing a new related collection it should suppress add and fire a reset event instead.

from backbone-relational.

DouweM avatar DouweM commented on June 30, 2024

@PaulUithol How do you propose I use update:pets in the setup I described?

from backbone-relational.

PaulUithol avatar PaulUithol commented on June 30, 2024

Well, no. This is a separate issue, that I can fix independently. See 98e7cad ;)

from backbone-relational.

DouweM avatar DouweM commented on June 30, 2024

@PaulUithol And obviously Backbone doesn't trigger reset when initializing a collection either, because at that point no-one can possibly be observing the collection yet ;-)

from backbone-relational.

DouweM avatar DouweM commented on June 30, 2024

@PaulUithol I'll ask you again, because I'm still not sure how this will work. I want one view to handle one collection, without any knowledge of what model and relation it belongs to. I just pass the collection to the view, and the view should be able to do its thing.

The reset, add and remove events are perfect for a setup like this in combination with a Backbone.Collection that's not managed by Backbone-relational: render upon initialize, re-render upon reset, add a subview upon add, remove a subview upon add. But with one that is, it doesn't work properly: the events that are triggered aren't the same even though the net changes to the collection's models are.

See this jsFiddle for an example. The result:

For a relation, using `model.set('related', models)`
Set the Person's pets attribute to [spot, whiskers]
add: Spot
add: Whiskers
Set the Person's pets attribute to [whiskers, moo]
remove: Spot
remove: Whiskers
add: Whiskers
add: Moo

For a relation's collection, using `model.get('related').reset(models)`
Reset the Person's pets collection to [spot, whiskers]
remove: Whiskers
reset: Spot,Whiskers
Reset the Person's pets collection to [whiskers, moo]
remove: Moo
reset: Whiskers,Moo

For a regular collection, using `collection.reset(models)`
Reset the collection to [spot, whiskers]
reset: Spot,Whiskers
Reset the collection to [whiskers, moo]
reset: Whiskers,Moo

from backbone-relational.

PaulUithol avatar PaulUithol commented on June 30, 2024

Yes, a collection that's involved in a relation will have different events being fired on it; it will also respond to stuff happening on the other end of the relation. But I don't really see what's bothering you here?

The add/remove sequence at the beginning is correct and should result in the right subviews; the extra remove happens when the other end of the relation notices the relation has been empties for the reset; then the reset event is fired and should cause a re-rendering of the total view.

Why wouldn't you arrive at the same final situation in your example?

from backbone-relational.

DouweM avatar DouweM commented on June 30, 2024

With the reset event gone when using model.set('relation', models), I do arrive at the correct final destination, so you're right: that's not really a problem anymore.

What's bothering me at the moment is that the events don't accurately represent what's happening. In all 3 scenarios described in my previous post the exact same thing is done to the collection, but the events that are triggered vary greatly. The problem is that my view or anything else observing a collection can't depend on the collection to trigger the same events for the same manipulations all of the time. We can't mess with Backbone's own reset method (which I'm not a fan of, for what it's worth: descriptive add and remove events are much better), but I think the Backbone-relational-handled model.set('related', models) and model.get('related').reset(models) should at least result in the same events being triggered. In both of those cases the reset event could be replaced by multiple add and remove events, describing exactly the results of the set() or get().reset() call. This would mean keeping track of what models were already in the collection before it was reset, but that shouldn't be too hard by checking a model's cid or id, I reckon.

I realize this isn't related to this issue anymore, and that my previous comment could have been a lot clearer on what I was complaining about.

from backbone-relational.

PaulUithol avatar PaulUithol commented on June 30, 2024

I think this is pretty well under control (at least the issue as it was originally raised, along with some fixes for related bugs). Closing this bug because it doesn't really have a defined purpose anymore.

from backbone-relational.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.