Added generic type determination based on binary value #3045
Conversation
WalkthroughThis update introduces generic boolean type parameters (typically named Changes
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm warn EBADENGINE Unsupported engine { 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧠 Learnings (2)📓 Common learningspackages/xrpl/test/integration/requests/ledgerData.test.ts (9)⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
🔇 Additional comments (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/xrpl/src/client/index.ts (1)
393-405: Avoid unsafe type casting in requestNextPage.The double cast
as unknown as Uon line 405 bypasses TypeScript's type checking and could hide type errors. Consider using a more type-safe approach.Instead of casting, ensure the return type is correctly inferred:
- return this.request(nextPageRequest) as unknown as U + return this.request(nextPageRequest) as Promise<RequestNextPageReturnMap<T, B>>
🧹 Nitpick comments (1)
packages/xrpl/src/models/methods/ledger.ts (1)
183-188: Consider the redundancy inLedgerRequestExpandedTransactionsBinary.The interface extends
LedgerRequest<true>and also explicitly definesbinary: true. While this ensures type safety, the explicitbinary: trueproperty might be redundant since the generic parameter already enforces this.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/xrpl/src/client/index.ts(4 hunks)packages/xrpl/src/models/methods/accountTx.ts(6 hunks)packages/xrpl/src/models/methods/baseMethod.ts(4 hunks)packages/xrpl/src/models/methods/index.ts(10 hunks)packages/xrpl/src/models/methods/ledger.ts(6 hunks)packages/xrpl/src/models/methods/ledgerData.ts(3 hunks)packages/xrpl/src/models/methods/ledgerEntry.ts(3 hunks)packages/xrpl/src/models/methods/nftHistory.ts(5 hunks)packages/xrpl/src/models/methods/tx.ts(7 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: mvadari
PR: XRPLF/xrpl.js#2801
File: packages/xrpl/test/models/Batch.test.ts:0-0
Timestamp: 2025-04-16T15:22:45.633Z
Learning: Using `as any` type assertions is acceptable in test files for the XRPL.js project, as strict typing is not required for test code.
Learnt from: mvadari
PR: XRPLF/xrpl.js#2829
File: packages/xrpl/src/models/methods/ledgerEntry.ts:74-85
Timestamp: 2024-12-12T01:12:01.868Z
Learning: In the XRPL.js library, request parameters use camelCase (e.g., `credentialType`), while transaction model fields follow the XRP Ledger protocol's PascalCase convention (e.g., `CredentialType`).
Learnt from: ckeshava
PR: XRPLF/xrpl.js#2874
File: packages/xrpl/test/models/permissionedDomainDelete.test.ts:15-20
Timestamp: 2025-01-08T02:11:21.997Z
Learning: In validation test cases for XRPL transactions, using generic JSON input (e.g., `as any`) is preferred over strict typing as it better represents real-world scenarios where the `validate` method needs to handle arbitrary JSON/map/dictionary input from users.
packages/xrpl/src/models/methods/baseMethod.ts (1)
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/clawback.test.ts:165-178
Timestamp: 2024-12-06T19:27:11.147Z
Learning: In the integration tests for `clawback.test.ts`, it's acceptable to use `@ts-expect-error` to bypass type checking when verifying ledger entries, and no additional type safety improvements are needed.
packages/xrpl/src/models/methods/ledgerData.ts (3)
Learnt from: mvadari
PR: XRPLF/xrpl.js#2829
File: packages/xrpl/src/models/methods/ledgerEntry.ts:74-85
Timestamp: 2024-12-12T01:12:01.868Z
Learning: In the XRPL.js library, request parameters use camelCase (e.g., `credentialType`), while transaction model fields follow the XRP Ledger protocol's PascalCase convention (e.g., `CredentialType`).
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/clawback.test.ts:165-178
Timestamp: 2024-12-06T19:27:11.147Z
Learning: In the integration tests for `clawback.test.ts`, it's acceptable to use `@ts-expect-error` to bypass type checking when verifying ledger entries, and no additional type safety improvements are needed.
Learnt from: ckeshava
PR: XRPLF/xrpl.js#3027
File: packages/xrpl/src/models/ledger/MPTokenIssuance.ts:13-16
Timestamp: 2025-06-26T17:25:36.429Z
Learning: In the XRPL ecosystem, type choices for amount fields (like `number` vs `string`) in ledger objects such as `LockedAmount` vs `OutstandingAmount` in `MPTokenIssuance` are deliberate design decisions made across multiple products and cannot be changed unilaterally by individual contributors.
packages/xrpl/src/models/methods/index.ts (4)
Learnt from: mvadari
PR: XRPLF/xrpl.js#2829
File: packages/xrpl/src/models/methods/ledgerEntry.ts:74-85
Timestamp: 2024-12-12T01:12:01.868Z
Learning: In the XRPL.js library, request parameters use camelCase (e.g., `credentialType`), while transaction model fields follow the XRP Ledger protocol's PascalCase convention (e.g., `CredentialType`).
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/models/MPTokenAuthorize.test.ts:60-71
Timestamp: 2024-12-06T18:44:55.095Z
Learning: In the XRPL.js library's TypeScript test file `packages/xrpl/test/models/MPTokenAuthorize.test.ts`, negative test cases for invalid `Account` address format, invalid `Holder` address format, invalid `MPTokenIssuanceID` format, and invalid flag combinations are not necessary.
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/mptokenAuthorize.test.ts:29-118
Timestamp: 2024-12-06T19:25:15.376Z
Learning: In the XRPLF/xrpl.js TypeScript client library, when writing tests (e.g., in `packages/xrpl/test/integration/transactions/`), we generally do not need to test rippled server behaviors, because those behaviors are covered by rippled's own integration and unit tests.
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/clawback.test.ts:165-178
Timestamp: 2024-12-06T19:27:11.147Z
Learning: In the integration tests for `clawback.test.ts`, it's acceptable to use `@ts-expect-error` to bypass type checking when verifying ledger entries, and no additional type safety improvements are needed.
packages/xrpl/src/models/methods/ledgerEntry.ts (2)
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/clawback.test.ts:165-178
Timestamp: 2024-12-06T19:27:11.147Z
Learning: In the integration tests for `clawback.test.ts`, it's acceptable to use `@ts-expect-error` to bypass type checking when verifying ledger entries, and no additional type safety improvements are needed.
Learnt from: mvadari
PR: XRPLF/xrpl.js#2829
File: packages/xrpl/src/models/methods/ledgerEntry.ts:74-85
Timestamp: 2024-12-12T01:12:01.868Z
Learning: In the XRPL.js library, request parameters use camelCase (e.g., `credentialType`), while transaction model fields follow the XRP Ledger protocol's PascalCase convention (e.g., `CredentialType`).
packages/xrpl/src/models/methods/tx.ts (5)
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/mptokenAuthorize.test.ts:29-118
Timestamp: 2024-12-06T19:25:15.376Z
Learning: In the XRPLF/xrpl.js TypeScript client library, when writing tests (e.g., in `packages/xrpl/test/integration/transactions/`), we generally do not need to test rippled server behaviors, because those behaviors are covered by rippled's own integration and unit tests.
Learnt from: mvadari
PR: XRPLF/xrpl.js#2801
File: packages/xrpl/src/Wallet/batchSigner.ts:0-0
Timestamp: 2025-04-16T15:55:50.121Z
Learning: When using `encodeForSigningBatch` for Batch transactions in the XRPL.js library, the field names should use camelCase (`flags`, `txIDs`) even though the transaction object properties themselves use PascalCase (`Flags`, `TxIDs`).
Learnt from: mvadari
PR: XRPLF/xrpl.js#2829
File: packages/xrpl/src/models/methods/ledgerEntry.ts:74-85
Timestamp: 2024-12-12T01:12:01.868Z
Learning: In the XRPL.js library, request parameters use camelCase (e.g., `credentialType`), while transaction model fields follow the XRP Ledger protocol's PascalCase convention (e.g., `CredentialType`).
Learnt from: ckeshava
PR: XRPLF/xrpl.js#2829
File: packages/xrpl/src/models/ledger/Credential.ts:36-36
Timestamp: 2024-12-09T07:06:47.258Z
Learning: In TypeScript interfaces, ensure that fields documented as optional are marked with the `?` operator to denote optionality.
Learnt from: mvadari
PR: XRPLF/xrpl.js#2690
File: packages/xrpl/tools/generateModels.js:18-18
Timestamp: 2024-10-08T16:29:11.194Z
Learning: `SField.cpp` and `TxFormats.cpp` are located in the `rippled` repository (`https://github.com/XRPLF/rippled/`), not in the current repository.
packages/xrpl/src/models/methods/nftHistory.ts (3)
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/mptokenAuthorize.test.ts:29-118
Timestamp: 2024-12-06T19:25:15.376Z
Learning: In the XRPLF/xrpl.js TypeScript client library, when writing tests (e.g., in `packages/xrpl/test/integration/transactions/`), we generally do not need to test rippled server behaviors, because those behaviors are covered by rippled's own integration and unit tests.
Learnt from: mvadari
PR: XRPLF/xrpl.js#2829
File: packages/xrpl/src/models/methods/ledgerEntry.ts:74-85
Timestamp: 2024-12-12T01:12:01.868Z
Learning: In the XRPL.js library, request parameters use camelCase (e.g., `credentialType`), while transaction model fields follow the XRP Ledger protocol's PascalCase convention (e.g., `CredentialType`).
Learnt from: mvadari
PR: XRPLF/xrpl.js#2690
File: packages/xrpl/tools/generateModels.js:18-18
Timestamp: 2024-10-08T16:29:11.194Z
Learning: `SField.cpp` and `TxFormats.cpp` are located in the `rippled` repository (`https://github.com/XRPLF/rippled/`), not in the current repository.
packages/xrpl/src/models/methods/accountTx.ts (5)
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/models/MPTokenAuthorize.test.ts:60-71
Timestamp: 2024-12-06T18:44:55.095Z
Learning: In the XRPL.js library's TypeScript test file `packages/xrpl/test/models/MPTokenAuthorize.test.ts`, negative test cases for invalid `Account` address format, invalid `Holder` address format, invalid `MPTokenIssuanceID` format, and invalid flag combinations are not necessary.
Learnt from: mvadari
PR: XRPLF/xrpl.js#2829
File: packages/xrpl/src/models/methods/ledgerEntry.ts:74-85
Timestamp: 2024-12-12T01:12:01.868Z
Learning: In the XRPL.js library, request parameters use camelCase (e.g., `credentialType`), while transaction model fields follow the XRP Ledger protocol's PascalCase convention (e.g., `CredentialType`).
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/mptokenAuthorize.test.ts:29-118
Timestamp: 2024-12-06T19:25:15.376Z
Learning: In the XRPLF/xrpl.js TypeScript client library, when writing tests (e.g., in `packages/xrpl/test/integration/transactions/`), we generally do not need to test rippled server behaviors, because those behaviors are covered by rippled's own integration and unit tests.
Learnt from: mvadari
PR: XRPLF/xrpl.js#2801
File: packages/xrpl/src/Wallet/batchSigner.ts:0-0
Timestamp: 2025-04-16T15:55:50.121Z
Learning: When using `encodeForSigningBatch` for Batch transactions in the XRPL.js library, the field names should use camelCase (`flags`, `txIDs`) even though the transaction object properties themselves use PascalCase (`Flags`, `TxIDs`).
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/clawback.test.ts:165-178
Timestamp: 2024-12-06T19:27:11.147Z
Learning: In the integration tests for `clawback.test.ts`, it's acceptable to use `@ts-expect-error` to bypass type checking when verifying ledger entries, and no additional type safety improvements are needed.
packages/xrpl/src/client/index.ts (4)
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/mptokenAuthorize.test.ts:29-118
Timestamp: 2024-12-06T19:25:15.376Z
Learning: In the XRPLF/xrpl.js TypeScript client library, when writing tests (e.g., in `packages/xrpl/test/integration/transactions/`), we generally do not need to test rippled server behaviors, because those behaviors are covered by rippled's own integration and unit tests.
Learnt from: mvadari
PR: XRPLF/xrpl.js#2829
File: packages/xrpl/src/models/methods/ledgerEntry.ts:74-85
Timestamp: 2024-12-12T01:12:01.868Z
Learning: In the XRPL.js library, request parameters use camelCase (e.g., `credentialType`), while transaction model fields follow the XRP Ledger protocol's PascalCase convention (e.g., `CredentialType`).
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/clawback.test.ts:165-178
Timestamp: 2024-12-06T19:27:11.147Z
Learning: In the integration tests for `clawback.test.ts`, it's acceptable to use `@ts-expect-error` to bypass type checking when verifying ledger entries, and no additional type safety improvements are needed.
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/models/MPTokenAuthorize.test.ts:60-71
Timestamp: 2024-12-06T18:44:55.095Z
Learning: In the XRPL.js library's TypeScript test file `packages/xrpl/test/models/MPTokenAuthorize.test.ts`, negative test cases for invalid `Account` address format, invalid `Holder` address format, invalid `MPTokenIssuanceID` format, and invalid flag combinations are not necessary.
packages/xrpl/src/models/methods/ledger.ts (6)
Learnt from: mvadari
PR: XRPLF/xrpl.js#2829
File: packages/xrpl/src/models/methods/ledgerEntry.ts:74-85
Timestamp: 2024-12-12T01:12:01.868Z
Learning: In the XRPL.js library, request parameters use camelCase (e.g., `credentialType`), while transaction model fields follow the XRP Ledger protocol's PascalCase convention (e.g., `CredentialType`).
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/clawback.test.ts:165-178
Timestamp: 2024-12-06T19:27:11.147Z
Learning: In the integration tests for `clawback.test.ts`, it's acceptable to use `@ts-expect-error` to bypass type checking when verifying ledger entries, and no additional type safety improvements are needed.
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/mptokenAuthorize.test.ts:29-118
Timestamp: 2024-12-06T19:25:15.376Z
Learning: In the XRPLF/xrpl.js TypeScript client library, when writing tests (e.g., in `packages/xrpl/test/integration/transactions/`), we generally do not need to test rippled server behaviors, because those behaviors are covered by rippled's own integration and unit tests.
Learnt from: ckeshava
PR: XRPLF/xrpl.js#2829
File: packages/xrpl/src/models/ledger/Credential.ts:36-36
Timestamp: 2024-12-09T07:06:47.258Z
Learning: In TypeScript interfaces, ensure that fields documented as optional are marked with the `?` operator to denote optionality.
Learnt from: ckeshava
PR: XRPLF/xrpl.js#3027
File: packages/xrpl/src/models/ledger/MPTokenIssuance.ts:13-16
Timestamp: 2025-06-26T17:25:36.429Z
Learning: In the XRPL ecosystem, type choices for amount fields (like `number` vs `string`) in ledger objects such as `LockedAmount` vs `OutstandingAmount` in `MPTokenIssuance` are deliberate design decisions made across multiple products and cannot be changed unilaterally by individual contributors.
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/models/MPTokenAuthorize.test.ts:60-71
Timestamp: 2024-12-06T18:44:55.095Z
Learning: In the XRPL.js library's TypeScript test file `packages/xrpl/test/models/MPTokenAuthorize.test.ts`, negative test cases for invalid `Account` address format, invalid `Holder` address format, invalid `MPTokenIssuanceID` format, and invalid flag combinations are not necessary.
🧬 Code Graph Analysis (7)
packages/xrpl/src/models/methods/baseMethod.ts (1)
packages/xrpl/src/models/common/index.ts (1)
APIVersion(4-4)
packages/xrpl/src/models/methods/ledgerData.ts (2)
packages/xrpl/src/models/methods/index.ts (7)
LedgerDataRequest(564-564)BaseRequest(512-512)LedgerDataLedgerState(568-568)LedgerDataBinaryLedgerEntry(566-566)LedgerDataLabeledLedgerEntry(565-565)LedgerDataResponse(567-567)BaseResponse(513-513)packages/xrpl/src/models/methods/baseMethod.ts (3)
BaseRequest(5-17)LookupByLedgerRequest(19-24)BaseResponse(32-41)
packages/xrpl/src/models/methods/ledgerEntry.ts (2)
packages/xrpl/src/models/methods/baseMethod.ts (3)
BaseRequest(5-17)LookupByLedgerRequest(19-24)BaseResponse(32-41)packages/xrpl/src/models/ledger/index.ts (1)
LedgerEntry(60-60)
packages/xrpl/src/models/methods/tx.ts (5)
packages/xrpl/src/models/methods/baseMethod.ts (2)
BaseRequest(5-17)BaseResponse(32-41)packages/xrpl/src/models/transactions/index.ts (2)
BaseTransaction(1-1)Transaction(7-7)packages/xrpl/src/models/transactions/transaction.ts (1)
Transaction(211-211)packages/xrpl/src/models/transactions/metadata.ts (1)
TransactionMetadata(95-108)packages/xrpl/src/models/common/index.ts (1)
RIPPLED_API_V1(1-1)
packages/xrpl/src/models/methods/nftHistory.ts (5)
packages/xrpl/src/models/methods/index.ts (5)
NFTHistoryRequest(653-653)BaseRequest(512-512)NFTHistoryTransaction(655-655)NFTHistoryResponse(654-654)BaseResponse(513-513)packages/xrpl/src/models/methods/baseMethod.ts (3)
BaseRequest(5-17)LookupByLedgerRequest(19-24)BaseResponse(32-41)packages/xrpl/src/models/common/index.ts (4)
APIVersion(4-4)DEFAULT_API_VERSION(3-3)RIPPLED_API_V2(2-2)ResponseOnlyTxInfo(110-131)packages/xrpl/src/models/transactions/index.ts (1)
Transaction(7-7)packages/xrpl/src/models/transactions/metadata.ts (1)
TransactionMetadata(95-108)
packages/xrpl/src/models/methods/accountTx.ts (4)
packages/xrpl/src/models/methods/baseMethod.ts (2)
BaseRequest(5-17)LookupByLedgerRequest(19-24)packages/xrpl/src/models/common/index.ts (5)
APIVersion(4-4)DEFAULT_API_VERSION(3-3)RIPPLED_API_V2(2-2)ResponseOnlyTxInfo(110-131)RIPPLED_API_V1(1-1)packages/xrpl/src/models/transactions/index.ts (1)
Transaction(7-7)packages/xrpl/src/models/transactions/metadata.ts (1)
TransactionMetadata(95-108)
packages/xrpl/src/models/methods/ledger.ts (2)
packages/xrpl/src/models/methods/index.ts (2)
LedgerRequest(554-554)BaseRequest(512-512)packages/xrpl/src/models/methods/baseMethod.ts (2)
BaseRequest(5-17)LookupByLedgerRequest(19-24)
🔇 Additional comments (26)
packages/xrpl/src/models/methods/baseMethod.ts (1)
1-1: Type safety improvement for API version fieldsThe change from
numbertoAPIVersiontype for theapi_versionproperties improves type safety by restricting the values to the supported API versions (V1 and V2).Also applies to: 16-16, 40-40, 58-58
packages/xrpl/src/models/methods/ledgerData.ts (3)
23-31: Well-implemented generic parameter for binary modeThe generic
Binaryparameter with a default value offalsemaintains backward compatibility while enabling precise typing of the binary mode.
56-60: Correct use of conditional types for discriminated unionsThe conditional type correctly discriminates between
LedgerDataBinaryLedgerEntryandLedgerDataLabeledLedgerEntrybased on theBinaryflag, ensuring type safety for consumers.
67-78: Consistent type propagation in response interfaceThe
Binaryparameter correctly flows through to thestatearray type, maintaining type consistency between request and response.packages/xrpl/src/models/methods/ledgerEntry.ts (3)
22-69: Consistent generic parameter implementationThe
Binarygeneric parameter follows the same pattern as other request interfaces, maintaining consistency across the codebase.
235-245: Good refactoring with LedgerEntryResponseBaseExtracting common response fields into a base interface reduces duplication and improves maintainability.
252-268: Improved type safety with discriminated unionThe conditional type ensures mutual exclusivity between
node_binaryandnodeproperties based on theBinaryflag, preventing ambiguous states where both could be present.packages/xrpl/src/models/methods/index.ts (3)
197-246: Correct selective application of Binary genericThe
Binarygeneric parameter is correctly applied only to request types that support binary mode, maintaining type safety without unnecessary complexity for other requests.
316-476: Comprehensive Binary parameter propagationThe
RequestResponseMapcorrectly propagates theBinaryparameter to all relevant response types, maintaining type safety throughout the request-response flow.
4-4: ConfirmLedgerEntryimport is requiredThe
LedgerEntryimport is used for theLedgerEntryResponse<LedgerEntry, Binary>type in theRequestResponseMap(around line 417), so it’s necessary. Approving this change.packages/xrpl/src/models/methods/tx.ts (6)
1-1: Clean import statementGood cleanup removing the unused
RIPPLED_API_V2import.
13-28: Consistent Binary generic implementationThe
TxRequestinterface follows the established pattern with theBinarygeneric parameter.
50-72: Improved type structure with simplified BaseTxResultMoving the format-specific fields (
meta_blob,meta, etc.) fromBaseTxResultto the conditional types in the response interfaces improves type safety by ensuring these fields only appear in the appropriate context.
79-109: Well-structured conditional response typesThe
TxResponseinterface correctly discriminates between binary and JSON responses, ensuring type safety for consumers.
116-145: Correct V1-specific field namingThe conditional types correctly use V1-specific field names (
txandmeta) for binary responses, maintaining compatibility with the V1 API.
152-157: Proper version mapping with Binary parameterThe
TxVersionResponseMapcorrectly propagates theBinaryparameter to both V1 and V2 response types.packages/xrpl/src/models/methods/nftHistory.ts (4)
1-6: LGTM! Imports support the new generic type system.The added imports are necessary for implementing the generic type parameters and API version discriminations.
18-20: Good implementation of generic binary parameter.The generic
Binaryparameter with a default value offalsemaintains backward compatibility while enabling stronger typing for binary mode requests.Also applies to: 42-42
69-111: Verify the inconsistent date/time fields across different modes.The discriminated union type correctly handles different response shapes, but there are inconsistencies in date/time fields:
- V2 binary mode uses
date- V2 JSON mode uses
close_time_iso- V1 binary mode uses
date- V1 JSON mode has no date/time field
Please confirm these differences align with the rippled API specification.
117-120: Well-structured generic response interface.The generic parameters correctly flow through to the
transactionsarray type, ensuring type-safe responses based on the request parameters.Also applies to: 147-147
packages/xrpl/src/models/methods/ledger.ts (1)
29-31: Consistent implementation of generic binary parameter.The changes align with the pattern used across other request interfaces, providing stronger typing for the binary flag.
Also applies to: 66-66
packages/xrpl/src/models/methods/accountTx.ts (3)
19-21: Consistent generic implementation.The changes follow the established pattern for binary mode typing.
Also applies to: 41-41
68-108: Verify property naming consistency across similar types.The discriminated union structure is well-implemented, but there are naming inconsistencies compared to
NFTHistoryTransaction:
- V1 binary uses
meta(string) here vsmeta_blobin NFTHistoryTransaction- Different date/time field handling
Please confirm these differences align with the rippled API specification for each endpoint.
155-156: Well-structured response type generics.The response types correctly propagate the generic parameters while maintaining backward compatibility through default values.
Also applies to: 163-164, 171-177
packages/xrpl/src/client/index.ts (2)
17-18: Required imports for API version inference.The API version constants are necessary for the enhanced type inference in the request methods.
126-126: Correct generic parameterization of paginated request types.Only the request types that support binary mode are parameterized with the
Binarygeneric, which is the correct approach.Also applies to: 131-132
| 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>, |
There was a problem hiding this comment.
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.
| 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.
1 participant
High Level Overview of Change
Context of Change
Type of Change
Did you update HISTORY.md?
Test Plan