feat: combine mint token function, remove - as it was confusing #13
Conversation
|
|
||
| if (!markets) return null; | ||
|
|
||
| const [mintAmount, setMintAmount] = useState<number>(); | ||
| const fromBase = markets.baseVault.underlyingTokenMint.equals(token.publicKey); | ||
| const [token, setToken] = useState<string>('meta'); |
There was a problem hiding this comment.
we could use selectedTokenName and token, might be a bit more clear
There was a problem hiding this comment.
Also, what's the point of using two variables instead of only storing a token and using its name attributes when needed?
There was a problem hiding this comment.
Totally, I think it was because I couldn't get typing to work and I revert back to "how can I get this to work" vs "this is the right way to do it" but yep will circle back on this.
| }, [token]); | ||
|
|
||
| // TODO: Fetch the two tokens | ||
| const fromBase = markets.baseVault.underlyingTokenMint.equals(_token.publicKey); |
There was a problem hiding this comment.
we could memoize fromFail, fromPass, vaultFail & vaultPass, then set fromBase and vault on _token change. Do we want to do that in that PR ?
There was a problem hiding this comment.
Anything that reduces overhead is best effort. So moving towards that is critical.
|
Looking for some feedback where and what to put in useMemo as when I toss the balances in there it fails. |
There may be a few errors with type checking and assuming tokens which need review. However I believe this simplifies a component so that we don't have a ton of information which isn't always relevant to the user.