Components resilience about blockchain-wallet-v4-frontend HOT 9 CLOSED

Discuss !

from blockchain-wallet-v4-frontend.

https://github.com/reactjs/redux/blob/master/docs/advanced/AsyncActions.md

from blockchain-wallet-v4-frontend.

Currently working on a prototype based on Jaume article. Main idea will be to save timestamp, isFetching and isSuccessful, so each component can react to those flags if the data is currently fetching...

from blockchain-wallet-v4-frontend.

Definitely a good idea to figure this out, thanks for starting the discussion @Lyncee59!

A few questions I have right away are:

• What would the timestamp data be used for? Making sure successive requests are spaced too closely?
• How would we distinguish between a "not asked" and a "failed" state? (As both would have !isFetching && !isSuccessful == true).
• If a request fails, do we store the error in the state? Is a bool enough information?
• Can this model handle the "refresh" case? (Data exists, we want to re-fetch).

This reminds me of this article, which describes a pattern we use in Elm to solve these kinds of issues, we may be able pull some ideas from there. I really like how it avoids boolean logic completely (which tends to be error-prone).

As far as how the loading should be triggered, I like how in your Solution 1, components are responsible for requesting the data they require. To make sure data isn't loaded multiple times, we could buffer data load requests in redux-saga (after a request to load some data has gone through, stop all other requests for that same resource until the first has completed).

Providing data to the components could be an issue as well, I can see all of these boolean states leading to some nasty if/then/else logic. It might be worthwhile to come up with some helper functions to prevent that.

// Take 2 resource state objects (ie. { isFetching: Bool, isSuccessful: Bool, ...}),
// return an object that is the union of the two (as if it was a single request)
combineResources(loadingDataA, succeededDataB) // => loadingDataAB
combineResources(succeededDataA, succeededDataB) // => succeededDataAB

from blockchain-wallet-v4-frontend.

What would the timestamp data be used for? Making sure successive requests are spaced too closely?

Let's say we load the transaction page, then switch to shapeshift, then back to transactions page. We can test against the timestamp to decide to force re-fetching the transactions or not, based on a time interval.

How would we distinguish between a "not asked" and a "failed" state? (As both would have !isFetching && !isSuccessful == true).

Do we need a 'not asked' state ? I am not that sure right now. Some components will require pre-fetching data (in the componentWillMount), some won't. In both cases, I think 'not asked' is not going to be useful ? Do you have a specific example in mind ? Otherwise maybe we should refresh the timestamp value for both 'fetch success' and 'fetch error' events, so if the timestamp is not there and both flags are false, we can consider nothing happened :)

If a request fails, do we store the error in the state? Is a bool enough information?

If a request fail, !isFetching && !isSuccessful === true, in this case, the component will render a 'disabled' state type of layout. We could have a user friendly hardcoded message, whatever the reason of failure, that could be override by the wallet-options messages. Also note that some components may simply be displayed but unusable: 'Send Bitcoin' button could simply be unclickable.

• Enabled: isFetching: false, isSuccessful: true
• Enabled with refresh: isFetching: false, isSuccessful: true, new Date() - timestamp > interval
• Disabled: isFetching: false, isSuccessful: false

Can this model handle the "refresh" case? (Data exists, we want to re-fetch).

As I was suggesting above, by adding a rule against the timestamp, we can decide to refresh based on a time interval, and this time interval could be 0 meaning refresh all the time !

from blockchain-wallet-v4-frontend.

The timestamp / interval logic seems a little arbitrary, are you sure we need that? The websocket should handle updates in 99% of cases, in the other 1% the user will manually press the refresh button.

I'm not sure if we would ever need to actually render a "not asked" state, but without it the state model seems incomplete. What do you call the state that comes before the "loading" state?

How do you think we can distinguish between a request failing (db down, service crash, etc) and a feature being "disabled" through wallet options? In the case that it's disabled through wallet options, we should probably prevent the request in the first place.

from blockchain-wallet-v4-frontend.

The timestamp / interval logic seems a little arbitrary, are you sure we need that? The websocket should handle updates in 99% of cases, in the other 1% the user will manually press the refresh button.

Well, there won't be a lot of cases where we want to re-fetch automatically. Most of time, in the componentWillMount, we will just fetch, even if the data is there. So maybe not very useful, maybe not bad if it is there ?

I am putting in place a generic set of actions:
FETCH_DATA, FETCH_DATA_SUCCESS, FETCH_DATA_ERROR that will reduce any ajax call data, so its not a lot of boilerplate to handle this timestamp anyway.

I'm not sure if we would ever need to actually render a "not asked" state, but without it the state model seems incomplete. What do you call the state that comes before the "loading" state?

Changed my point of view after chatting with Jaume and reading your article, I am now thinking that we need a state string instead of isFetching and isSuccessful, for the 4 states you described including 'not asked'. Its gonna be less selectors and more flexibility if we find a new state one day.

How do you think we can distinguish between a request failing (db down, service crash, etc) and a feature being "disabled" through wallet options? In the case that it's disabled through wallet options, we should probably prevent the request in the first place.

Wallet-options are going to kick in before we fetch any data. Each component will have selectors pointing to its own wallet-options setup if available.

• A component disabled in wallet-options will not fetch anything, its just going to render the 'disabled' template.

• A component enabled in the wallet options will try fetching, and the 'state' (or combination of 'state' of these calls if there are multiple data sources) will just toggle the same 'disabled' template.

Not sure that we want to distinguish those 2 scenarios which lead to a disabled components.
'Hey sorry dude, we decided to disable this feature', or
'Hey this feature was supposed to work but its not, AHAH. (Nelson@Simpsons)'

from blockchain-wallet-v4-frontend.

Well, there won't be a lot of cases where we want to re-fetch automatically. Most of time, in the componentWillMount, we will just fetch, even if the data is there. So maybe not very useful, maybe not bad if it is there ?

See https://martinfowler.com/bliki/Yagni.html 😉

Changed my point of view after chatting with Jaume and reading your article, I am now thinking that we need a state string instead of isFetching and isSuccessful, for the 4 states you described including 'not asked'. Its gonna be less selectors and more flexibility if we find a new state one day.

Nice, sounds good.

Wallet-options are going to kick in before we fetch any data. Each component will have selectors pointing to its own wallet-options setup if available.

Works for me!

from blockchain-wallet-v4-frontend.

See https://martinfowler.com/bliki/Yagni.html 😉

I know, I am not a YAGNI type of dude, this is a dream wallet still in my mind ! ahah

from blockchain-wallet-v4-frontend.