safe-wallet-web icon indicating copy to clipboard operation
safe-wallet-web copied to clipboard

Refactor txDetails and remove the optional txDetails argument

Open compojoom opened this issue 1 year ago • 0 comments

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 grafik

Then I looked at who is calling ExpandableTransactionItem grafik

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.

compojoom avatar Aug 08 '24 13:08 compojoom