-
Notifications
You must be signed in to change notification settings - Fork 127
refactor: convert some remaining JS(X) files to TS(X) #1269
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
Conversation
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.
Pull request overview
This PR converts the remaining JavaScript/JSX files to TypeScript/TSX as part of an effort to achieve 100% TypeScript coverage in the repository. The changes add type annotations, interfaces, and proper type signatures to approximately 20 files across the rippled API layer and container components, improving type safety and code maintainability.
Key Changes:
- Added TypeScript interfaces and type definitions for API responses, transaction data, and component props
- Converted function signatures to include parameter and return types
- Replaced function constructors with ES6 classes (e.g., RippledError)
- Added proper type narrowing using type predicates in filter operations
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/rippled/transactions.ts | Added TypeScript types for transaction retrieval including TransactionData interface and ExplorerXrplClient type |
| src/rippled/token.ts | Made reserve property optional in TokenData interface |
| src/rippled/quorum.ts | Added type annotations and proper Error constructor usage with status codes |
| src/rippled/offers.ts | Added OrderBook interface and typed parameters for offer retrieval functions |
| src/rippled/nUNL.ts | Added type annotations for negative UNL retrieval and array mapping |
| src/rippled/lib/utils.ts | Added extensive interfaces for account info, NFT data, MPT data, and converted RippledError to ES6 class; added type predicate for filter operation |
| src/rippled/lib/txSummary/index.ts | Added TransactionSummary interface and typed transaction summarization function |
| src/rippled/lib/test/utils.test.ts | Updated test calls to include required tokenType parameter and added @ts-expect-error directives for invalid type tests |
| src/rippled/lib/summarizeLedger.ts | Added LedgerSummary interface and separated ledger_time (raw) from close_time (converted) |
| src/rippled/lib/rippled.ts | Added comprehensive type annotations for all rippled API functions with ExplorerXrplClient type |
| src/rippled/lib/logger.ts | Added string type annotation for message parameter |
| src/rippled/ledgers.ts | Added type annotations for ledger retrieval with proper string/number handling |
| src/rippled/index.ts | Reorganized exports to use type-aware import/export pattern |
| src/rippled/accountTransactions.ts | Added AccountTransactionsResult interface and typed account transaction retrieval |
| src/rippled/accountState.ts | Made reserve property optional in AccountState interface |
| src/rippled/NFTTransactions.ts | Updated imports to use ExplorerXrplClient instead of XrplClient |
| src/containers/shared/transactionUtils.ts | Added return type annotation for decodeHex function |
| src/containers/shared/components/Transaction/NFTokenMint/test/NFTokenMintSimple.test.tsx | Added non-null assertions for details property access in tests |
| src/containers/shared/components/Transaction/NFTokenMint/parser.ts | Updated URI parameter handling with nullish coalescing |
| src/containers/shared/components/Transaction/NFTokenCancelOffer/test/NFTokenCancelOfferSimple.test.tsx | Added non-null assertion for details property access |
| src/containers/Transactions/DetailTab/Meta/types.ts | Created new types file with render function signatures for Meta components |
| src/containers/Transactions/DetailTab/Meta/RippleState.tsx | Applied MetaRenderFunction type to render function |
| src/containers/Transactions/DetailTab/Meta/PayChannel.tsx | Applied MetaRenderFunction type to render function |
| src/containers/Transactions/DetailTab/Meta/Offer.tsx | Applied MetaRenderFunctionWithTx type and added explicit array typing |
| src/containers/Transactions/DetailTab/Meta/MPTokenIssuance.tsx | Applied MetaRenderFunction type to render function |
| src/containers/Transactions/DetailTab/Meta/MPToken.tsx | Applied MetaRenderFunction type to render function |
| src/containers/Transactions/DetailTab/Meta/DirectoryNode.tsx | Applied DirectoryNodeRenderFunction type to render function |
| src/containers/Transactions/DetailTab/Meta/AccountRoot.tsx | Applied MetaRenderFunction type to render function |
| src/containers/Header/Search.tsx | Updated type imports to use ExplorerXrplClient instead of XrplClient |
| src/containers/Footer/index.tsx | Added FC (FunctionComponent) type annotation |
Comments suppressed due to low confidence (14)
src/rippled/lib/utils.ts:235
- The
warningsproperty in FormattedNFTInfo is typed asany[]. Consider using a more specific type for warnings to improve type safety.
src/rippled/lib/utils.ts:248 - The fallback value has changed from just the optional chaining result to
?? 0. This means if bothnft_serialandnft_sequenceare undefined, NFTSerial will now be 0 instead of undefined. Verify this is the intended behavior, as it could affect how consumers handle missing serial numbers.
src/rippled/lib/rippled.ts:712 - The parameter type for
tokenIdisstring | null, but the function uses it directly in the request. If null is passed, it could cause issues. Consider either removing null from the type signature or adding null handling logic in the function body.
src/rippled/transactions.ts:12 - The TransactionData interface uses
anyfor all its properties. Consider using more specific types from the xrpl library (e.g., TransactionMetadata, Transaction) or creating proper interfaces that match the actual structure of these objects. This would provide better type safety and IDE support.
src/rippled/lib/txSummary/index.ts:28 - The parameter
dis typed asany, which loses type safety. Consider creating a proper interface for the transaction data structure or using the Transaction type from the xrpl library.
src/rippled/lib/rippled.ts:41 - The error message construction has been simplified to remove the endpoint URL. This could make debugging more difficult when working with multiple endpoints. Consider keeping the endpoint information in the error message or logging it separately for troubleshooting purposes.
src/rippled/lib/rippled.ts:204 - The type for
accountparameter isstring | unknown, which is unusual. Theunknowntype doesn't add value here sincestring | unknowneffectively becomesunknown. Consider using juststringif that's the expected type, or create a union of specific valid types if multiple types are actually supported.
src/rippled/lib/rippled.ts:279 - The marker type is changed from
marker = ''default parameter tomarker?: any. Theanytype loses type safety - consider usingstring | undefinedsince the marker appears to be a string based on its usage in the function.
src/rippled/lib/rippled.ts:502 - The marker variable is typed as
string | undefinedbut initialized with themarkerparameter. Since the parameter default is an empty string,currentMarkerwill never be undefined initially. Consider usingstringtype or usingmarker || undefinedif you want to treat empty strings as undefined.
src/rippled/accountTransactions.ts:24 - The transactions array is typed as
any[], which loses type safety. Consider using a more specific type for transaction data, possibly from the TransactionSummary interface or a similar type.
src/rippled/offers.ts:8 - The OrderBook interface uses
any[]for the offers array. Consider creating a proper type for offer objects instead of using any, which would improve type safety and make the code more maintainable.
src/rippled/offers.ts:23 - The variable
orderBookis explicitly typed asany, which defeats the purpose of TypeScript. Consider using the OrderBook interface that's defined in the same file, or creating a more specific intermediate type if needed.
src/rippled/lib/txSummary/index.ts:20 - The function parameter types use
anyfort,tx, and the instruction return type. Consider importing proper types from react-i18next for the translation function and using the Transaction type from xrpl library.
src/rippled/lib/rippled.ts:252 - The
escrowsvariable is explicitly typed asany, which loses type safety. Consider creating a proper interface for the escrows object structure with propertiesin,out,total,totalIn, andtotalOut.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| tokenID: meta.nftoken_id, | ||
| tokenTaxon: tx.NFTokenTaxon, | ||
| uri: convertHexToString(tx.URI), | ||
| uri: convertHexToString(tx.URI ?? undefined), |
Copilot
AI
Dec 11, 2025
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.
The conversion from tx.URI to tx.URI ?? undefined is redundant. If tx.URI is already undefined or null, the nullish coalescing to undefined doesn't change the value. You can simply pass tx.URI directly to convertHexToString since it now accepts string | undefined.
| uri: convertHexToString(tx.URI ?? undefined), | |
| uri: convertHexToString(tx.URI), |
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.
nit, but a valid comment?
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.
It doesn't technically work since tx.URI is string | null, not string | undefined
| // Union type for all possible node types - using any to handle the different field structures | ||
| export type MetaNode = any | ||
|
|
||
| // Render function signatures for Meta components | ||
| export type MetaRenderFunction = ( | ||
| t: any, | ||
| language: string, | ||
| action: string, | ||
| node: MetaNode, | ||
| index: number, | ||
| ) => JSX.Element | ||
|
|
||
| export type MetaRenderFunctionWithTx = ( | ||
| t: any, | ||
| language: string, | ||
| action: string, | ||
| node: MetaNode, | ||
| index: number, | ||
| tx: any, | ||
| ) => JSX.Element | ||
|
|
||
| export type DirectoryNodeRenderFunction = ( | ||
| t: any, | ||
| action: string, | ||
| node: MetaNode, | ||
| index: number, | ||
| ) => JSX.Element |
Copilot
AI
Dec 11, 2025
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.
The entire types file uses any extensively, which defeats the purpose of TypeScript. Consider defining more specific types for MetaNode (perhaps a union of specific node types), proper TFunction type from react-i18next for the translation parameter, and specific transaction types from the xrpl library.
| tokenID: meta.nftoken_id, | ||
| tokenTaxon: tx.NFTokenTaxon, | ||
| uri: convertHexToString(tx.URI), | ||
| uri: convertHexToString(tx.URI ?? undefined), |
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.
nit, but a valid comment?
Co-authored-by: Copilot <[email protected]>
## High Level Overview of Change This PR is a fast follow from #1269 to refactor the files converted to TS to use proper async/await structure instead of `then` structure. There is no functionality change, only refactors. ### Context of Change Best practices, easier to read/write ### Type of Change - [x] Refactor (non-breaking change that only restructures code) ### Codebase Modernization Already done ;) ## Before / After No changes ## Test Plan CI passes.
High Level Overview of Change
Title pretty much says it all. No functionality changes.
Context of Change
General repo improvement/cleanup
90% of the repo is already using TS, let's get it to 100%
Augment is quite good at this sort of basic conversion
Type of Change
Codebase Modernization
Before / After
Test Plan
CI passes.