Skip to content
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

feature: review arguments, moving some to objects, and ensuring defaults are respected #232

Open
elboletaire opened this issue Aug 1, 2023 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@elboletaire
Copy link
Member

elboletaire commented Aug 1, 2023

Describe the feature

This is a followup of https://github.com/vocdoni/vocdoni-sdk/pull/226/files#r1277794806. It's created as an enhancement because it's not an issue right now (it will be an issue after anonymous is merged, but only for some methods).

If I'm not wrong, @marcvelmer idea is to rewrite some of the function arguments to be objects, but also ensuring the defaults are properly handled (not like in the triggering commented code).

Edit: also with this issue, it would be awesome to have all the function parameters types defined, so developers can import them later, rather than redefining the type entirely like it happens now.

Motivation

Some arguments would be better grouped in a single object, but their defaults management would be a bit different, since you have to merge that object with a defaults, rather than using defaults in the argument.

Proposal

To review every method, convert to object those params that make sense to be objects, and ensuring the defaults are properly handled.

Components affected

Mainly client.ts

@elboletaire elboletaire added the enhancement New feature or request label Aug 1, 2023
@elboletaire
Copy link
Member Author

elboletaire commented Nov 15, 2023

Also, for this feature, I think it's important to create all the types of all the arguments required by functions. So when developers use a function like createAccount, they do not need to redefine the type of the args passed (extracted from ui-components):

  /**
   * Creates an account.
   *
   * @returns {Promise<AccountData>}
   */
  // having the type defined, this would not be required
  const createAccount = (options: {
    account: Account
    faucetPackage?: string
    signedSikPayload?: string
    password?: string
  }) => {
    if (state.loading.account) return

    actions.fetchAccount()
    return state.client.createAccount(options).then(actions.setAccount).catch(actions.errorAccount)
  }

Trying to not define a type right now is not an option (and is less option to use any...):
imatge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants