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

Should didUpdate be triggered on reload? #3748

Closed
jcbvm opened this issue Sep 10, 2015 · 10 comments
Closed

Should didUpdate be triggered on reload? #3748

jcbvm opened this issue Sep 10, 2015 · 10 comments

Comments

@jcbvm
Copy link

jcbvm commented Sep 10, 2015

I stumbled across an issue in my current app, I was under the impression that a reload would trigger a didUpdate event of a model, but it only seems to be triggered when calling save. Looks like I'm not the only one #1441.

Shouldn't didUpdate be triggered when the model gets updated from the server (either via reload, save, pushPayload etc...)?

@st-h
Copy link

st-h commented Jul 17, 2017

I was running into the same issue, being unable to properly observe changes to a current record that is initiated by the server via pushPayload: https://discuss.emberjs.com/t/observe-reload-changes-to-model/13182

I noticed emberfire is currently having an open issue with a similar issue: FirebaseExtended/emberfire#497

I have been trying to find out if there is a reason for ember data to not trigger any hooks via pushPayload, however nobody was able to come up with a reason so far. I think it would make things a lot easier if we had a consistent behaviour for something like didUpdate (which actually sounds like it should trigger every time the record gets updated on the server - and hence the client). At least it seems to be possible to work around this issue by using a customised adapter, as described in the ember fire issue.

@danielspaniel
Copy link

I notice that the when the store does a background reload or even store.findRecord({reload: true}) the ready or didLoad hooks are not called again, which I think is a flaw.
But I am curious about what others think.

@reppa-fe
Copy link

reppa-fe commented Oct 4, 2017

I think it makes sense to trigger didUpdate whenever a model is updated "externally." We too receive data from a live data source and update it by pushing it into the store. didUpdate not being called in this case was both surprising and lead us to build a workaround that somewhat break separation.

@st-h
Copy link

st-h commented Oct 4, 2017

Yes! I would really like to get this resolved. It makes it quite difficult to use things like ember changeset together with server push. Or write components that display data of a record without getting out of sync. Do we need an application that shows the issue? I think it should be sufficient to agree on the wanted behavior - and then have a look at the existing tests?

@danielspaniel
Copy link

@wecc any conclusions from team discussions? need help implementing this if you are ok with the basic idea of calling didLoad after reload, pushPayload etc.

@runspired
Copy link
Contributor

We are looking to deprecate record events/event-hooks in favor of using promises and functional patterns. Closing this because we do not with to add more complexity to a layer we are seeking to remove.

@reppa-fe
Copy link

@runspired Is there an Issue or RFC to track this pattern change?

@ro0gr
Copy link

ro0gr commented Feb 4, 2020

I just found didUpdate hooks doesn't work with pushPayload(, and it was surprising, cause didUpdate is mentioned in the docs as a public API https://api.emberjs.com/ember-data/3.16/classes/Model/events/didUpdate?anchor=didUpdate

Also didLoad seems to work properly to me, which is inconsistent.

Is there any alternative to didUpdate? Should didUpdate be removed from the docs?

@runspired
Copy link
Contributor

@ro0gr didUpdate does work, more likely this wasn't a situation in which an update was performed.

@ro0gr
Copy link

ro0gr commented Feb 11, 2020

Oh, then that is probably related to version of ED@3.12 which I'm currently pinned to. I'll check. Thanks for letting me know didUpdate is supposed to work!

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

7 participants