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

Sideloaded records not available on init of model (behavior change between beta 19.2 and 1.13 series) #3545

Closed
jcope2013 opened this issue Jul 13, 2015 · 6 comments

Comments

@jcope2013
Copy link
Contributor

you will see that in beta 19.2, the sideloaded records are available and that calling store.find doesnt send out an extra request

USER COUNT NOTIFICATION MODEL: 1
USER COUNT NOTIFICATION COMPONENT: 1

beta 19.2 http://emberjs.jsbin.com/yepilijuva/1/edit

in 1.13+ the behavior changes, the user count is 0 during the init process and eventually gets to 1 at a later point, so a request is sent out even though the record is sideloaded

USER COUNT NOTIFICATION MODEL: 0
USER COUNT NOTIFICATION COMPONENT: 1

http://emberjs.jsbin.com/duweqosizi/1/edit

i am thinking about losing the observer and instead making it a computed property which is prolly better suited at this point to guarantee the record is sideloaded first

http://emberjs.jsbin.com/fazejoviyu/1/edit

@runspired
Copy link
Contributor

Bumped into this too, the same bug existed on ember-data beta.18, I upgraded to ember-data 1.13.5 to see if it would fix and it did not.

@pwfisher
Copy link

We are getting spurious requests for sideloaded models, too.

@wecc
Copy link
Contributor

wecc commented Jul 16, 2015

Would it be possible to create a minimal JSBin demonstrating this issue? Without a component and observers? I'd be happy to dig into this but I'm having a hard time following the JSBins provided :(

@runspired
Copy link
Contributor

@wecc I figured out how to consistently trigger the issue, it wasn't as spurious as I'd at first supposed and I don't think it's something that ED should account for but which perhaps should be documented for users with partial records and sideloading.

Let's say you have this foo model. In my case here, the isPartial flag is set to false when a record is loaded directly instead of via the index. It's not necessary to have this to replicate this issue but it helps to understand the flow of what triggers this to know that it was related to a partial record.

/models/foo.js

export default DS.Model.extend({
    name: DS.attr(),
    bars: DS.hasMany('bar', { async: true }),
    isPartial: true
});

Given the following record where none of the bars have been loaded (because the record is still in partial form).

{
    foo: {
       name: 'hello world',
       bars: [1, 2, 3],
       isPartial: true
    }
}

Load the record as the model for a route

model: function(params) {
    return this.store.find('foo', params.id).then(
      (record) => {
        if (record.get('isPartial')) {
          record.reload()
            .then(() => {
              record.set('isPartial', false);
            });
        }
        return record;
      }
    );
  },

The bars are sideloaded when the record is loaded directly. By instead waiting for the record to finish reloading you avoid this issue.

model: function(params) {
    return this.store.find('foo', params.id).then(
      (record) => {
        if (record.get('isPartial')) {
          return record.reload()
            .then(() => {
              record.set('isPartial', false);
              return record;
            });
        }
        return record;
      }
    );
  },

@fivetanley
Copy link
Member

This looks like it was fixed on the ActiveModelAdapter repository, so we should backport the changes to 1.13. Here's an example that uses rawgithub.com to load the dist version of the active model adapter, but is working for this case. http://emberjs.jsbin.com/yezosubijo/1/edit?html,css,js,output

@fivetanley
Copy link
Member

Backporting the AMS changes to the release branch here. It'll land tomorrow in 1.13.6. #3564

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

No branches or pull requests

5 participants