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

assert-must-preload compatible with loadAll()? #66

Closed
kumy opened this issue Feb 9, 2019 · 3 comments
Closed

assert-must-preload compatible with loadAll()? #66

kumy opened this issue Feb 9, 2019 · 3 comments
Labels

Comments

@kumy
Copy link
Contributor

kumy commented Feb 9, 2019

I'm wondering if assert-must-preload component is compatible with the loadAll() function.

I have a component template such as:

// templates/components/geokret-details.hbs

{{assert-must-preload geokret "owner"}}
<h1>{{geokret.name}}</h1>
Owner: {{geokret.owner.name}}

Route template for details:

// templates/geokrety/details.hbs

{{geokret-details geokret=model}}

My route for visualizing just one item is as follow:

// routes/geokrety/details.js

import Route from '@ember/routing/route';
export default Route.extend({
  model(params) {
    return this.store.loadRecord('geokret', params.geokret_id, {
      include: 'owner'
    });
  }
});

If I wish to display one GeoKret details it's working fine, but if I wish to display a list of GeoKrety, it fails with error Error: Assertion Failed: You tried to render a component:geokret-details that accesses relationships off of a geokret, but that model didn't have all of its required relationships preloaded ('owner'). Please make sure to preload the association. [ember-data-storefront]

Route template for list:

// templates/geokrety/index.hbs

{{#each model as |geokret|}}
  {{geokret-details geokret=geokret}}
{{/each}}
// routes/geokrety/index.js

import Route from '@ember/routing/route';
export default Route.extend({
  model() {
    return this.store.loadAll('geokret', {
      page: {
        size: 2,
        number: 1
      },
      include: 'owner'
    });
  }
});

Ember           : 3.7.3
Ember Data      : 3.7.0
jQuery          : 3.3.1
Ember Bootstrap : 2.4.0

Does someone see what I'm doing wrong?

I've pushed all my code there in case someone need to look deeper.

Thanks!

@kumy kumy changed the title assert-must-preload compatible with loadAll() assert-must-preload compatible with loadAll()? Feb 10, 2019
@ryanto ryanto added the Bug label Feb 12, 2019
@ryanto
Copy link
Member

ryanto commented Feb 12, 2019

Thanks! This is a bug. Right now loadAll doesn't pass the relationships that were sideloaded to the models created by Ember Data.

As a way to get you unstuck today, I'd drop assert-must-preload and instead inside the geokret-details didInsertElement hook use the references API to check if the relationship was loaded.

// geokret-details

didInsertElement() {
  if (!this.geokret.belongsTo('owner').value()) {
    // the geokret's owner wasn't loaded (or it's null). 
    throw "load the owner";
  }
}

The only downside to this approach is that it will fail when the owner is null. If you run into that let me know and I can give you some pointers.

@kumy
Copy link
Contributor Author

kumy commented Feb 12, 2019

@ryanto thanks. I have to test that tonight. Regarding the null case, I'll encounter the problem for the 'holder' field. I would love to see how you solve that. Thanks again.

@ryanto
Copy link
Member

ryanto commented Apr 10, 2019

Hey @kumy, this fell off my radar, but it should be fixed now. It's merged into master and released in 0.16.1, see release here: https://github.com/embermap/ember-data-storefront/releases/tag/v0.16.1

Let me know how it goes, happy to make more changes to our relationship tracking based on your feedback!

@ryanto ryanto closed this as completed Apr 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants