update types to be more identifiable #508
Conversation
| never | ||
| >; | ||
| state?: Partial<GasFeeState>; | ||
| state?: GasFeeState; |
There was a problem hiding this comment.
State is now very much required to have all pieces to write it, but that doesn't always have to be true. if in the future one of the state keys is optional in all shapes, we can add it to the shapes with the ?: syntax so that partial spreads are supported, but this would still be GasFeeState versus Partial
| estimatedGasFeeTimeBounds: { persist: true, anonymous: false }, | ||
| gasEstimateType: { persist: true, anonymous: false }, | ||
| }; | ||
|
|
There was a problem hiding this comment.
So, just to confirm I understand: state will always have a estimatedGasFeeTimeBounds property, but it will be an empty object unless isEIP1559Compatible is true?
There was a problem hiding this comment.
Yep, all fields are always present, but their shape is determined by the value of gasEstimateType.
There was a problem hiding this comment.
and the fee-market value will only be set if we get a successful response from the EIP1559 gas estimate API.
|
|
||
| export type unknownString = 'unknown'; | ||
|
|
||
| export type FeeMarketEstimateType = 'fee-market'; |
There was a problem hiding this comment.
It might be helpful to add a brief comment here that explains that the FeeMarketEstimateType is the type used for EIP1559 compatible estimates
This makes it such that the state tree of the Gas Fee Controller is variable, but also known, and you can use
state.gasEstimateType === GAS_ESTIMATE_TYPES.FEE_MARKETand the like to put Typescript in the branch that has the gasEstimate shape typed. This removes the need to have helper methods to check for object structure to identify type.This also has a 'NONE' state that can be used to identify states in which we can rely on no values (still loading values)