Trigger event / update isReloading on findRecord({ reload: true })

[lcoq]

Version

3.1.0

Scenario

Given a Post with id 1 already loaded, reload it with the following code does not :

• trigger any event (didLoad?)
• update the isReloading property
• call post.reload()

store.findRecord('post', '1', { reload: true });

Thus, other references of this post cannot know if it has been reloaded, contrary to other events (didLoad, didUpdate, etc).

Workaround

The workaround is to add and trigger a didReload event :

// other `post` references that needs to know when it has been reloaded
post.on('didReload', aCallback)

let isAlreadyLoaded = store.peekRecord('post', '1')
store.findRecord('post', '1', { reload: true }).then(function (post) {
  if (isAlreadyLoaded) post.trigger('didReload')
})

I would suggest to also trigger this event on record.reload(). I think this event existed for a while but has been removed, not sure why...

[runspired]

@lcoq by "version 3.1" do you mean this is broken in 3.1 but working previously?

[lcoq]

@runspired I only tried on version 3.1, but I'd tell it was not working previously

[runspired]

isReloading flag is tied directly to record.reload(). @igorT and I recently discussed whether we should also set the flag for reloads that are triggered by passing reload: true and decided against, though our conversation lumped reload and backgroundReload together. Our reasoning was that background reloading triggers often enough it might break some UIs if isReloading is suddenly true very often. Considering reload can only be triggered explicitly there's an argument to be made that it should affect the flag. I'll bring this up for discussion.

[runspired]

the ultimate decision was to not do this, as it would either unnecessarily couple the store to the Model or require fairly substantial logic for relatively minor gain, though individual records might (and can) via usage of the request-cache and filtering for reload on the request options.