Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Updating to 0.6.2: _proxyCallback issues #145

Closed
chiplay opened this Issue · 5 comments

2 participants

@chiplay

I'm trying to get our app up to date with the latest changes to 0.6.2 but I'm running into one issue. The if(!relationValue._proxyCallback) { check that happens on line 355 is preventing some of my nested models from bubbling correctly to their parents. The issue arrises when there is already a relationship established with one parent, then a second parent also wishes to relate to to the same model - and relationValue._proxyCallback already exist. In my use case the two parent models have different relationKeys - so the _proxyCallback function only bubbles correctly for the first callback that was registered.

Any thoughts on how to allow a child model to bubble events to two different parents, with two different relationKeys? Thanks!

@chiplay

Here's an attempt at some sudo code for the issue:

var Foo = Backbone.AssociatedModel.extend({});

var Bar = Backbone.AssociatedModel.extend({
       relations: [{
       type: Backbone.One,
            key: 'foo_a',
            relatedModel: Foo
       }]
});

var Baz = Backbone.AssociatedModel.extend({
       relations: [{
       type: Backbone.One,
            key: 'foo_b',
            relatedModel: Foo
       }]
});

Create some instances:

var foo = new Foo();

var bar = new Bar({ foo_a: foo});
var baz = new Baz({ foo_b: foo});

And some listeners:

bar.on('change:foo_a.name', someMethod); => fires correctly
baz.on('change:foo_b.name', someMethod); => does not fire

Now when we have an event - foo.set({ name: 'John' }); - only bar bubbles the event from foo because the _proxyCallback method on foo has the relationKey foo_a in its closure.

All that said - I'm not sure I have a solution for registering mulitple _proxyCallbacks for each model, perhaps its an array or object with the relationKey as the unique identifier?

@chiplay

Hey @dhruvaray - I think I've got a working solution - I'm a bit crunched for time this week, but let me know if you need a proper PR for this:

Line 351:

relationValue._proxyCallbacks = relationValue._proxyCallbacks || {};

// Add proxy events to respective parents.
// Only add callback if not defined or new Ctx has been identified.
if (newCtx || (relationValue && !relationValue._proxyCallbacks[relationKey])) {
    // https://github.com/dhruvaray/backbone-associations/issues/145
    if(!relationValue._proxyCallbacks[relationKey]) {
        relationValue._proxyCallbacks[relationKey] = function () {
            return Backbone.Associations.EVENTS_BUBBLE &&
                this._bubbleEvent.call(this, relationKey, relationValue, arguments);
        };
    }
    relationValue.on("all", relationValue._proxyCallbacks[relationKey], this);
}

Line 367:

//Distinguish between the value of undefined versus a set no-op
if (attributes.hasOwnProperty(relationKey))
    this._setupParents(attributes[relationKey], this.attributes[relationKey], relationKey);

Line 479:

//Maintain reverse pointers - a.k.a parents
_setupParents: function (updated, original, relationKey) {
    // Set new parent for updated
    if (updated) {
        updated.parents = updated.parents || [];
        (_.indexOf(updated.parents, this) == -1) && updated.parents.push(this);
    }
    // Remove `this` from the earlier set value's parents (if the new value is different).
    if (original && original.parents.length > 0 && original != updated) {
        original.parents = _.difference(original.parents, [this]);
        // Don't bubble to this parent anymore
        original._proxyCallbacks[relationKey] && original.off("all", original._proxyCallbacks[relationKey], this);
    }
},

Line 661:

// Call this if you want to set an `AssociatedModel` to a falsy value like undefined/null directly.
// Not calling this will leak memory and have wrong parents.
// See test case "parent relations"
cleanup:function (options) {
    options = options || {};

    _.each(this.relations, function (relation) {
        var val = this.attributes[relation.key];
        if(val) {
            val._proxyCallbacks[relation.key] && val.off("all", val._proxyCallbacks[relation.key], this);
            val.parents = _.difference(val.parents, [this]);
        }
    }, this);

    (!options.listen) && this.off();
},
@chiplay

@jdkanani any thoughts on this problem / solution?

@jdkanani
Collaborator

Hey @chiplay, solution looks promising. I think you should go ahead with primary pull request. So we can try it out at our end.

@dhruvaray what do you think?

@chiplay

@jdkanani @dhruvaray PR is up: #146 - closing this ticket and we can continue discuss there.

@chiplay chiplay 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.