Should requester collections trigger load-begin and load-complete on fetch if they aren't tracking ids

[svadla-vecna]

This error makes IE unresponsive.
Looks like fetch() is not handling empty ids correctly. The workaround I had to use is to only call fetch() when there are ids to track.

EDIT:
The original issue was a side effect of mishandling a load-begin/load-complete when there were no ids being tracked.

[kentmw]

@svadla-vecna in a recent release https://github.com/vecnatechnologies/backbone-torso/releases/tag/v0.6.2, we changed a bit of how empty fetches look like.

Is the failure on an unexpected fetch request to the server or is something dying in the javascript? Was there an error in the console?

This is the "track" part of trackAndFetch:
https://github.com/vecnatechnologies/backbone-torso/blob/master/modules/mixins/cacheMixin.js#L93-L95

I don't see where an empty array would kill it. Is it an empty array or a null/undefined?

[svadla-vecna]

@kentmw seems like something is failing in the javascript and causing IE to become unresponsive. I don't see any errors in the console.
trackIds() function is working fine. It is the fetch() function that seems to fail when called using an empty array. And yes, it is an empty array, not null/undefined.
Instead of calling trackAndFetch(), if I just call trackIds() and skip fetch() for empty array, things work fine.

[kentmw]

Does it get to to line https://github.com/vecnatechnologies/backbone-torso/blob/master/modules/mixins/cacheMixin.js#L68?

It shouldn't be doing pretty much any logic if the array is empty.

This check: https://github.com/vecnatechnologies/backbone-torso/blob/master/modules/mixins/cacheMixin.js#L65 should be skipping the bulk of the work.

[kentmw]

We could try to skip the __loadWrapper as well if there was an empty array. That removes the load-begin / load-complete events - which may be expected to fire even if there are no ids to fetch. Not sure I'd like this. If there is any more details at all about debugging the problem, it would help a lot.

[svadla-vecna]

So I have debugged this a little bit more and found out that it's the handlebars template that's causing this issue in IE. The view that I am using is calling render on load-complete event and the handlebars template fails to render when this event is triggered on an empty array. This is because the template is referencing a model attribute and the model ends up being null when we trigger load-complete on empty array. I have fixed this issue by adding a null check to the handlebars template.

I am okay with closing this issue but I guess the question is should we really be firing load-begin/load-complete events on empty arrays.

[kentmw]

@svadla-vecna renamed the issue and marked it as a question

[mandragorn]

For now I think we shouldn't treat empty arrays as special. The act of marking the array as empty should trigger the same events as passing an array of values in case render is attached to the load-begin and load-complete events. If you are empting the array likely you want it to still re-render to render the empty content.

[svadla-vecna]

Not sure if I agree with the justification. I think load events are not the right events to rely on to re-render when emptying the array.

[kentmw]

I don't think re-rendering has much to do with what's in question. What we're all trying to determine is if when you call myCollection.fetch() it should call load-begin and load-complete if it's not tracking any ids or not.

My opinion is that the caller of fetch, shouldn't have to know if there are any ids being tracked. They should expect the load begin and load complete to occur when calling fetch, regardless of which ids or no ids are being tracked. Adding a special case for being silent during fetch seems like it would cause more confusion and errors than if we didn't.

It's a matter of whether you think load-begin and load-complete is part of the fetch api, or it's deeper than fetch and is more connected to ajax requests.

[svadla-vecna]

I agree that it depends on what you want the events to be. To me, the names of these events suggest that they are more tied to the ajax requests than the higher level fetch api. Also looking at the code, these events seem to be wrapping the ajax calls.