-
Notifications
You must be signed in to change notification settings - Fork 943
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
Improve tx page state management #866
Conversation
// In case a transaction reference is found from a previous | ||
// data load -> clear the state. Otherwise keep the non-null | ||
// values which may have been set from a previous page. | ||
const initialValues = txRef ? {} : pickBy(state, v => !!v); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Arrays are a problem here.
!![]
=> true
v => (Array.isArray(v) && !!v.length) || !!v
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Gnito I thought about that and reasoned that if the state has en empty array it can be passed as an initial value as in initialState
the value of messages
, which seems to be the only array, is []
.
However, what you proposed would be more explicit. Also empty objects are truthy too and for example there's the transactionRef
which is an object and should be overridden with the value from initialState
in case it is somehow has been set to {}
.
I'll add handling for empty arrays and objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, passing []
to
(Array.isArray(v) && !!v.length) || !!v
resolves to true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check the comment
Also override empty arrays and objects with values from initialState.
The state management in the
loadData
function inTransactionPage.duck
has been somewhat problematic and this PR aims to improve it.Now
loadData
checks if atransactionRef
is found from state. It is set when a tx fetch succeeds. If atransactionRef
is found it means the state holds data from a previous page load and the state is cleared. If the ref can not be found it means that the page has not been loaded or that thesetInitialValues
has been invoked for example with an initial message sending error information fromCheckoutPage
.