-
Notifications
You must be signed in to change notification settings - Fork 523
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 apiVersion
option to Client constructor
#2877
base: main
Are you sure you want to change the base?
Changes from all commits
b426c97
f0c88e4
5a63291
44f078c
d31a3ad
34493ab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,7 +28,6 @@ import { | |
AccountOffersRequest, | ||
AccountOffersResponse, | ||
AccountTxRequest, | ||
AccountTxResponse, | ||
// ledger methods | ||
LedgerDataRequest, | ||
LedgerDataResponse, | ||
|
@@ -40,7 +39,9 @@ import type { | |
MarkerRequest, | ||
MarkerResponse, | ||
SubmitResponse, | ||
BaseRequest, | ||
} from '../models/methods' | ||
import { AccountTxResponseBase } from '../models/methods/accountTx' | ||
import type { BookOffer, BookOfferCurrency } from '../models/methods/bookOffers' | ||
import type { | ||
EventTypes, | ||
|
@@ -92,7 +93,9 @@ import { | |
handleStreamPartialPayment, | ||
} from './partialPayment' | ||
|
||
export interface ClientOptions extends ConnectionUserOptions { | ||
export interface ClientOptions< | ||
ClientAPIVersion extends APIVersion = typeof DEFAULT_API_VERSION, | ||
> extends ConnectionUserOptions { | ||
/** | ||
* Multiplication factor to multiply estimated fee by to provide a cushion in case the | ||
* required fee rises during submission of a transaction. Defaults to 1.2. | ||
|
@@ -111,6 +114,12 @@ export interface ClientOptions extends ConnectionUserOptions { | |
* Duration to wait for a request to timeout. | ||
*/ | ||
timeout?: number | ||
/** | ||
* API Version to use for requests. | ||
* | ||
* @default DEFAULT_API_VERSION | ||
*/ | ||
apiVersion?: ClientAPIVersion | ||
} | ||
|
||
// Make sure to update both this and `RequestNextPageReturnMap` at the same time | ||
|
@@ -122,7 +131,10 @@ type RequestNextPageType = | |
| AccountTxRequest | ||
| LedgerDataRequest | ||
|
||
type RequestNextPageReturnMap<T> = T extends AccountChannelsRequest | ||
type RequestNextPageReturnMap< | ||
T extends BaseRequest, | ||
V extends APIVersion = typeof DEFAULT_API_VERSION, | ||
> = T extends AccountChannelsRequest | ||
? AccountChannelsResponse | ||
: T extends AccountLinesRequest | ||
? AccountLinesResponse | ||
|
@@ -131,7 +143,7 @@ type RequestNextPageReturnMap<T> = T extends AccountChannelsRequest | |
: T extends AccountOffersRequest | ||
? AccountOffersResponse | ||
: T extends AccountTxRequest | ||
? AccountTxResponse | ||
? AccountTxResponseBase<V> | ||
: T extends LedgerDataRequest | ||
? LedgerDataResponse | ||
: never | ||
|
@@ -184,7 +196,9 @@ const NORMAL_DISCONNECT_CODE = 1000 | |
* | ||
* @category Clients | ||
*/ | ||
class Client extends EventEmitter<EventTypes> { | ||
class Client< | ||
ClientAPIVersion extends APIVersion = typeof DEFAULT_API_VERSION, | ||
> extends EventEmitter<EventTypes> { | ||
/* | ||
* Underlying connection to rippled. | ||
*/ | ||
|
@@ -222,7 +236,7 @@ class Client extends EventEmitter<EventTypes> { | |
* API Version used by the server this client is connected to | ||
* | ||
*/ | ||
public apiVersion: APIVersion = DEFAULT_API_VERSION | ||
public apiVersion: APIVersion | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Type of Currently, Apply this diff to update the type: -public apiVersion: APIVersion
+public apiVersion: ClientAPIVersion
|
||
|
||
/** | ||
* Creates a new Client with a websocket connection to a rippled server. | ||
|
@@ -238,7 +252,10 @@ class Client extends EventEmitter<EventTypes> { | |
* ``` | ||
*/ | ||
/* eslint-disable max-lines-per-function -- the constructor requires more lines to implement the logic */ | ||
public constructor(server: string, options: ClientOptions = {}) { | ||
public constructor( | ||
server: string, | ||
options: ClientOptions<ClientAPIVersion> = {}, | ||
) { | ||
super() | ||
if (typeof server !== 'string' || !/wss?(?:\+unix)?:\/\//u.exec(server)) { | ||
throw new ValidationError( | ||
|
@@ -249,6 +266,8 @@ class Client extends EventEmitter<EventTypes> { | |
this.feeCushion = options.feeCushion ?? DEFAULT_FEE_CUSHION | ||
this.maxFeeXRP = options.maxFeeXRP ?? DEFAULT_MAX_FEE_XRP | ||
|
||
this.apiVersion = options.apiVersion ?? DEFAULT_API_VERSION | ||
|
||
this.connection = new Connection(server, options) | ||
|
||
this.connection.on('error', (errorCode, errorMessage, data) => { | ||
|
@@ -332,7 +351,7 @@ class Client extends EventEmitter<EventTypes> { | |
*/ | ||
public async request< | ||
R extends Request, | ||
V extends APIVersion = typeof DEFAULT_API_VERSION, | ||
V extends APIVersion = ClientAPIVersion, | ||
T = RequestResponseMap<R, V>, | ||
>(req: R): Promise<T> { | ||
const request = { | ||
|
@@ -377,8 +396,8 @@ class Client extends EventEmitter<EventTypes> { | |
*/ | ||
public async requestNextPage< | ||
T extends RequestNextPageType, | ||
U extends RequestNextPageReturnMap<T>, | ||
>(req: T, resp: U): Promise<RequestNextPageReturnMap<T>> { | ||
U extends RequestNextPageReturnMap<T, ClientAPIVersion>, | ||
>(req: T, resp: U): Promise<U> { | ||
if (!resp.result.marker) { | ||
return Promise.reject( | ||
new NotFoundError('response does not have a next page'), | ||
|
@@ -417,7 +436,10 @@ class Client extends EventEmitter<EventTypes> { | |
public on< | ||
T extends EventTypes, | ||
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- needs to be any for overload | ||
U extends (...args: any[]) => void = OnEventToListenerMap<T>, | ||
U extends (...args: any[]) => void = OnEventToListenerMap< | ||
T, | ||
ClientAPIVersion | ||
>, | ||
>(eventName: T, listener: U): this { | ||
return super.on(eventName, listener) | ||
} | ||
|
@@ -455,7 +477,7 @@ class Client extends EventEmitter<EventTypes> { | |
|
||
public async requestAll< | ||
T extends MarkerRequest, | ||
U = RequestAllResponseMap<T, APIVersion>, | ||
U = RequestAllResponseMap<T, ClientAPIVersion>, | ||
>(request: T, collect?: string): Promise<U[]> { | ||
/* | ||
* The data under collection is keyed based on the command. Fail if command | ||
|
@@ -483,7 +505,8 @@ class Client extends EventEmitter<EventTypes> { | |
// eslint-disable-next-line no-await-in-loop -- Necessary for this, it really has to wait | ||
const singleResponse = await this.connection.request(repeatProps) | ||
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Should be true | ||
const singleResult = (singleResponse as MarkerResponse<APIVersion>).result | ||
const singleResult = (singleResponse as MarkerResponse<ClientAPIVersion>) | ||
.result | ||
if (!(collectKey in singleResult)) { | ||
throw new XrplError(`${collectKey} not in result`) | ||
} | ||
|
@@ -833,7 +856,7 @@ class Client extends EventEmitter<EventTypes> { | |
// A wallet to sign a transaction. It must be provided when submitting an unsigned transaction. | ||
wallet?: Wallet | ||
}, | ||
): Promise<TxResponse<T>> { | ||
): Promise<TxResponse<T, ClientAPIVersion>> { | ||
const signedTx = await getSignedTx(this, transaction, opts) | ||
|
||
const lastLedger = getLastLedgerSequence(signedTx) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this typing change is needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without this,
Client
class will not be able to recognize the API version set in the constructor, and the response ofrequest()
andrequestAll()
will only be the types of APIv2 (DEFAULT_API_VERSION
).