Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error: Cannot instantiate more than one Backbone.RelationalModel with the same id per type! #186

Closed
utiq opened this issue Aug 24, 2012 · 63 comments

Comments

@utiq
Copy link

utiq commented Aug 24, 2012

Has anyone have this error? Error: Cannot instantiate more than one Backbone.RelationalModel with the same id per type!

@PaulUithol
Copy link
Owner

See #180. Is this happening to you on fetchRelated?

@gfarrell
Copy link

I get this problem as well. I am trying to call Collection.reset(json_response, {parse:true}) and am getting the following error + stack trace:

child() backbone.js (line 1388)
child() backbone.js (line 1388)
build(...) relational.js (line 1458)
findOrCreate(...) relational.js (line 1517)
_prepareModel(...) relational.js (line 1538)
(?)(...) relational.js (line 1573)
forEach(...) underscore.js (line 77)
add(...) relational.js (line 1579)
reset(...) backbone.js (line 738)
reset(...) relational.js (line 1625)
__parse(...) Index.js (line 113)
fire(...) jquery-1.8.0.js (line 973)
fireWith() jquery-1.8.0.js (line 1080)
done() jquery-1.8.0.js (line 7583)
callback() jquery-1.8.0.js (line 8298)

child = function(){ parent.apply(this, arguments); };  backbone.js (line 1388)

I'm not sure why this is happening...

@philfreo
Copy link
Collaborator

Well it's happening because you're trying to instantiate more than one version of the same model, which is no longer supported (it never really was supported, but now it's explicit). The idea is that backbone's relational store can only keep track of one version of each object.

@gfarrell
Copy link

But if I am resetting the collection with new data, there is always a chance that some of that data will be the same as before (for example, I have a series of filters on my data, which, when changed, trigger a Controller.fetch()). Surely reset() should then destroy all the models and make new ones, which should not be a problem

@gfarrell
Copy link

And everything works fine if I remove RelationalModel from the inheritance chain and just use Backbone.Model.

@rgarcia
Copy link

rgarcia commented Aug 31, 2012

Running into this problem as well. What's the proper way to re-fetch the latest version of a collection/models if you're using backbone.relational?

@stpn
Copy link

stpn commented Sep 1, 2012

Same here, no error if I just do Backbone.Model (as in gfarrel's comment). Even asked a SO question (http://bit.ly/S6SlyI) but then saw this issue..

@gfarrell
Copy link

gfarrell commented Sep 1, 2012

@stpn I think your problem is slightly different from (at least) @rgarcia's and mine. Basically we are having a problem with Collection.fetch() (which is the normal way to get new data, of course) whereas you have a problem with the model where you can just call findOrCreate().

I think the issue, @philfreo and @PaulUithol, is that the Collection needs perhaps to use findOrCreate not just a generic Backbone.Model fetch.

@rgarcia
Copy link

rgarcia commented Sep 5, 2012

It looks like this error condition was added in the latest commit to master. I was able to get my code to work by pulling down the code as it was prior to this commit: https://github.com/PaulUithol/Backbone-relational/blob/4e6da7551522c87af0f466e8995d104deaa2f768/backbone-relational.js

Hope that helps other people having this issue!

@gfarrell
Copy link

gfarrell commented Sep 6, 2012

But that doesn't solve the underlying problem. We shouldn't be instantiating the same model twice and so there must be a way around it. Removing the error condition just hides the problem, it doesn't solve it.

On Thursday, 6 September 2012 at 07:29, Rafael wrote:

It looks like this error condition was added in the latest commit to master. I was able to get my code to work by pulling down the code as it was prior to this commit: https://github.com/PaulUithol/Backbone-relational/blob/4e6da7551522c87af0f466e8995d104deaa2f768/backbone-relational.js
Hope that helps other people having this issue!


Reply to this email directly or view it on GitHub (#186 (comment)).

@gmussi
Copy link

gmussi commented Sep 6, 2012

Running into the same issue.
In my case, I load an instance of a object when the system routers to a specific URL. The second time I am routed to that URL, I get this problem.

@gfarrell
Copy link

@gmussi I have the same symptoms. It's because for some reason Collection.reset doesn't reset Backbone.RelationalModel's "cache" of Models. I wonder if @PaulUithol has a solution?

@gmussi
Copy link

gmussi commented Sep 11, 2012

@gfarrell I fixed my problem by calling findOrCreate first in an auxiliary method:

var entity = MyEntity.findOrCreate(id);
if (!entity) {
   entity = new MyEntity({
      _id : id
   });
}
entity.fetch();

This fixed the problem everywhere for me.

@gfarrell
Copy link

@gmussi The problem with this is that a lot of my data is fetched by the collection (which is reset). Logically the 'reset' should clear the model cache. Having to use this workaround would be a bit of a pain although I suppose I could build it into Collection/Model .parse().

On Tuesday, 11 September 2012 at 16:54, gmussi wrote:

@gfarrell (https://github.com/gfarrell) I fixed my problem by calling findOrCreate first in an auxiliary method:
var entity = MyEntity.findOrCreate(id); if (!entity) { config = new MyEntity({ _id : id }); } entity.fetch();
This fixed the problem everywhere for me.


Reply to this email directly or view it on GitHub (#186 (comment)).

@davedx
Copy link

davedx commented Sep 13, 2012

I'm also having this issue, basically by following the backbone-relational tutorial: I have a collection of Users, then the router is trying to route me to a single user, so as in the tutorial code I'm doing a new with the User model and then getting this exception.

As I'm still trying to learn backbone/backbone-relational I'm not sure how I should fix it. Should/can I hard reset backbone's data store, or should I use gmussi's solution of using findOrCreate?

@gmussi
Copy link

gmussi commented Sep 13, 2012

An update to my previous post, there is a shortcut:
var entity = MyEntity.findOrCreate({_id : id});
more about it: http://whaticode.com/2012/09/13/backbone-relational-and-duplicate-objects/

@davedx
Copy link

davedx commented Sep 13, 2012

@gmussi - thanks a lot, I'll give that a try this evening.

@al
Copy link

al commented Oct 16, 2012

@PaulUithol I encounter this when I pass the update option to fetchRelated. My understanding of it is that the code added by @DouweM (3e69fad) later updated by you (1648136) is ignoring the store and always building a new model. If the model has been seen before you get this error.

I cobbled together a solution 787d711 (no tests) which just looks in the store first and only attempts to build if necessary. Just wondering what you guys think? Is it on the right track?

@gfarrell
Copy link

@al These were my findings as well. From testing my setup (resetting the Controller with new data) and looking at the stack trace when the error was thrown, I could see that while findOrCreate was being called, the error was still being thrown so it seemed to ignore the store and just try and build a new model, as you say.

@al
Copy link

al commented Oct 16, 2012

@gfarrell If you get a chance try using my branch and see if it works/you have any feedback.

@gfarrell
Copy link

@al I've applied the path, will test later.

On Tuesday, 16 October 2012 at 12:47, Alan Larkin wrote:

@gfarrell (https://github.com/gfarrell) If you get a chance try using my branch and see if it works/you have any feedback.


Reply to this email directly or view it on GitHub (#186 (comment)).

@gfarrell
Copy link

Hi @al, could you post the whole of your backbone-relational.js file? I tried just now and I am still getting the "cannot instantiate more than one..." error whenever I go back/reload data.

@al
Copy link

al commented Oct 28, 2012

@gfarrell You can find it in my fork here: https://github.com/al/Backbone-relational/blob/bugfix-fetchRelated-with-update/backbone-relational.js. That's the full extent of my changes.

@gfarrell
Copy link

@al Still getting errors when I load data into the controller, some of which may have been seen before...

@al
Copy link

al commented Oct 28, 2012

@gfarrell I've just put up fiddle that illustrates the difference when you use my fork.

http://jsfiddle.net/UhYzK/2/ uses the official Backbone-relational. You should be able to page forward, but when you try to go back you should get the error because the Issue it is trying to fetch have been cached in the store.

http://jsfiddle.net/ah5wM/2/ is an identical fork except that it uses my modified copy of Backbone-relational. You should be able to page forwards and backwards because my change recognises the update option and first checks the store to see if the Issue being sought has already been cached, in which case it uses the cached copy rather than trying to instantiate a new one.

It's a very scrappy Backbone app, but perhaps it illustrates how my change is working for me.

@gfarrell
Copy link

@al, thanks for the example. I can see why it works for you, because you have a parent model which is basically acting like a collection so you can use fetchRelated.

It doesn't work for me, so far, because I use

this.Rooms.fetch({
    url:     this.__buildUrl(),
    add:     append,
    success: append ? this.collectionHasAdded.bind(this) : null
});

Where Rooms is a collection, not a model. The stupid thing about this for me is that in the stack trace of the error I can clearly see Backbone.RelationalModel.Backbone.Model.extend.findOrCreate, yet Backbone.RelationalModel.Backbone.Model.extend.build is still being called. I've spent far too long staring at it and I don't understand why on fetch(), if some of the models being fetched have already been seen, I get this error. It should just be pulling them up again from the Store.

@al
Copy link

al commented Oct 28, 2012

@gfarrell I have to admit I don't really understand what you're doing. Perhaps a fiddle of your own demonstrating the issue would help?

@gfarrell
Copy link

@al I created a very basic fiddle, but couldn't reproduce my problem so am going to search back through my source code to try and find out what the problem is. I have a feeling it could be to do with a duplicated controller fetching data.

@jayd3e
Copy link
Contributor

jayd3e commented Nov 26, 2012

I think the changes made in @al's fork should be added to master(or the latest dev version). It seems reasonable to me that if a model is fetched from the server that has the same id as one that is already in Store, then backbone-relational should simply use the cached one in it's place, and continue along with execution.

This is why this fix is important for me. So I have a backend which returns a nested arrangement of models up to a certain depth that I can specify. Here is an example returned from the server: https://gist.github.com/4151234. Notice how at line 24, I am introducing the same model as the root model. This is because somewhere down the line of relationships, we end up with a reference to the element that was being requested in the first place. This is something that I think backbone-relational should be able to handle, as the fetch() method should be idempotent.

The two clear solutions are to do what @al did and grab that model from the store if it already exists, or we could update the model in the store with the one that has been most recently fetched from the server. I say go with the second. Can anyone see any possible implications with this? It seems to me like it work as though all of the subModels had a call to fetch() each time a superModel was fetched from the server. You could possibly have problems with models getting updated, someone is trying to set an attribute on one of them though. Unsure if the second solution would work.

@PaulUithol
Copy link
Owner

Well, this is a very long thread, and it seems to be quite muddy what the real issue is (mostly because the OP didn't define his problem). I still do think there quite likely is a problem with fetchRelated.

It doesn't seem like anyone has an actual stack trace of the particular error he's dealing with, but if I had to guess I'd say it the lines using rel.relatedModel.build look suspect, because they don't check the store before calling a function that will create a new model. Could anyone that's actually experiencing problems with fetchRelated try to replace these with rel.relatedModel.findOrCreate?

@al
Copy link

al commented Nov 27, 2012

@PaulUithol I guess that's probably a more elegant solution than my code which uses if/else to find or build. I can try it out later today probably and get back to you.

@al
Copy link

al commented Nov 27, 2012

@PaulUithol Yeah, that seems to work. http://jsfiddle.net/8RrNk/1/. Here is the same fiddle using the current code http://jsfiddle.net/UhYzK/2/. And again using the code I proposed (your solution is cleaner): http://jsfiddle.net/ah5wM/2/.

@PaulUithol
Copy link
Owner

Okay, made the change. Hope this does the trick!

@cob4lt
Copy link

cob4lt commented Dec 1, 2012

Can someone explain me same error that I am having? What I am doing wrong?

http://jsfiddle.net/n6tZ9/

If I before creating second model Znak, save Stup model on server and then create new Znak, I get same error?
How can I achive that I can create more than I model at a time?

@DouweM
Copy link
Collaborator

DouweM commented Dec 1, 2012

@cob4lt This happens because you're defaulting your model ids to "", which Backbone(-relational) doesn't know is your way of saying there's no id; Backbone(-relational) interprets "" as being the actual id, so when you create two Znakovi objects, they'll both have the same id (namely ""), resulting in the exception. You also don't need to set a default value for znakovi, Backbone-relational will handle that.

@cob4lt
Copy link

cob4lt commented Dec 1, 2012

Thanku very much. I didn't knew that

@lovanwubing
Copy link

I got the same issue, and it makes me feel really uncomfortable for the upgrading.

Besides that, I think both Backbone and RelationalModel becomes more and more complicated, and definitly it's not the right direction. More simple, much better.

@patrick-radius
Copy link

I still have this issue when using 0.8.5 in combination with backbone paginator 0.7.0.
I 'fixed' this by changing
if ( duplicate && model !== duplicate ) {
to
if ( duplicate && model === duplicate ) {

on line 423

@theturtle32
Copy link

What if you specifically WANT two different instances of a model with the same data, so that changes to one are isolated from the other?

Say you have a shopping cart display, with an edit form that pops up over the top with options for modifying the quantity and size/color selections. The popup window should be bound to a completely separate instance of the CartLineItem based on the same source data, with the same ID, so that if the user cancels the edit dialog, the underlying cart form is not updated as a side effect of changing the values in the edit overlay.

This is a major, major showstopper issue for me in my current project. There are many, MANY, instances in which I would potentially want to have multiple — disconnected — instances of a model sourced from the same data.

@philfreo
Copy link
Collaborator

philfreo commented Oct 8, 2013

@theturtle32 in the case you're describing (having a form where the values shouldn't really be saved in case the user clicks cancel) it sounds like a better idea to not actually update your model every time they change the form values, but rather to update your model once they click "Save" or close the modal or whatever.

I do this frequently with Backbone-Forms. The form values can get changed as much as you want but then you don't call form.commit() (which internally calls model.set()) until you're ready.

If you HAD to do what you're describing, then the hack would be to remove the ID when you clone the model.

@theturtle32
Copy link

My form doesn't use standard HTML form elements... Each parameter is represented by a backbone view that needs to be re-rendered when the data changes to show the form state being updated.

@theturtle32
Copy link

Basically two-way data bindings all the way around from top to bottom.

@al
Copy link

al commented Oct 8, 2013

@theturtle32 I think what you're asking for is unreasonable/magic.

The obvious answer is that suggested by @philfreo. I.e. you don't persist the changes (locally) until the user tells you to ... this is surely the most intuitive [obvious] behaviour. If you require something extra (live updating of other sections of the page or something?? ... I really can't imagine the problem), then implement it in a decorator fashion because that it s whats it is ... decoration. Leave the core behaviour rational and decorate it.

What you're suggesting (multiple representations of a single instance) is a recipe for havoc that you will surely regret.

@theturtle32
Copy link

I must disagree. For me, the primary strength of using a framework like Backbone and Backbone-relational is for the view components to instantaneously respond to changes on the model. Models back views. That's how Backbone is designed and meant to be used. It's not at all out of the ordinary to want to have your view components update themselves in response to changes on the models, and to design your application's views around that very property. I can't imagine why that would be a recipe for havoc? This is the paradigm around which many other frameworks, both for HTML5 apps and native apps are based even more prolifically.

@theturtle32
Copy link

The whole point of using client-side models is to be able to change them on a whim and then decide when to persist them on the server, or to throw away those changes when the user hits the cancel button.

@al
Copy link

al commented Oct 8, 2013

Of course. "instantaneously respond to changes on THE model" ... That is what we all do. You are suggesting the creation of two models for the same object to achieve this, which to me suggests a fundamental misunderstanding of the architecture. Sorry, but it is clearly a nonsense to have two representations of a single instance and somehow expect them to be connected, and when you try to implement it in the context of code that anyone other than you has written, it will result in havoc.

This is way off topic for this issue. Respectfully, I think you should post to the forum and be shot down there :)

@theturtle32
Copy link

...what forum? There's no forum listed on the backbonerelational.org website?

@theturtle32
Copy link

And respectfully, I don't think it is off topic for this thread, both because the OP didn't indicate the use case that triggered the error message -- which is the rather arbitrary error that is getting in my way -- and I've used previous versions of Backbone-relational that did not have this limitation for other projects with great success. My contention is that this error condition shouldn't exist in the first place, or should be a silenceable warning. I'm used to most other frameworks actually having facilities for a clone() method on models to enable exactly the use case I'm describing.

@theturtle32
Copy link

Also: "two representations of a single instance and somehow expect them to be connected" -> I specifically want them to NOT be connected in any way shape or form.

@philfreo
Copy link
Collaborator

philfreo commented Oct 8, 2013

Regardless of how you think it SHOULD work, there's no simple way to make Backbone-Relational really support multiple instantiations of the same ID-ed model. For better or worse, this is by design. BBR relies on a global object store to automatically link models together anytime their ids are referenced. You could comment out the id check but all sorts of subtle things would start breaking (#180 for one example).

  • If you're not ready to commit to your model, then don't! There's plenty of great ways to use the DOM that doesn't involve immediately writing to your model. My preference is backbone-forms.
  • Commit to the single model right away but preserve the state of the attributes so you can "rollback" upon canceling.
  • Come up with some clone() that works by not copying the ID over.
  • Use something more lightweight that doesn't rely on a global object store / single instantiations but then lose the benefits that having such a store provides you (automatically linking models up from various fetches, etc.). I'd recommend backbone-associations in this case.

@pcompassion
Copy link

I have this issue with backbone-0.8.6 and backbone.paginator-0.8.1

line 439.

@nexxTM
Copy link

nexxTM commented Dec 17, 2013

After the server changed the name of the id field, I had this issue, because the checkId function got "" as an id.

Besides that, it works for me with:
backbone 1.0.0
backbone.paginator 0.8.1
backbone-relational 0.8.5

@dcrockwell
Copy link

@philfreo While I appreciate that your approach is the most efficient in terms of Backbone.js's architecture, there is a serious pain for iterative legacy system upgrades. This design decision forces me refactor all model-related code at once for systems that inefficiently pull two models of the same id, but on different "pages", lest I apply the monkey patch from above in these comments.

I hope you'll sympathize that before this design decision was imposed, an entire application worked, and since I applied a simple hack to remove the checkId method, the application continues to work. Therefore, it goes to reason that there should be an option to disable this check while accepting the "subtle" consequences that you warned of before.

automatic-frog pushed a commit to osp/osp.work.oralsite.www that referenced this issue Jan 1, 2015
the revision browser which works but would need better integration in
the interface.

This commit also include the switch from backbone-relational to
backbone-associations because it solves the issue of fetching multiple
times a model with different revisions (see
<PaulUithol/Backbone-relational#186>).

Also the switch from Underscorejs to Lodash: a superset of underscore
with one useful extra feature to get the indexOf a map in a collection

I included font-awesome for the convenient player icons it offers but
frankly the aesthetic is tasteless and I'd better dig the hotglue-like
interface of contextual-menus.
@jameshowe
Copy link

I am experiencing a similar problem, partially touched on in this thread by @gfarrell.

I too am fetching a collection which contains Backbone.RelationalModel's, the problem I have is when I re-fetch this collection the relations property does not appear to update (even though the returned data has now changed) because it's re-using the existing model from the store.

Shouldn't the library internally be calling fetchRelated per model in a collection when fetch is called? Anyway, just as a test, I tried clearing the cache before calling fetch i.e. Backbone.RelationalModel.store.reset() and that's when the error is thrown.

Can't seem to see a reasonable way around this, to be honest I've found my time using the library that I've spent more time working against it than I have with it :(

@dilame
Copy link

dilame commented May 21, 2017

Somebody, please, tell me, how could it be?

2017-05-21 22 29 03

If you wanted to protect me from bugs, you overdid it. I've spent much more time to fight with Backbone-Relational. still no luck.
Please, let me decide by myself how to organise my code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests