-
Notifications
You must be signed in to change notification settings - Fork 77
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
Injecting an item with relationships a second time breaks relationships #253
Comments
It's probably obvious, but simply adding |
OK, I've found a solution that may or may not be acceptable, because it's a tiny-bit hackish: At line 150 of inject.js, inserting this code works, while preventing infinite recursion: //149 DS.utils.deepMixIn(item, attrs);
if (definition.relations && !item.$$angular_data_did_inject) {
item.$$angular_data_did_inject = true;
_injectRelations.call(DS, definition, item, options);
delete item.$$angular_data_did_inject;
}
// if (definition.resetHistoryOnInject) { The hack is, hopefully obviously, that I needed to track which items were already injected this time around, so I set an arbitrary variable directly on the object, then delete it when I'm done. This won't prevent relationship trees from being processed more than once, if (for example) the same item is attached multiple times, but it will at least guarantee you never get a stack overflow. If this seems like an acceptable solution, I'd be happy to turn this into a PR. This is a pretty serious issue for the way we have been intending to use angular-data, and is going to really set me back in the design of the application. |
Angular-data actually used to have the implementation you're suggesting, but there were other issues that were solved when I removed it. The relations are a complicated feature... |
I'm trying desperately to find where it used to work like this, and what problems were fixed, but it's really hard to track down what changes were made to Do you have any good starting point as to what issues were being caused? I'd hate to have to maintain a fork of this library because it's a fairly serious issue (and I can't imagine it's never caught anyone before, where a relation was loaded in two different ways). Our use case right now is something like this:
I'm pretty sure if I tried to purge the ParentA object, it will break anything that has a reference to ParentA. The only fix I can see is to manually hook into the load process for ParentA and manually extract the Children (which seems overly complicated, since we already have the |
The first thing I'd like to do for this issue is add tests for the use cases that have obviously been missed. I'll try to tackle the fix within the next day or two. |
Ah, sorry I didn't reply sooner, I had already written a test to verify my hack. |
And thank you for looking into this! We're rushing to get a product built, so I'm sorry if I sounded ungrateful. |
I believe I have a working fix that does not rely on decorating items with angular-data-specific properties. Because angular-data uses identity-mapping, I can shift the injection of relations to the "pre-inject" phase and simply check the data store itself for items that have already been injected in the current injection process. I will push it shortly, though I'd like to hold off on tagging a new version of angular-data until you have a chance to test the fix with your code. |
@OverZealous I tested the fix with your plunker and it worked, so I went ahead and created a release. |
Looks like that worked for me, I really appreciate it! FYI: there's a ton of |
If you load an item from the server with partial server-side included information, then load it a second time with additional information, relationships will not be correctly injected and updated on the item.
Example showing issue: http://plnkr.co/edit/YpLgDBxL1ro05MKR3v9h
Notice, when you first load the page, the single injected
Parent.children
item is picked up. Upon clicking the button, you would expect the list of children to update, but they do not. This is because_injectRelations
is not called if the item already exists in the cache.The issues caused by this extend much further than just the items not being in the cache. The child items are actually no longer DS resources (even if they were previously), and so they are missing all functions and other behavior that would be expected.
The text was updated successfully, but these errors were encountered: