safe-wallet-web
safe-wallet-web copied to clipboard
Refactor txDetails and remove the optional txDetails argument
While working on this PR: https://github.com/safe-global/safe-wallet-web/pull/4020 I refactored the code in TxDetails to "always" do a network request and fetch the txDetails. In the old code we were sometimes using the passed txDetails object: https://github.com/safe-global/safe-wallet-web/blob/c3431c7ad24cff843af773cc6c76af51436d644b/src/components/transactions/TxDetails/index.tsx#L167
Upon investigating it turned out that in 99% of the cases no component was passing any txDetails.
TxDetails prop was passed to ExpandableTransactionItem and it was used to determine if the Accordion should be expanded
Then I looked at who is calling ExpandableTransactionItem
And it turns out that it is only SingleTxGrid which passes a details to ExpandleTransactionitem. Who calls SingleTxGrid?
Well it is the SingleTx, where we fetch the txDetails. So the flow is like this
SingleTx -> SingleTxGrid-> ExpandableTransactionItem -> TxDetails.
In this flow we first fetch the details from the server in SingleTx, then again we fetch them in TxDetails. Now that we are using RTK Query it's no longer necessary to worry that we are going to make 2 requests to the server, since if we subscribe to txDetails in SingleTx, in the TxDetails component we will just fetch from the cache and won't do a new request.
So instead of passing txDetails to ExpandableTransactionItem I would go for "isExpanded" flag and scrap the txDetails. This way we won't have to pass props around.