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

How to tell if a relationship has been loaded #5629

Closed
ryanto opened this issue Sep 11, 2018 · 12 comments
Closed

How to tell if a relationship has been loaded #5629

ryanto opened this issue Sep 11, 2018 · 12 comments
Labels
feature-suggestion 🏷️ feat This PR introduces a new feature

Comments

@ryanto
Copy link
Contributor

ryanto commented Sep 11, 2018

I'm wondering if Ember Data has a way to know if a relationship has already been loaded.

I'm running into a situation where reference's load API changes based on wether or not the relationship has been loaded.

For example...

let reference = post.hasMany('comments');
reference.load() // no-ops if the comments have already been loaded

I'd love to have a method to check if the relationship has been loaded in Ember Data's eyes. This would allow me to do something like...

let reference = post.hasMany('comments');

if (reference.hasLoaded()) {
  reference.reload();
} else {
  reference.load();
}
@runspired
Copy link
Contributor

@ryanto we have a great deal of information about the relationship state and whether we need to fire a request encapsulated in the relationship state class. I've been thinking about the possibility of making this public API, because I want folks to write better relationship computeds (it is very doable if the state class is public).

From a private API perspective, you'll find that the relationship state class is attached to the reference. That should be enough to get you started and to figure out if there are some useful things to RFC. I suspect that proxying the method shouldMakeRequest and some of the other flags would make sense.

@ryanto
Copy link
Contributor Author

ryanto commented Sep 11, 2018

Cool! I had this working with some private APIs off of the reference, but it looks like this has changed since 3.1. Previously, I was relying on hasData and hasLoaded, but these no long exist.

That made me realize that my code is pretty brittle and I need to do this in a blessed public API way.

Given that the relationship state class isn't public, is there a way to figure this out with public APIs? Some decision tree based on reference values or ids? I've been playing around with a few things, but can't get anything to stick.

If there's no public API for this sort of thing, which it sounds like that's the case, what's the best way for me to help push one through? Addon code that leads to an RFC?

@sly7-7
Copy link
Contributor

sly7-7 commented Sep 12, 2018

@ryanto I think you can test reference.value() for this ?
Maybe I don't understand what you want, but it seems to me that something like

let reference = post.hasMany('comments');

if (reference.value() === null) {
  reference.load();
} else {
  reference.reload();
}

should work

@ryanto
Copy link
Contributor Author

ryanto commented Sep 12, 2018

Thanks, unfortunately that won't work for a belongsTo relationship that is null.

let user = await store.findRecord('user', 1, { include: 'employer' });

let reference = user.belongsTo('employer');

// value is null, but employer has already been loaded. so we need to use reload here.

if (reference.value() === null) {
  reference.load(); // no-ops
} else {
  reference.reload();
}

This however does work for hasMany relationships.

@runspired
Copy link
Contributor

I am surprised it works for hasMany relationships.

@YoranBrondsema
Copy link

I confirm, it seems that value() === null for unloaded hasMany relationships in both 3.1 and 3.2 (I haven't tried on 3.3 nor 3.4).

That aside, I agree that it would be very helpful to have a public hasLoaded function.

@abdullah2891
Copy link

can we have a listener which store operation is being used on the model?
for example this.get('store').queryrecord('model'... and we can have a property which tracks it

@Herriau
Copy link

Herriau commented Feb 10, 2020

I am surprised it works for hasMany relationships.
@runspired

It works because null isn't ambiguous for hasMany relationships:

  • ref.value() === null if and only if relationship is not loaded
  • ref.value() instanceof DS.ManyArray if and only if relationship is loaded

It's ambiguous for belongsTo relationships because:

  • ref.value() === null: if relationship is not loaded
  • ref.value() === null: if relationship has loaded but its value is null ("optional belongsTo")

So we need a way to disambiguate between these two cases. The introduction of a hasLoaded() method on BelongsToReference / HasManyReference as suggested by @ryanto would certainly help with that.

I spent some time trying to come up with a way to disambiguate the API of BelongsToReference objects using only public EmberData APIs. I haven't had enough time to try and poke holes in this approach, so I'm not quite sure about its viability, but I'll leave this here anyway:

app/serializers/application.js

export default DS.JSONAPISerializer.extend({
  /** @inheritdoc */
  extractRelationship(...args) {
    const relationshipHash = this._super(...args);

    // There is no public EmberData API that lets us probe whether a `belongsTo` relationship
    // has been loaded (see https://github.com/emberjs/data/issues/5629), so we'll just throw
    // a `loaded` flag in the meta hash for any `belongsTo` relationship included in the payload
    // (i.e. where `data` is a resource identifier object). This flag can be consumed on
    // `BelongsToReference` objects (e.g. `record.belongsTo('someRel').meta()`).
    return Object.assign('data' in relationshipHash ? { meta: { loaded: true } } : {}, relationshipHash);
  },

  /** @inheritdoc */
  normalizeFindBelongsToResponse(...args) {
    // There is no public EmberData API that lets us probe whether a `belongsTo` relationship
    // has been loaded (see https://github.com/emberjs/data/issues/5629), so we'll just throw
    // a `loaded` flag in the top-level meta hash of any payload we got as a result of (re)loading
    // a `belongsTo` relationship. This flag can be consumed on `BelongsToReference` objects
    // (e.g. `record.belongsTo('someRel').meta()`).
    return Object.assign({ meta: { loaded: true } }, this._super(...args));
  },
});

app/utils/is-belong-to-relationship-loaded.js

export default function isBelongToRelationshipLoaded(ref) {
  // We'll consider a `belongsTo` relationship to have been loaded if it was fetched either
  // directly (using a dedicated `Adapter.findBelongsTo()` request) or indirectly (through
  // includes when fetching the owned resource), or when the inverse relationship has been
  // loaded (i.e. `ref.value() !== null`).
  return ref.meta()?.loaded || ref.value() !== null;
}

Edit (2/11/2020)

Turns out the above solution does not work in the case a belongsTo relationship is loaded separately (e.g. someRecord.belongsTo('someRel').load()) and the server responds with { "data": null } because when data is null EmberData won't even bother copying the meta hash over to the BelongsToReference object.

I experimented a bit with DS.Snapshot (even though the docs are terribly ambiguous as to whether DS.Snapshot is in public or private land...). The API Documentation for DS.Snapshot.belongsTo gave me a lot of hope:

returns (Snapshot|String|null|undefined)
A snapshot or ID of a known relationship or null if the relationship is known but unset. undefined will be returned if the contents of the relationship is unknown.

From the sound of it, DS.Snapshot's belongsTo() exposes the exact API that I would have expected from BelongsToReference.value() by properly disambiguating the "loaded but empty" vs. "not loaded" cases. Yay!

Here is a quick way to get our hands on a DS.Snapshot just for experimentation purposes:

app/serializers/application.js

export const returnSnapshot = Symbol();

export default DS.JSONAPISerializer.extend({
  serialize(snapshot, options) {
    if (options[returnSnapshot]) {
      return snapshot;
    }

    return this._super(...arguments);
  },
});

app/utils/get-snapshot.js

import { returnSnapshot } from 'my-app/serializers/application';

// Don't use this in a real app, this is bad
getSnapshot(record) {
  return record.serialize({ [returnSnapshot]: true });
}

However sadly:

const someRecord = store.findRecord(...);
await someRecord.belongsTo('someRel').load(); // request returns `{ "data": null }`
getSnapshot(someRecord).belongsTo('someRel'); // undefined 😫

Note that the following works:

const someRecord = store.findRecord(..., { include: 'someRel' });
getSnapshot(someRecord).belongsTo('someRel'); // null

EmberData is at least beautifuly consistent in how broken it is. I am not goign to lie, I am getting a bit frustrated over this. I was... just trying to determine whether a given relationship has been loaded.

@runspired
Copy link
Contributor

@Herriau reach out on discord I'd love to pair for a little to poke at this

@joshkg
Copy link

joshkg commented Feb 12, 2021

Yearly check-in -- any progress on this issue? 🤔 Would also love to be able to check if my async relationship has been fetched or not 🙏

@runspired
Copy link
Contributor

@joshkg yes, several of the linked issues have likely been resolved but need test PRs to confirm. They can be fixed fairly easily at this point if they aren't fixed already.

@runspired
Copy link
Contributor

runspired commented May 26, 2021

@joshkg @Herriau fixes are on master and will be in 3.28 beta 2 for meta / links / null vs undefined on belongsTo.

Closing this as this seems sufficient at this time to determine if a load has occurred.

Relevant PRs: #7532 #7533 #7534

@runspired runspired added feature-suggestion 🏷️ feat This PR introduces a new feature and removed Feature labels Sep 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-suggestion 🏷️ feat This PR introduces a new feature
Projects
None yet
Development

No branches or pull requests

7 participants