.unset() is not working as expected

[mdamien]

Here's a test to reproduce it:

class Pony extends Model {
    relations() {
        return {
            favorite_food: Backbone.Model,
        };
    }
}

var me = new Pony();
var lasagna = new Backbone.Model({id: 42, name: 'Spinach Lasagna'});
me.set('favorite_food', lasagna);
me.unset('favorite_food');
console.log(me.get('favorite_food').toJSON()); // => {id: 42, name: 'Spinach Lasagna'} instead of undefined

Looks like the problem is here https://github.com/CodeYellowBV/backbone-relation/blob/master/index.js#L237. We set the id of the model to be undefined even if we do an unset.

[SpaceK33z]

What would you expect the output of console.log to be?

[mdamien]

I edited the content of the issue to make this clear, it should be undefined.

Here's a QUnit test for that, I'm taking a look at fixing it myself:

QUnit.test('doing an unset on a related model should remove it', 2, (assert) => {
    const mPost = new MPost();

    mPost.set('author', { id: 5, name: 'Burhan' });
    assert.equal(mPost.get('author').get('id'), 5);

    mPost.unset('author');
    assert.equal(mPost.get('author'), undefined, '.unset should make the model undefined');
});

[SpaceK33z]

Hm, I'm not sure that is how backbone-relation should work. One of backbone-relation's strengths is that it keeps the same model instance. As a result of this, listening in a view this.listenTo(this.model.get('user), 'change:id', this.blaat); will always work.

Make sure you talk to @abzainuddin about this before fixing it.

[SpaceK33z]

Oh, actually the output is incorrect, after an unset, the model should have the default values again.

[abzainuddin]

The manual gives:

This implies that once a relation is set, it cannot be overridden with another value. If you do really want to set another instance for a relation, then first use unset:

mAuthor.unset('user');
mAuthor.set('user', mUser);

The original idea was that unset is the escape where you can really unset a relation, so:

mPost.unset('author');

should remove the author.

[SpaceK33z]

Ah whoops, remembered it wrong.
@mdamien, even if you don't manage to fix it yourself, could you add the failing test?

[mdamien]

Hm, I'm not sure that is how backbone-relation should work. One of backbone-relation's strengths is that it keeps the same model instance. As a result of this, listening in a view this.listenTo(this.model.get('user), 'change:id', this.blaat); will always work.

@SpaceK33z I think the problem you're describing is better solved by the ability to do "deep listening". Like:

this.listenTo(this.model, 'change:user.id', this.blaat);

[SpaceK33z]

The problem I'm describing is already solved with backbone-relation. Using unset is still something you shouldn't do most of the times; instead better clear the model.