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

Recursive calls on relationships #1814

Closed
asaf opened this issue Mar 17, 2014 · 2 comments · Fixed by #2208
Closed

Recursive calls on relationships #1814

asaf opened this issue Mar 17, 2014 · 2 comments · Fixed by #2208

Comments

@asaf
Copy link

asaf commented Mar 17, 2014

Hey,

I have two models with hasMany relationship between them, invoking:
modelA.get('model_b') performs a server call to retrieve the relationships,

This is okay, but then per relationship retrieved, I see a server call for the opposite side,

After some debugging, the cause of this is because:

RelationshipChangeAdd.prototype.sync = function() {
  ...
  if (secondRecord instanceof Model && firstRecord instanceof Model) {
  ...
         else if(this.secondRecordKind === "hasMany"){
          secondRecord.suspendRelationshipObservers(function(){
            var relationship = get(secondRecord, secondRecordName);
            if (isValue(relationship)) { relationship.addObject(firstRecord); }
          });
  }
}

The line var relationship = get(secondRecord, secondRecordName); causes a get call to the secondRecord relationship which causes a second server side call per fetched relationship,

This behavior blows up the browser with many server side calls when there are many relationships,

(I currently commented out both lines of the callback but i'm not sure about the impact)

Thanks,

Asaf.

@igorT igorT added ssot and removed ssot labels May 28, 2014
@asaf
Copy link
Author

asaf commented Jul 15, 2014

Bump, this is super important, how come no one suffers from this ?

@altrim
Copy link

altrim commented Jul 15, 2014

@asaf I guess this is related to this issue #2057 also.

@igorT igorT mentioned this issue Aug 20, 2014
igorT added a commit that referenced this issue Sep 5, 2014
This commit refactors relationship handling code. It adds full
support for loading relationships described on only one side in the
api. It sets up the infrastructure for separting server vs local
updates to relationships. In a future PR, we will use that infrastructure
to add support for rollback, relationship dirtying and relationship merging.

Lots of old, crappy code, removed, -550 lines before new tests.
Pushing records into the store is now 15-20% faster.
Relationships now properly refetch if the links value is updated.
We no longer churn records that are not changed in a hasMany push.

Fixes #1919 Fixes #1814 Fixes #2057 Fixes #2191 Fixes #2177
Fixes #2125 #2100  #1790 #1532 #1389
igorT added a commit that referenced this issue Sep 5, 2014
This commit refactors relationship handling code. It adds full
support for loading relationships described on only one side in the
api. It sets up the infrastructure for separting server vs local
updates to relationships. In a future PR, we will use that infrastructure
to add support for rollback, relationship dirtying and relationship merging.

Lots of old, crappy code, removed, -550 lines before new tests.
Pushing records into the store is now 15-20% faster.
Relationships now properly refetch if the links value is updated.
We no longer churn records that are not changed in a hasMany push.

Fixes #1919 Fixes #1814 Fixes #2057 Fixes #2191 Fixes #2177
Fixes #2125 #2100  #1790 #1532 #1389
igorT added a commit that referenced this issue Sep 5, 2014
This commit refactors relationship handling code. It adds full
support for loading relationships described on only one side in the
api. It sets up the infrastructure for separting server vs local
updates to relationships. In a future PR, we will use that infrastructure
to add support for rollback, relationship dirtying and relationship merging.

Lots of old, crappy code, removed, -550 lines before new tests.
Pushing records into the store is now 15-20% faster.
Relationships now properly refetch if the links value is updated.
We no longer churn records that are not changed in a hasMany push.

fixes #1919 fixes #1814 fixes #2057 fixes #2191 fixes #2177
fixes #2125 fixes #2100 fixes #1790 fixes #1532 fixes #1389
igorT added a commit that referenced this issue Sep 5, 2014
This commit refactors relationship handling code. It adds full
support for loading relationships described on only one side in the
api. It sets up the infrastructure for separting server vs local
updates to relationships. In a future PR, we will use that infrastructure
to add support for rollback, relationship dirtying and relationship merging.

Lots of old, crappy code, removed, -550 lines before new tests.
Pushing records into the store is now 15-20% faster.
Relationships now properly refetch if the links value is updated.
We no longer churn records that are not changed in a hasMany push.

fixes #1919 fixes #1814 fixes #2057 fixes #2191 fixes #2177
fixes #2125 fixes #2100 fixes #1790 fixes #1532 fixes #1389
igorT added a commit that referenced this issue Sep 5, 2014
This commit refactors relationship handling code. It adds full
support for loading relationships described on only one side in the
api. It sets up the infrastructure for separating server vs local
updates to relationships. In a future PR, we will use that infrastructure
to add support for rollback, relationship dirtying and relationship merging.

Lots of old, crappy code, removed, -550 lines before new tests.
Pushing records into the store is now 15-20% faster.
Relationships now properly refetch if the links value is updated.
We no longer churn records that are not changed in a hasMany push.

fixes #1919 fixes #1814 fixes #2057 fixes #2191 fixes #2177
fixes #2125 fixes #2100 fixes #1790 fixes #1532 fixes #1389
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

Successfully merging a pull request may close this issue.

3 participants