XRPLF / xrpl.js

A JavaScript/TypeScript API for interacting with the XRP Ledger in Node.js and the browser
https://xrpl.org/
1.2k stars 512 forks source link

Improve method response types for other libraries #2370

Open tequdev opened 1 year ago

tequdev commented 1 year ago

Currently, AccountInfoResponse and other Response types contain a result field, which contains the original response of the method.

https://github.com/XRPLF/xrpl.js/blob/74de24cf75ff5a3d42ebe2fa17c9733812103881/packages/xrpl/src/models/methods/accountInfo.ts#L137-L178

Since other client libraries, such as xrpl-client, will only use the contents of the result field, it is preferable to define this part separately.

Otherwise, other libraries will need to define it as AccountInfoResponse['result'].

In xrpl.js, it may be better to use it in the form of MethodResponse<AccountInfoResponse>.

interface AccountInfoResponse {
     account_data: AccountRoot & { signer_lists?: SignerList[] } 
     account_flags?: AccountInfoAccountFlags 
     ...
}

interface MethodResponse<T extends any> extends BaseResponse {
    result: T
}

xrpl-client example

const client = new XrplClient('wss://xrpl.ws')

client.send({
  command: "account_info",
  account: 'rQQQrUdN1cLdNmxH4dHfKgmX5P4kf3ZrM'
}).then((res:any) => {
  console.log(res)
})

result

{
  account_data: {
    Account: 'rQQQrUdN1cLdNmxH4dHfKgmX5P4kf3ZrM',
    Balance: '185507840',
    BurnedNFTokens: 1,
    Domain: '746571752E646576',
    EmailHash: 'DCA87D72C399A1993247F6BE6E3C929E',
    Flags: 0,
    LedgerEntryType: 'AccountRoot',
    MintedNFTokens: 2,
    OwnerCount: 20,
    PreviousTxnID: '255A382D18A885647C6E56985155D5FAD98EBA7F5E91D1EE343D92E3EEE4EFBB',
    PreviousTxnLgrSeq: 80823196,
    Sequence: 61531627,
    index: '6A6192D86708D68A1678859E30F945F3179F55838CB9E56935F618FF103FF113',
    urlgravatar: 'http://www.gravatar.com/avatar/dca87d72c399a1993247f6be6e3c929e'
  },
  account_flags: {
    defaultRipple: false,
    depositAuth: false,
    disableMasterKey: false,
    disallowIncomingXRP: false,
    globalFreeze: false,
    noFreeze: false,
    passwordSpent: false,
    requireAuthorization: false,
    requireDestinationTag: false
  },
  ledger_current_index: 80959981,
  validated: false,
  _nodepref: 'nonfh'
}
tequdev commented 1 year ago

What I'm trying to say is that instead of providing type definitions for xrpl.js, it would be better to provide more generic type definitions.

Projects and other client libraries will be able to spend their time on more valuable things.

ckniffen commented 1 year ago

The types try to reflect what the websocket response is. xrpl-client simplifies it when it is successful and only returns the result part. Since the xrpl.org Explorer also uses xrpl-client this would be a very nice edition to break these portions of the response out.