Partial failures in fetch

[idolizesc]

The fetch API has a failed state, but if you are using the Marty containers then there is no way to get the parts of the fetch that succeeded on a failed fetch (like there is with pending). This would be very useful if you are fetching several items for one page, passing them down as props, and you rely on part of the data even if some other part has failed.

Perhaps adding the fetch result to the callback here as an optional second parameter (besides just error) would provide a workable API?

I assume current limitation comes from the behavior of getFetchResult; it looks like as soon as there is any fetch result in error it does not add any results, and I'm also not sure if the container would wait for other fetches if one goes into error right away.

How hard would this be to add?

[taion]

I wonder if this isn't more a limitation of the API. An alternative way you could do this would be to, in Store#fetch, specify what you want the returned value to be during pending and failed states, and only invoke the container's pending or failed handler if not all fetches have those values specified.

[idolizesc]

@taion I don't follow what you mean here. Are you suggesting having some default results in the store?

To clarify: the Marty container components wrap 1-N fetch results with one single fetch result. The case where at least one of the fetches fails but at least one succeeds is difficult to handle, because there is no easy way to access the data returned by the successful fetch via the Marty container's failed() render function.

[taion]

Instead of just having

store.fetch({id, locally, remotely});

also support

store.fetch({id, locally, remotely, pending, failed});

And pass those through as "done" values to the container when in those states.

[idolizesc]

As an aside, the only possible workaround I can see with the current implementation is to use multiple Marty container components wrapping each other, which is very ugly and hard to reason about.

[idolizesc]

@taion I see. I do not think that is necessary though because the existing semantics of an individual fetch already work - I believe the container's wrapping of these fetches and the behavior of that is where the problem lies (not passing back partial results in a failure and not waiting for all results before deeming the entire thing failed).

[taion]

The problem is that in the general case, you might have some mix of done, pending, and failed fetches for a container with multiple fetches. I don't know that it's necessarily that expressive or manageable to give you the ability to sort all of those out on the container API itself - it might be better just to pass user-configurable values down to the wrapped component to robustly handle error cases.

That's effectively what the pending example already does - I'm just saying to put the logic to treat the pending value of user on the fetch call in getUser.

[taion]

In the example there you might have something like

// UserStore.js
class UserStore extends Store {
  getUser(id) {
    return this.fetch({
      id,
      locally: () => this.state.users[id],
      remotely: () => this.app.userQueries.getUser(id)
    });
  }
}

// UserComponent.js
createContainer(UserComponent, {
  listenTo: 'userStore',
  fetch: {
    user() {
      return this.app.userStore.getUser(this.props.id);
    }
  },
  pending() {
    return this.done({
      user: {}
    });
  }
});

but I'd rather express this as

// UserStore.js
class UserStore extends Store {
  getUser(id) {
    return this.fetch({
      id,
      locally: () => this.state.users[id],
      remotely: () => this.app.userQueries.getUser(id),
      pending: () => Object()
    });
  }
}

// UserComponent.js
createContainer(UserComponent, {
  listenTo: 'userStore',
  fetch: {
    user() {
      return this.app.userStore.getUser(this.props.id);
    }
  }
});

In the case where you want detailed per-fetch error/pending handling, it seems like it's just better just to shove that down to the component anyway.

The current behavior of just rendering an empty div and allowing to render some sort of message in the case of any failure is fine, but it seems like an anti-pattern to try to do much more than that in the container.

Ultimately you're most likely just trying to resolve the missing values in some appropriate way and pass them down to the wrapped component, so you might as well specify those fetch-by-fetch.

[idolize]

@taion I get what you are saying now. I think it definitely would be worth considering your option, although I do kind of like the ability to be explicit about failure/loading cases inside my views without having to clutter up my Store with that kind of information (or at least having that option). The current implementation also avoids having to return/check for sentinel values like empty objects (which is one of the things I don't really like about other implementations such as alt).

Maybe there could be several fetch implementations out there, given that the fetchResult type is exposed, which we could pick from depending on the specifics of what we are trying to do? Or even as separate npm modules.

[taion]

I think we want both.

By default, fetch should return PENDING and FAILED sentinel values that the container can handle using the current logic.

In the event that the user wants additional control, he can specify custom pending and failed values for the fetch, that will appear to the container as "done" values, and then handle these in the component's render function.

The former gives an easy way to get generic load handling logic. The latter allows extending to cover more complex cases without excessively cluttering up the container API. There's a little bit of redundancy, but I think the use cases differ enough that it'd be justifiable to have both.