Skip to content

Added generic type determination based on binary value #3045

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.

Sign up for GitHub

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

Open
tequdev wants to merge 5 commits into
base: main
Choose a base branch
from
+288 −177
Open

Added generic type determination based on binary value #3045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6fd0579
Added generic type determination based on binary value
tequdev Jul 17, 2025
c8e7fdd
fix lint error
tequdev Jul 17, 2025
9807984
fix TxResponse
tequdev Jul 17, 2025
3e5b325
fix
tequdev Jul 17, 2025
5ae4f5b
remove empty line
tequdev Jul 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 23 additions & 14 deletions packages/xrpl/src/client/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import {
LedgerIndex,
Balance,
DEFAULT_API_VERSION,
RIPPLED_API_V2,
RIPPLED_API_V1,
} from '../models/common'
import {
Request,
Expand Down Expand Up @@ -121,26 +123,29 @@ export interface ClientOptions extends ConnectionUserOptions {
}

// Make sure to update both this and `RequestNextPageReturnMap` at the same time
type RequestNextPageType =
type RequestNextPageType<Binary extends boolean = false> =
| AccountChannelsRequest
| AccountLinesRequest
| AccountObjectsRequest
| AccountOffersRequest
| AccountTxRequest
| LedgerDataRequest
| AccountTxRequest<Binary>
| LedgerDataRequest<Binary>

type RequestNextPageReturnMap<T> = T extends AccountChannelsRequest
type RequestNextPageReturnMap<
T extends RequestNextPageType<Binary>,
Binary extends boolean = false,
> = T extends AccountChannelsRequest
? AccountChannelsResponse
: T extends AccountLinesRequest
? AccountLinesResponse
: T extends AccountObjectsRequest
? AccountObjectsResponse
: T extends AccountOffersRequest
? AccountOffersResponse
: T extends AccountTxRequest
? AccountTxResponse
: T extends LedgerDataRequest
? LedgerDataResponse
: T extends AccountTxRequest<Binary>
? AccountTxResponse<Binary>
: T extends LedgerDataRequest<Binary>
? LedgerDataResponse<Binary>
: never

/**
Expand Down Expand Up @@ -338,9 +343,12 @@ class Client extends EventEmitter<EventTypes> {
* ```
*/
public async request<
R extends Request,
V extends APIVersion = typeof DEFAULT_API_VERSION,
T = RequestResponseMap<R, V>,
R extends Request<B>,
V extends APIVersion = R['api_version'] extends typeof RIPPLED_API_V1
? typeof RIPPLED_API_V1
: typeof RIPPLED_API_V2,
B extends boolean = R['binary'] extends true ? true : false,
T = RequestResponseMap<R, V, B>,
Comment on lines +346 to +351
Copy link
Contributor

@coderabbitai coderabbitai bot Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Review the generic constraint order in the request method.

The generic signature has a potential issue where R extends Request<B> uses B before it's defined in the parameter list. This might cause TypeScript compilation issues.

Consider reordering the generic parameters or using a different constraint pattern:

-public async request<
-  R extends Request<B>,
-  V extends APIVersion = R['api_version'] extends typeof RIPPLED_API_V1
-    ? typeof RIPPLED_API_V1
-    : typeof RIPPLED_API_V2,
-  B extends boolean = R['binary'] extends true ? true : false,
-  T = RequestResponseMap<R, V, B>,
->(req: R): Promise<T> {
+public async request<
+  R extends Request,
+  V extends APIVersion = R['api_version'] extends typeof RIPPLED_API_V1
+    ? typeof RIPPLED_API_V1
+    : typeof RIPPLED_API_V2,
+  B extends boolean = R['binary'] extends true ? true : false,
+  T = RequestResponseMap<R, V, B>,
+>(req: R): Promise<T> {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
R extends Request<B>,
V extends APIVersion = R['api_version'] extends typeof RIPPLED_API_V1
? typeof RIPPLED_API_V1
: typeof RIPPLED_API_V2,
B extends boolean = R['binary'] extends true ? true : false,
T = RequestResponseMap<R, V, B>,
public async request<
R extends Request,
V extends APIVersion = R['api_version'] extends typeof RIPPLED_API_V1
? typeof RIPPLED_API_V1
: typeof RIPPLED_API_V2,
B extends boolean = R['binary'] extends true ? true : false,
T = RequestResponseMap<R, V, B>,
>(req: R): Promise<T> {
// …
}
🤖 Prompt for AI Agents 
In packages/xrpl/src/client/index.ts around lines 346 to 351, the generic
parameter B is used in the constraint of R before B is defined, which can cause
TypeScript compilation errors. To fix this, reorder the generic parameters so
that B is declared before R, or refactor the constraints to avoid referencing B
before its declaration. Ensure that all generic parameters are declared before
they are used in constraints.

>(req: R): Promise<T> {
const request = {
...req,
Expand Down Expand Up @@ -383,9 +391,10 @@ class Client extends EventEmitter<EventTypes> {
* ```
*/
public async requestNextPage<
T extends RequestNextPageType,
U extends RequestNextPageReturnMap<T>,
>(req: T, resp: U): Promise<RequestNextPageReturnMap<T>> {
T extends RequestNextPageType<B>,
U extends RequestNextPageReturnMap<T, B>,
B extends T['binary'] extends true ? true : false,
>(req: T, resp: U): Promise<RequestNextPageReturnMap<T, B>> {
if (!resp.result.marker) {
return Promise.reject(
new NotFoundError('response does not have a next page'),
Expand Down
84 changes: 55 additions & 29 deletions packages/xrpl/src/models/methods/accountTx.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ import { BaseRequest, BaseResponse, LookupByLedgerRequest } from './baseMethod'
*
* @category Requests
*/
export interface AccountTxRequest extends BaseRequest, LookupByLedgerRequest {
export interface AccountTxRequest<Binary extends boolean = false>
extends BaseRequest,
LookupByLedgerRequest {
command: 'account_tx'
/** A unique identifier for the account, most commonly the account's address. */
account: string
Expand All @@ -36,7 +38,7 @@ export interface AccountTxRequest extends BaseRequest, LookupByLedgerRequest {
* If true, return transactions as hex strings instead of JSON. The default is
* false.
*/
binary?: boolean
binary?: Binary
/**
* If true, returns values indexed with the oldest ledger first. Otherwise,
* the results are indexed with the newest ledger first.
Expand All @@ -55,40 +57,61 @@ export interface AccountTxRequest extends BaseRequest, LookupByLedgerRequest {
marker?: unknown
}

export interface AccountTxTransaction<
Version extends APIVersion = typeof DEFAULT_API_VERSION,
> {
/** The ledger index of the ledger version that included this transaction. */
ledger_index: number
/**
* If binary is True, then this is a hex string of the transaction metadata.
* Otherwise, the transaction metadata is included in JSON format.
*/
meta: string | TransactionMetadata
/** JSON object defining the transaction. */
tx_json?: Version extends typeof RIPPLED_API_V2
? Transaction & ResponseOnlyTxInfo
: never
/** JSON object defining the transaction in rippled API v1. */
tx?: Version extends typeof RIPPLED_API_V1
? Transaction & ResponseOnlyTxInfo
: never
/** The hash of the transaction. */
hash?: Version extends typeof RIPPLED_API_V2 ? string : never
/** Unique hashed String representing the transaction. */
tx_blob?: string
interface AccountTxTransactionBase {
/**
* Whether or not the transaction is included in a validated ledger. Any
* transaction not yet in a validated ledger is subject to change.
*/
validated: boolean
}

export type AccountTxTransaction<
Version extends APIVersion = typeof DEFAULT_API_VERSION,
Binary extends boolean = false,
> = AccountTxTransactionBase &
(Version extends typeof RIPPLED_API_V2
? Binary extends true
? {
/** Unique hashed String representing the transaction. */
tx_blob: string
/** hex string of the transaction metadata. */
meta_blob: string
/** The ledger index of the ledger version that included this transaction. */
ledger_index: number
}
: {
/** JSON object defining the transaction. */
tx_json: Transaction & ResponseOnlyTxInfo
/** The transaction metadata in JSON format. */
meta: TransactionMetadata
/** The ledger index of the ledger version that included this transaction. */
ledger_index: number
ledger_hash: string
hash: string
close_time_iso: string
}
: Binary extends true
? {
/** Unique hashed String representing the transaction. */
tx_blob: string
/** Hex string of the transaction metadata. */
meta: string
/** The ledger index of the ledger version that included this transaction. */
ledger_index: number
}
: {
/** JSON object defining the transaction in rippled API v1. */
tx: Transaction & ResponseOnlyTxInfo
/** The transaction metadata in JSON format. */
meta: TransactionMetadata
})

/**
* Base interface for account transaction responses.
*/
interface AccountTxResponseBase<
Version extends APIVersion = typeof DEFAULT_API_VERSION,
Binary extends boolean = false,
> extends BaseResponse {
result: {
/** Unique Address identifying the related account. */
Expand All @@ -114,7 +137,7 @@ interface AccountTxResponseBase<
* Array of transactions matching the request's criteria, as explained
* below.
*/
transactions: Array<AccountTxTransaction<Version>>
transactions: Array<AccountTxTransaction<Version, Binary>>
/**
* If included and set to true, the information in this response comes from
* a validated ledger version. Otherwise, the information is subject to
Expand All @@ -129,14 +152,16 @@ interface AccountTxResponseBase<
*
* @category Responses
*/
export type AccountTxResponse = AccountTxResponseBase
export type AccountTxResponse<Binary extends boolean = false> =
AccountTxResponseBase<typeof DEFAULT_API_VERSION, Binary>

/**
* Expected response from an {@link AccountTxRequest} with `api_version` set to 1.
*
* @category ResponsesV1
*/
export type AccountTxV1Response = AccountTxResponseBase<typeof RIPPLED_API_V1>
export type AccountTxV1Response<Binary extends boolean = false> =
AccountTxResponseBase<typeof RIPPLED_API_V1, Binary>

/**
* Type to map between the API version and the response type.
Expand All @@ -145,6 +170,7 @@ export type AccountTxV1Response = AccountTxResponseBase<typeof RIPPLED_API_V1>
*/
export type AccountTxVersionResponseMap<
Version extends APIVersion = typeof DEFAULT_API_VERSION,
Binary extends boolean = false,
> = Version extends typeof RIPPLED_API_V1
? AccountTxV1Response
: AccountTxResponse
? AccountTxV1Response<Binary>
: AccountTxResponse<Binary>
8 changes: 4 additions & 4 deletions packages/xrpl/src/models/methods/baseMethod.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { LedgerIndex } from '../common'
import { LedgerIndex, APIVersion } from '../common'

import type { Request } from '.'

Expand All @@ -13,7 +13,7 @@ export interface BaseRequest {
/** The name of the API method. */
command: string
/** The API version to use. If omitted, use version 1. */
api_version?: number
api_version?: APIVersion
}

export interface LookupByLedgerRequest {
Expand All @@ -37,7 +37,7 @@ export interface BaseResponse {
warning?: 'load'
warnings?: ResponseWarning[]
forwarded?: boolean
api_version?: number
api_version?: APIVersion
}

/**
Expand All @@ -55,5 +55,5 @@ export interface ErrorResponse {
error_message?: string
error_exception?: string
request: Request
api_version?: number
api_version?: APIVersion
}
46 changes: 24 additions & 22 deletions packages/xrpl/src/models/methods/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-disable no-inline-comments -- Necessary for important note */
/* eslint-disable max-lines -- There is a lot to export */
import type { APIVersion, DEFAULT_API_VERSION } from '../common'
import { LedgerEntry } from '../ledger'

import {
AccountChannelsRequest,
Expand Down Expand Up @@ -193,7 +194,7 @@ import { VaultInfoRequest, VaultInfoResponse } from './vaultInfo'
/**
* @category Requests
*/
type Request =
type Request<Binary extends boolean = false> =
// account methods
| AccountChannelsRequest
| AccountCurrenciesRequest
Expand All @@ -202,21 +203,21 @@ type Request =
| AccountNFTsRequest
| AccountObjectsRequest
| AccountOffersRequest
| AccountTxRequest
| AccountTxRequest<Binary>
| GatewayBalancesRequest
| NoRippleCheckRequest
// ledger methods
| LedgerRequest
| LedgerRequest<Binary>
| LedgerClosedRequest
| LedgerCurrentRequest
| LedgerDataRequest
| LedgerEntryRequest
| LedgerDataRequest<Binary>
| LedgerEntryRequest<Binary>
// transaction methods
| SimulateRequest
| SubmitRequest
| SubmitMultisignedRequest
| TransactionEntryRequest
| TxRequest
| TxRequest<Binary>
// path and order book methods
| BookOffersRequest
| DepositAuthorizedRequest
Expand All @@ -242,7 +243,7 @@ type Request =
| NFTSellOffersRequest
// clio only methods
| NFTInfoRequest
| NFTHistoryRequest
| NFTHistoryRequest<Binary>
| NFTsByIssuerRequest
// AMM methods
| AMMInfoRequest
Expand Down Expand Up @@ -303,7 +304,7 @@ type Response<Version extends APIVersion = typeof DEFAULT_API_VERSION> =
| NFTSellOffersResponse
// clio only methods
| NFTInfoResponse
| NFTHistoryResponse
| NFTHistoryResponse<Version>
| NFTsByIssuerResponse
// AMM methods
| AMMInfoResponse
Expand All @@ -315,6 +316,7 @@ type Response<Version extends APIVersion = typeof DEFAULT_API_VERSION> =
export type RequestResponseMap<
T,
Version extends APIVersion = typeof DEFAULT_API_VERSION,
Binary extends boolean = false,
> = T extends AccountChannelsRequest
? AccountChannelsResponse
: T extends AccountCurrenciesRequest
Expand All @@ -329,8 +331,8 @@ export type RequestResponseMap<
? AccountObjectsResponse
: T extends AccountOffersRequest
? AccountOffersResponse
: T extends AccountTxRequest
? AccountTxVersionResponseMap<Version>
: T extends AccountTxRequest<Binary>
? AccountTxVersionResponseMap<Version, Binary>
: T extends AMMInfoRequest
? AMMInfoResponse
: T extends GatewayBalancesRequest
Expand Down Expand Up @@ -397,22 +399,22 @@ export type RequestResponseMap<
// LedgerResponseExpanded.
T extends LedgerRequestExpandedTransactionsBinary
? LedgerVersionResponseMap<Version>
: T extends LedgerRequestExpandedAccountsAndTransactions
: T extends LedgerRequestExpandedAccountsAndTransactions<Binary>
? LedgerResponseExpanded<Version>
: T extends LedgerRequestExpandedTransactionsOnly
: T extends LedgerRequestExpandedTransactionsOnly<Binary>
? LedgerResponseExpanded<Version>
: T extends LedgerRequestExpandedAccountsOnly
: T extends LedgerRequestExpandedAccountsOnly<Binary>
? LedgerResponseExpanded<Version>
: T extends LedgerRequest
: T extends LedgerRequest<Binary>
? LedgerVersionResponseMap<Version>
: T extends LedgerClosedRequest
? LedgerClosedResponse
: T extends LedgerCurrentRequest
? LedgerCurrentResponse
: T extends LedgerDataRequest
? LedgerDataResponse
: T extends LedgerEntryRequest
? LedgerEntryResponse
: T extends LedgerDataRequest<Binary>
? LedgerDataResponse<Binary>
: T extends LedgerEntryRequest<Binary>
? LedgerEntryResponse<LedgerEntry, Binary>
: T extends SimulateBinaryRequest
? SimulateBinaryResponse
: T extends SimulateJsonRequest
Expand All @@ -425,8 +427,8 @@ export type RequestResponseMap<
? SubmitMultisignedVersionResponseMap<Version>
: T extends TransactionEntryRequest
? TransactionEntryResponse
: T extends TxRequest
? TxVersionResponseMap<Version>
: T extends TxRequest<Binary>
? TxVersionResponseMap<Version, Binary>
: T extends BookOffersRequest
? BookOffersResponse
: T extends DepositAuthorizedRequest
Expand Down Expand Up @@ -467,8 +469,8 @@ export type RequestResponseMap<
? NFTInfoResponse
: T extends NFTsByIssuerRequest
? NFTsByIssuerResponse
: T extends NFTHistoryRequest
? NFTHistoryResponse
: T extends NFTHistoryRequest<Binary>
? NFTHistoryResponse<Version, Binary>
: T extends VaultInfoRequest
? VaultInfoResponse
: Response<Version>
Expand Down
Loading
Loading