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

Add generics of TransactionType for TxResponse #2332

Merged
merged 3 commits into from
Jun 8, 2023

Conversation

tequdev
Copy link
Contributor

@tequdev tequdev commented Jun 6, 2023

High Level Overview of Change

Make Tx command response possible by specifying a transaction.

Type of Change

  • [ x] Documentation Updates

@justinr1234
Copy link
Collaborator

@develoQ can you show the intended use case that isn’t possible today and how this helps?

@tequdev
Copy link
Contributor Author

tequdev commented Jun 6, 2023

Additional commits have been made.

For example, it would be possible to provide a more detailed type for the response value of the submitAndWait method.

const paymentTx: Payment = {
...
}

const response = client.submitAndWait(paymentTx)

In this case, response will be resolved as TxResponse<Payment>, not TxResponse<Transaction>.

@ckniffen
Copy link
Collaborator

ckniffen commented Jun 6, 2023

Please update https:/XRPLF/xrpl.js/blob/main/packages/xrpl/HISTORY.md.

@mvadari
Copy link
Collaborator

mvadari commented Jun 6, 2023

I was a bit worried that this would make DX more complicated, since users would need to do TxResponse<txtype> instead of TxResponse when constructing a new TxResponse, but users never need to construct a TxResponse themselves so it's fine.

Does the request definition need to be adjusted with this to return TxResponse<Transaction> instead of just TxResponse? Seems like we don't actually run a TypeScript check in our CI to confirm.

@tequdev
Copy link
Contributor Author

tequdev commented Jun 7, 2023

I was a bit worried that this would make DX more complicated, since users would need to do TxResponse instead of TxResponse when constructing a new TxResponse, but users never need to construct a TxResponse themselves so it's fine.

Does the request definition need to be adjusted with this to return TxResponse instead of just TxResponse? Seems like we don't actually run a TypeScript check in our CI to confirm.

If TxResponse is specified, it is treated as TxResponse<Transaction>.

export interface TxResponse<T extends BaseTransaction = Transaction>
   extends BaseResponse {

The default value is set at = Transaction.

@justinr1234 justinr1234 merged commit 456eac0 into XRPLF:main Jun 8, 2023
@tequdev tequdev deleted the txresponse-generics branch June 8, 2023 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants