dhruvaray / backbone-associations

Create object hierarchies with Backbone models; Respond to hierarchy changes using regular Backbone events.
http://dhruvaray.github.io/backbone-associations/
MIT License
492 stars 75 forks source link

Why must my child be an AssociatedModel in 1:1 #124

Closed fenduru closed 10 years ago

fenduru commented 10 years ago

When creating an AssociatedModel, specifying a Backbone.Model as the relatedModel gives an error, but its okay to specify a normal Backbone.Collection for 1:M. What's the reasoning here? It makes it hard for me to use Associations in an ecosystem where I don't have control of all of the models I'm using

dhruvaray commented 10 years ago

A collection by default is a container for many models. So there was no reason to specialize it. But AssociatedModel forms the basis for specifying the nature of relationships (1:1 and 1:M) with it's children (and their type too). Without that supplement, it would be difficult to infer types and the nature of the relationships. That being said, if you have any alternative ways, we could handle it, I would be glad to hear. (with a PR hopefully :))

fenduru commented 10 years ago

I understand the need for AssociatedModel in general, but what I am asking is why I can't have an AssociatedModel that has 1 Backbone.Model? If there is no technical reason for this I'll gladly submit a PR, but wanted to be sure first.

dhruvaray commented 10 years ago

I am not sure I follow you. Can you put out some pseudo code on what you are trying to achieve?

fenduru commented 10 years ago

http://jsfiddle.net/2F65W/

dhruvaray commented 10 years ago

Ok... But I suspect that this would be useful in only in a limited scenarios (leaf level of the graph only).A PR with a test case would be appreciated!

fenduru commented 10 years ago

Gladly. Only thing is I'm running into some odd behavior in the test suite.

ChildModel.prototype instanceof Backbone.AssociatedModel

is returning true in the test suite, but is false in reality, so I can't get my tests to fail.

dhruvaray commented 10 years ago

That may be because of environment.js... You may have to use Backbone.OriginalModel

jdkanani commented 10 years ago

@fenduru @dhruvaray It was supported before v0.5.2. I am not sure why we removed it. But of course adding it back would be great.

dhruvaray commented 10 years ago

@fenduru : I didn't hear back from you. So I made the changes. You should now be able to set the relatedModel to plain 'ol Backbone.Models as well. See the test case here and here.

fenduru commented 10 years ago

@dhruvaray had a busy week so didn't get a chance to look into this. Thank you so much for doing this though! Much appreciated.

Looked at the test case, and it seems a little bit weird to do

Backbone.AssociatedModel = Backbone.Model;
Backbone.Model = Backbone.OriginalModel;

Inside a test case. Would it make sense to isolate the environment.js test, and have the AssociatedModel tests use Backbone.AssociatedModel and Backbone.Model as they would be used in the wild?

dhruvaray commented 10 years ago

That's because in environment.js, we set Backbone.Model to AssociatedModel to run all Backbone library tests with AssociatedModel. This test case is an exception so that decision needs to be overridden.