-
Notifications
You must be signed in to change notification settings - Fork 212
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
refactor: allow gas estimation when unconnected #2231
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
) { | ||
return { | ||
status: 'unavailable', | ||
estimatedParentChainGasFees: undefined, |
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.
This no longer specifies undefined
for this case, just returning estimatedParentChainGasFees
as it was.
I checked the UI for native USDC withdrawal from Arb Sepolia to Sepolia and it looks fine
} | ||
|
||
if ( | ||
(gasEstimatesError && gasEstimatesError !== 'walletNotConnected') || |
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.
new condition to bypass 'walletNotConnected'
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.
encountered this error with import statement of react in import { useDebounce } from '@uidotdev/usehooks'
so we need to configure this and install ts-jest
and babel-jest
'gasEstimates' | ||
] as const) | ||
: null, | ||
[ |
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.
We will need to make a similar change in useOftV2FeeEstmates
hook
@@ -113,5 +109,9 @@ export function useGasEstimates({ | |||
} | |||
) | |||
|
|||
if (typeof walletAddress === 'undefined') { |
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.
Why would we need this error now that we're handling undefined
walletAddress?
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, if really needed, can this be moved inside the fetcher?
Part of #1452
also added test for
useGasSummary