Initial Account Kit #7
Conversation
dafuga
force-pushed
the
adding-account-class
branch
from
79f5b6e
to
f9ebfbb
Compare
dafuga
force-pushed
the
adding-account-class
branch
from
f9ebfbb
to
e64a702
Compare
|
@aaroncox @jnordberg I removed a lot of the methods and addressed some of your feedback. Let me know if you guys think that we can merge this. |
|
I'd merge it since ultimately it doesn't matter, but if we merge now we'd lose some of the comments that are still unaddressed. I'd say let's resolved those first. I can take a shot at it if you'd like and add some commits to this branch? |
… loading all the data
02-getting-account-data/index.js
Outdated
| const account: Account = Account.from('teamgreymass', Chains.EOS) | ||
|
|
||
| // Retrieves data from the APIs (v1/chain/get_account) | ||
| await account.loadData() |
There was a problem hiding this comment.
Continuing from the other thread - the above takes the Account and makes it a representation of an account, and then has a loadData method to make the API call and populate the data.
I'm not sure if we want to use this pattern. It has it's pros/cons.
- Pro: Allows you to just call
loadDatato load/refresh theAccountdata. Could have a timestamp to indicate some form of staleness. - Con: Always feels weird to just await an async function like this to me.
There was a problem hiding this comment.
Actually lets continue this discussion in a new PR. I'm going to merge this.
If anyone has suggestions on how this best changes, open a PR!x
| const account: Account = Account.from('teamgreymass', Chains.EOS) | ||
|
|
||
| // Retrieves data from the APIs (v1/chain/get_account) | ||
| await account.loadData() |
There was a problem hiding this comment.
I personally would prefer not having this extra step where you need to load the data manually.
There was a problem hiding this comment.
I'm not sure its great either - but this was the latter option Johan has mentioned.
issue #6