Improved semantics around model hook reloading

[davidlormor]

I've recently run into some frustrations with the way model hooks are called as it relates to forcing the reload of a model. The conversation originated in emberjs/data#3704 with me thinking there was a problem with the model not calling reload despite the reload: true option being passed into findRecord in the route's model hook.

Basically, I had the query in my model hook set to call reload, but because I was passing in the entire model to the link-to helper (rather than an id) the model hook never fired, therefore never calling reload on the passed-in model.

There's also not a lot of clarity around the timing of model loading in the route. It's hard to tell from a particular whether the route will wait for the model, initialize and load the model in the background, or some other behavior. I'm super excited about the new Ember Data background reloading and caching functionality, but it seems to be more well documented in the guides, perhaps.

I think there are several things that could be done to improve this from API changes to better docs. Just curious if anyone else has experienced similar issues, and what feedback they might have on how to make this less confusing (I'm a "veteran" Ember programmer, but could imagine this being especially painful to some newcomers if they don't understand all the semantics of the link-to helper and route-level events).

Some thoughts:

• An explicit modelHook attribute on link-to helpers, to specify that, regardless of whether an id or model is passed to a dynamic url segment, the model is refreshed
• Better documentation - the Guides only briefly mention passing the model vs. the id in link-to helpers, here and here.

I realize that these pain points can all be avoided with "good programming practices," but there is a lot of subtlety in the API here that could be made more declarative to improve the dev experience.

Thanks and looking forward to input from others! Happy to help out where I can.

[locks]

That's two different places in the guides where the semantics are referenced, on top of the API documentation. Suggest specific improvements to the Guides repo, as that is easier for us to keep track of and give feedback.

I don't agree with the modelHook attribute, especially if it'll override the current semantics leading to more unexpected behaviour. Another question that might necessitate some more discussion is what happens when the model hook isn't just, say, a store call, but a hash of several of them?

[davidlormor]

Thanks for the input locks. I'll definitely make any specific suggestions re: improving the guides over in that repo.

I know that skipping the model hook via a passed in object is legacy API that has been around for quite sometime (and there are definitely cases where the behavior is warranted) but I believe a more declarative syntax would be beneficial in reasoning about an application behavior. I also believe this behavior was much more desirable when Ember Data lacked the semantics and facilities to do determination about when to reload models (via {reload: true} in queries, shouldBackgroundReload properties, etc.). Because this functionality now exists, the ability to pass in a model to avoid an API call is much less necessary.

Also, given JS's dynamic typing, there isn't anyway to guarantee what's being passed in to the helper, which leads to potential nondeterministic behavior in route initialization. Ultimately, there is no way to specify the desired behavior, either in the helper or the route...it is all determined at run time based on the type of object passed in. In general, this would not be good practice and potentially leads to challenges in reasoning about and debugging parts of an application.

For examples of the confusion caused, see discussions here and here.

IMHO (for whatever that may be worth...), I think having a clear interface to define whether a route should fire the model hook is a win across the board. The existing behavior could continue to remain the same, so as not to be a breaking change, but a higher degree of specificity could be achieved where a developer desires.

Finally, regarding your last point about more complex model hook behaviors...I think that's something that could warrant discussion, but is really outside the scope of what I'm proposing.

Thanks for taking the time to consider this change!

[wagenet]

I'm closing this due to inactivity. This doesn't mean that the issue is invalid, but that, unfortunately, nobody has taken the effort to spearhead it and bring it to completion. Please feel free to advocate for it if you believe that this is still worth pursuing. Thanks!