chore: add typings to snxjs #485
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/synthetixio/synthetix-exchange/au4xvmeau |
| @@ -1,16 +1,14 @@ | |||
| import throttle from 'lodash/throttle'; | |||
|
|
|||
| export type NetworkId = 1 | 3 | 4 | 42; | |||
There was a problem hiding this comment.
now its easier to match for networkId, its not just a "number", rather it must be this ^
There was a problem hiding this comment.
where the name points to the number
There was a problem hiding this comment.
maybe supported networks is just an enum?
There was a problem hiding this comment.
let's grab it from snxjs when its there... agree, can be an enum.
|
|
||
| export const defaultNetwork = { name: 'MAINNET', networkId: 1 }; | ||
| export const defaultNetwork: { name: string; networkId: NetworkId } = { | ||
| name: 'MAINNET', |
There was a problem hiding this comment.
should prob have an enum for the names here
There was a problem hiding this comment.
yeah all of that stuff should be in the snxjs lib
| gasPrice: number | null, | ||
| gasLimit: number | null, | ||
| ethPrice: number | null | ||
| ) => { | ||
| if (!gasPrice || !gasLimit || !ethPrice) return 0; |
There was a problem hiding this comment.
should prob say gasPrice == null || gasLimit == null || ethPrice == null since it is either going to be null or a number. What if gasPrice is 0 though?
There was a problem hiding this comment.
I actually didn't want to touch the actual logic since it worked, just added the types here. Falsy values should be fine here! 0,null,etc...all good - and if one of them is falsy, it should return 0. Because, the formula is
(gasPrice * ethPrice * gasLimit) / GWEI_UNIT
| if (!window.ethereum) return; | ||
| const listener = throttle(cb, 1000); | ||
| window.ethereum.on('networkChanged', listener); | ||
| } | ||
|
|
||
| export const isMainNet = (networkId) => networkId === SUPPORTED_NETWORKS.MAINNET; | ||
| export const isMainNet = (networkId: NetworkId) => networkId === 1; |
There was a problem hiding this comment.
would be good to have Networks as an enum to make equality checks easier
There was a problem hiding this comment.
happy to replace this when we switch to the new library (which has the networks, etc)
These are some of the changes I ported from #448
Added stronger typing around
NetworkId, and initial typings for SynthetixJS contracts.