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

Ember Data tries to reload an already-loaded belongsTo #4099

Closed
samselikoff opened this issue Jan 19, 2016 · 19 comments · Fixed by #5410
Closed

Ember Data tries to reload an already-loaded belongsTo #4099

samselikoff opened this issue Jan 19, 2016 · 19 comments · Fixed by #5410

Comments

@samselikoff
Copy link
Contributor

Ember data models:

// models/author.js
import DS from 'ember-data';

export default DS.Model.extend({

  name: DS.attr(),
  posts: DS.hasMany()

});

// models/post.js
import DS from 'ember-data';

export default DS.Model.extend({

  author: DS.belongsTo()

});

I'm using a JSON:API backend (and adapter on the frontend). My application route (with the ds-finder-include feature flag enabled):

// routes/application.js
export default Ember.Route.extend({
  model() {
    return this.store.findRecord('author', 1, {
      include: 'posts'
    });
  }
});

which correctly sends out the request

GET http://localhost:3000/authors/1?include=posts

which responds with https://gist.github.com/samselikoff/20012aa58075db6124de.

In my application template,

<h2>I am {{model.name}}, here are my posts:</h2>

<ul>
  {{#each model.posts as |post|}}
    <li>{{post.id}} (written by {{post.author.name}})</li>
  {{/each}}
</ul>

{{post.author.name}} correctly displays the author's name, but triggers n+1 requests to /posts/1/author, /posts/2/author and so on. So, the inverse relationship is loaded correctly in Ember Data, but ED is still making an ajax request.

@wecc tracked it down to

BelongsToRelationship.prototype.getRecord = function() {
//TODO(Igor) flushCanonical here once our syncing is not stupid
if (this.isAsync) {
var promise;
if (this.link) {
if (this.hasLoaded) {
promise = this.findRecord();
} else {
promise = this.findLink().then(() => this.findRecord());
}
} else {
promise = this.findRecord();
}
return PromiseObject.create({
promise: promise,
content: this.inverseRecord ? this.inverseRecord.getRecord() : null
});
} else {
if (this.inverseRecord === null) {
return null;
}
var toReturn = this.inverseRecord.getRecord();
assert("You looked up the '" + this.key + "' relationship on a '" + this.record.type.modelName + "' with id " + this.record.id + " but some of the associated records were not loaded. Either make sure they are all loaded together with the parent record, or specify that the relationship is async (`DS.belongsTo({ async: true })`)", toReturn === null || !toReturn.get('isEmpty'));
return toReturn;
}
};

@wecc
Copy link
Contributor

wecc commented Jan 19, 2016

Simplified reproduction: http://emberjs.jsbin.com/qetidomara/1/edit?html,js,output

There shouldn't be additional requests to /posts/1/author and /posts/2/author

@wecc
Copy link
Contributor

wecc commented Jan 19, 2016

I think we need to differentiate between loaded and unloaded records (record.dataHasInitialized?) in relationship.addCanonicalRecord() to be able to set the relationship's hasLoaded flag correctly.

@csantero
Copy link
Contributor

I don't think this is specific to belongs-to relationships. I've been experiencing the same behavior with has-many.

@hvgotcodes
Copy link

Posting to follow. Also see this.

@wecc
Copy link
Contributor

wecc commented Oct 21, 2016

This should have been solved since #3762 – would you be able to confirm?

@wecc
Copy link
Contributor

wecc commented Oct 21, 2016

Never mind, this is probably Part 2 of the local data vs. links and therefor not solved by #3762. I did a writeup in #4292 which I think applies to this issue too.

@runspired
Copy link
Contributor

@wecc this appears to be unsolved based on helping someone in Slack this evening. Will likely try to dig in this week.

@digia
Copy link

digia commented Nov 27, 2016

I'm having what appears a pretty similar experience, @runspired and i actually debugged a bit.

However i'm finding that if i use a store.findAll('author', { include: 'posts' }) or a store.query('author', { include: 'posts' }) all of the ED models relationships will link properly.

@samselikoff could you check that on your end as well?

@nadnoslen
Copy link

I'm running a Rails JSONAPI-Resources back-end with Ember-Data 2.13. No particular customizations to the Ember-Data adapters or serializers or the JSONAPI-Resources ValueFormatters. No Ember canary flags enabled.

I use the 'include' directive extensively in my store.query(...) calls; these work most of the time. I have found that store.findRecord(..., { include: '...'} is inconsistent in processing any included relationship data. This introduces several additional relationship requests even though I've included them and see them come down in the payload from the server.

When I upgrade past Ember-Data 2.13 (up to Ember-Data 2.14 -> 2.17 at time of writing this message) I observe a plethora of additional server api request for relationship data due to include payload-data not being properly consumed/linked by the store. This was not an issue in Ember-Data 2.13 and most certainly impacts my previously successful store.query(..., { include: '...' }) calls. I have locked my Ember-Data version to 2.13 for now to avoid this problem...not ideal.

@jvenezia
Copy link

jvenezia commented Dec 22, 2017

Having a similar problem.

This works as expected without triggering unnecessary queries:

    this.store.findRecord('trip', params.trip_id, {include: 'steps,steps.activities'});

But when records are already loaded before (without including associations):

    this.get('store').findAll('trip', {reload: true})
    ... route transition ...
    this.store.findRecord('trip', params.trip_id, {include: 'steps,steps.activities'});

It triggers a query for each step, even if they are already successfully loaded with the include.

@adaml881
Copy link

I'm having the same issue I can eager-load a belongsTo relationship successfully and i see the records in Ember data (in the developer tools plugin). But as soon as I try to use those records it attempts to reload the data from api. This was in 2.18.0. Following @nadnoslen and downgrading to 2.13.0 seems to fix it

@elonmulder
Copy link

elonmulder commented Jun 22, 2018

Can anyone confirm this is actually fixed? I've upgraded Ember Data to the latest version (3.2.0-beta.2) but am still getting the exact same behaviour @jvenezia is describing.

@runspired
Copy link
Contributor

@elonmulder @adamlawson @jvenezia it’s likely your payloads aren’t specifying the relationship correctly.

@nadnoslen
Copy link

Thanks @runspired!

I just finally got around to upgrading from Ember-Data-2.13 to Ember-Data-3.2 in one of my larger projects and was able to see this fix (among others) in action.

Thank you Ember-Data team.

@tpetrone
Copy link

tpetrone commented Dec 7, 2018

I can confirm this is still an issue with ember-data-3.5.0:
Using findRecord in a specific resource with the include option causes an additional request to host/_includedResource_/id.

Downgrading to ember-data-3.2.0 showed expected behaviour:
Using findRecord in a specific resource with the include option triggers exactly one request to target resource.

In both scenarios includedResource properties are being called in the template like:
resource.includedResource.prop

@skthr
Copy link

skthr commented Dec 20, 2018

I'm currently trying to upgrade Ember Data from 2.11.1 to 3.5 and having this exact problem. Going back to 3.2.0 Doesn't fix the problem for me.

@runspired you suggested that perhaps the problem is that the payloads aren't specifying the relationship correctly. Is there any documentation out there that specifies the exact payload/json structure? Our project uses a serializers/application.js file, so I'm hoping that I can just adjust the normalizeResponse function to ensure that the JSON is formatted correctly?

@nadnoslen
Copy link

nadnoslen commented Dec 20, 2018

Hey @skthr,
I've noticed that Ember Data does an incredibly good job of adhering to the JSONAPI specification. Verify that your payloads are compliant: https://jsonapi.org/format/

Here's the blurb on relationships

and here are some simple examples

Make sure you're looking at v1.0 of the spec. I say this because a release candidate of v1.1 has been published.

@runspired
Copy link
Contributor

@nadnoslen @skthr the reload issue is fixed through 3.4, but may be present again 3.5.

@skthr
Copy link

skthr commented Dec 21, 2018

Thanks @nadnoslen and @runspired

Going back to 3.4 didn't help so I'm pretty confident that there is something wrong with the payload (at least that seems like the best place to look). If I compare our payloads to the JSONAPI spec I'm sure I'll find the issue eventually.

Much appreciated. I'll update when I make some progress.

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.