Don't trigger `relational:*` events when `options.silent` is set to `true`. #153

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants

When calling the save method with the option wait: true, backbone passes the option silent: true to the set methods so the events are not fired until the server confirms the save. This patches introduces the check for the options.silent before firing events throughout Backbone-relational.

Collaborator

philfreo commented Jul 20, 2012

👍 this fixes a bug for me (was getting change events firing when there were no changes)

Owner

PaulUithol commented Aug 2, 2012

Hmm.. the relational: events are used to maintain relations, and keep these consistent, even when using silent: true. Is this still the case with this patch?

There are too little (if any?) tests using silent: true, so tests still passing doesn't mean too much here I'm afraid.

Owner

PaulUithol commented Aug 2, 2012

Just checked, lots of tests do fail with this patch.. not such a good idea I think.

Owner

PaulUithol commented Aug 2, 2012

@philfreo: do you know if your patch also takes care of this issue (by a different route)?

Collaborator

philfreo commented Aug 2, 2012

I ended up un-applying this patch actually since unit tests were failing and the bug I thought this fixed was something else. My patch may help with some of this, not sure if it's exactly the same thing thiugh

Owner

PaulUithol commented Aug 3, 2012

Okay; closing this one then.

PaulUithol closed this Aug 3, 2012

I'm using this patch in my system. Without it, my system doesn't work. But perhaps, as @philfreo said, I'm solving the wrong problem. The observable problem is that add events are fired more than once on a collection for a single item. So instead my views listen to relational:add, which fixes the problem in most cases. The exception is that when saving a model with wait: true the relational: events are fired when the temp model is created in backbone even though backbone passes silent: true internally, and then again when the actual model is created. Again, this was messing my list views. What am I missing?

Collaborator

philfreo commented Aug 6, 2012

(Note: this confirmed Backbone.js bug documentcloud/backbone#1478 may be somewhat related)

Thanks for the reference, @philfreo - that may have been causing me problem as well.

Correct me if I'm wrong here. My limited understanding of how Relational works is that when relational: events are triggered, relationships are created between the temporary model and permanent models. As a result, any view listening on add events to dynamically update a list, will display the temporary item since it will be added to the collection of it's relation item. Needless to say, this is a undesired behaviour and this patch was my workaround. Is there a better way to handle this?

Collaborator

philfreo commented Aug 6, 2012

I haven't run into any problems with "temporary" models. In general the best way to move your specific problem forward would be to reduce it to the smallest possible QUnit test case that fails but shouldn't (and ideally a solution which fixes that test without breaking any others). Though it sounds like additional tests are needed to prove functionality around { silent: true }

Fair enough. I never used QUnit, and though I think this is something I should learn (and use?), it will be some time before I'll get to it. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment